Skip to content

extend option directive syntax#10840

Merged
AA-Turner merged 1 commit into
sphinx-doc:5.xfrom
marxin:extend-option-directive-syntax
Sep 27, 2022
Merged

extend option directive syntax#10840
AA-Turner merged 1 commit into
sphinx-doc:5.xfrom
marxin:extend-option-directive-syntax

Conversation

@marxin

@marxin marxin commented Sep 20, 2022

Copy link
Copy Markdown
Contributor

The motivation of pull request is a documentation syntax we use in GCC docs (working on the porting):

.. option:: -Wimplicit-fallthrough

  :option:`-Wimplicit-fallthrough` is the same as :option:`-Wimplicit-fallthrough=3`
  and :option:`-Wno-implicit-fallthrough` is the same as
  :option:`-Wimplicit-fallthrough=0`.

.. option:: -Wno-implicit-fallthrough

  Default setting; overrides :option:`-Wimplicit-fallthrough`.

.. option:: -Wimplicit-fallthrough#{n}

  Warn when a switch case falls through.  For example:
  ...

.. option:: -Wimplicit-fallthrough#0

   Disable it altogether.

.. option:: -Wimplicit-fallthrough#1

   Level 1

.. option:: -Wimplicit-fallthrough#2

   Level 2

.. option:: -Wimplicit-fallthrough#3

   Level 3

  ...

Try :option:`-Wimplicit-fallthrough=` or :option:`-Wimplicit-fallthrough` or :option:`-Wimplicit-fallthrough=n`

As seen, we basically need to distinguish:

  1. -Woption and -Woption= directives
  2. -Woption=value1, -Woption=value2, ...

Where it would be handy to reference to any of the aforementioned versions. I came up with a new syntax # that is a non-breaking version of =. Is the suggestion something the community would accept?

Example:
https://splichal.eu/tmp/demo2/html/

@marxin marxin marked this pull request as draft September 20, 2022 10:25
@marxin

marxin commented Sep 26, 2022

Copy link
Copy Markdown
Contributor Author

A bit of context: as you likely know I'm working on transition of th GCC compiler documentation from Texinfo to Sphinx and this feature would help you reference option much better. As seen, we commonly document option values.

Is the Sphinx community interested in the feature? I can add more tests and document it.
What do you think @AA-Turner, @tk0miya ?

@AA-Turner

Copy link
Copy Markdown
Member

My concern is about making the option directive too complicated to maintain for quite a niche usecase.

Is it possible in your conversion work to implement a custom option directive that works for GCC, or a flag to the standard option directive that hooks into your extra processing? If there are constraints that GCC must use only vanilla Sphinx then those routes aren't useable, clearly.

Is the following style usable?

.. option:: -Wimplicit-fallthrough

   Warn when a switch case falls through. Legal values are:
  
   - 0: Disable it altogether.
   - 1: Level 1
   - 2: Level 2
   - 3: Level 3

   :option:`-Wimplicit-fallthrough` is the same as :option:`-Wimplicit-fallthrough=3`
   and :option:`-Wno-implicit-fallthrough` is the same as
   :option:`-Wimplicit-fallthrough=0`.

.. option:: -Wno-implicit-fallthrough

  Default setting; overrides :option:`-Wimplicit-fallthrough`.

Try :option:`-Wimplicit-fallthrough=` or :option:`-Wimplicit-fallthrough` or :option:`-Wimplicit-fallthrough=n`

This would only require changing the option role to ignore = and anything after =.

A

@marxin

marxin commented Sep 26, 2022

Copy link
Copy Markdown
Contributor Author

My concern is about making the option directive too complicated to maintain for quite a niche usecase.

I do share the concert about the maintainability of the functionality.

Is it possible in your conversion work to implement a custom option directive that works for GCC, or a flag to the standard option directive that hooks into your extra processing? If there are constraints that GCC must use only vanilla Sphinx then those routes aren't useable, clearly.

We would like to support only vanilla Sphinx.

Is the following style usable?

.. option:: -Wimplicit-fallthrough

   Warn when a switch case falls through. Legal values are:
  
   - 0: Disable it altogether.
   - 1: Level 1
   - 2: Level 2
   - 3: Level 3

   :option:`-Wimplicit-fallthrough` is the same as :option:`-Wimplicit-fallthrough=3`
   and :option:`-Wno-implicit-fallthrough` is the same as
   :option:`-Wimplicit-fallthrough=0`.

.. option:: -Wno-implicit-fallthrough

  Default setting; overrides :option:`-Wimplicit-fallthrough`.

Try :option:`-Wimplicit-fallthrough=` or :option:`-Wimplicit-fallthrough` or :option:`-Wimplicit-fallthrough=n`

Yes, that's a feasible solution for us, I think.

This would only require changing the option role to ignore = and anything after =.

Yep. That would be a pretty small change, let me push it.

@marxin marxin force-pushed the extend-option-directive-syntax branch from 9f0d216 to 38eae9f Compare September 26, 2022 18:43
@marxin marxin marked this pull request as ready for review September 26, 2022 18:43
@marxin marxin force-pushed the extend-option-directive-syntax branch 4 times, most recently from 2592cfd to ac32bbe Compare September 27, 2022 10:37
Comment thread sphinx/domains/std.py
@AA-Turner

Copy link
Copy Markdown
Member

Please merge 5.x to fix mypy

@AA-Turner AA-Turner added this to the 5.3.0 milestone Sep 27, 2022
@AA-Turner

Copy link
Copy Markdown
Member

Please also add a CHANGES entry

@marxin marxin force-pushed the extend-option-directive-syntax branch from ac32bbe to d631093 Compare September 27, 2022 17:41
@marxin

marxin commented Sep 27, 2022

Copy link
Copy Markdown
Contributor Author

Both done now.

One can cross-reference an option value: :option:`--module=foobar`.
@marxin marxin force-pushed the extend-option-directive-syntax branch from d631093 to 9dee9f7 Compare September 27, 2022 17:45
@AA-Turner AA-Turner merged commit 29e6ada into sphinx-doc:5.x Sep 27, 2022
@AA-Turner

Copy link
Copy Markdown
Member

I don't like the GitHub UI where you're not prompted to review the commit message if your last merge was a "rebase and merge"... for posterity I was going to use:

Allow cross referencing options with values (#10840)

This change means that text following `=` is ignored when searching for a
corresponding option directive to an option cross reference role.

A

@marxin marxin deleted the extend-option-directive-syntax branch September 27, 2022 19:25
@marxin

marxin commented Sep 27, 2022

Copy link
Copy Markdown
Contributor Author

I like the wording improvement, please commit it.

marxin added a commit to marxin/sphinx that referenced this pull request Sep 29, 2022
This supports a commonly used format when it comes to optional argument:
-fauto-profile[=path], where `path` is optional part.

Extends: sphinx-doc#10840
marxin added a commit to marxin/sphinx that referenced this pull request Sep 29, 2022
This supports a commonly used format when it comes to optional argument:
-fauto-profile[=path], where `path` is optional part. Plus it support
a format when space is used: -fauto-profile path.

Extends: sphinx-doc#10840
marxin added a commit to marxin/sphinx that referenced this pull request Sep 29, 2022
This supports a commonly used format when it comes to optional argument:
-fauto-profile[=path], where `path` is optional part. Plus it supports
a format when space is used: -fauto-profile path.

Extends: sphinx-doc#10840
marxin added a commit to marxin/sphinx that referenced this pull request Oct 1, 2022
This supports a commonly used format when it comes to optional argument:
-fauto-profile[=path], where `path` is optional part. Plus it supports
a format when space is used: -fauto-profile path.

Extends: sphinx-doc#10840
marxin added a commit to marxin/sphinx that referenced this pull request Oct 2, 2022
This supports a commonly used format when it comes to optional argument:
-fauto-profile[=path], where `path` is optional part. Plus it supports
a format when space is used: -fauto-profile path.

Extends: sphinx-doc#10840
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
marxin added a commit to marxin/sphinx that referenced this pull request Oct 2, 2022
This supports a commonly used format when it comes to optional argument:
-fauto-profile[=path], where `path` is optional part. Plus it supports
a format when space is used: -fauto-profile path.

Extends: sphinx-doc#10840
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
marxin added a commit to marxin/sphinx that referenced this pull request Oct 2, 2022
This supports a commonly used format when it comes to optional argument:
-fauto-profile[=path], where `path` is optional part. Plus it supports
a format when space is used: -fauto-profile path.

Extends: sphinx-doc#10840
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Oct 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants