Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Substantial revision #32

Open
1 of 6 tasks
mikemckiernan opened this issue Jan 26, 2023 · 4 comments
Open
1 of 6 tasks

Substantial revision #32

mikemckiernan opened this issue Jan 26, 2023 · 4 comments

Comments

@mikemckiernan
Copy link

mikemckiernan commented Jan 26, 2023

Great extension. It's used by my team and I have suggestions for some significant revision.

I have an implementation of all the following items, but I'm cataloging them to find out the most approachable way to open PRs and request feedback.

  • Add basic HTML tests, inspired by test_build_html.py from the Sphinx project. My goal with this item is to prevent introducing regressions. PR: Add HTML tests to avoid regressions #33.
  • Implement the Domain API to enable the following improvements:
  • (Kind of a freebie from implementing the domain...) Create domain-specific xref targets like :command:fancytool install``. These xref targets are valid within the project and can be referenced cross-project with intersphinx.
  • Add an optional command index of all commands and sub-commands in a docset.
  • Add an optional command-by-group index. The proposal here is to add a :idxgroups: directive so that commands can be grouped in an index. In the proposed tests, I used "ham on a stick" and "spam in a cone" as groups, but IRL, the groups are like "Core Services" and "Common Utilities."
  • Add a full_subcommand_name option so that when headings are printed for sub-commands, the fully-qualified command name is printed. My opinion is that this helps orient readers for projects that use deeply nested sub-commands.

I made a fake PR in my fork so you can skim the diffs. If you are open to reviewing all the changes at once, that's fine by me, I just fear that could be unnecessarily challenging. I can make one PR for the first issue. I think the next four have to be one PR, though maybe I can make the domain and tests for xrefs into one PR and maybe break the two different indicies into separate PRs.

I'm not 100% sure, but I think that implementing the domain addresses #11. Refer to an example in a test file or some of the tests.

I hope the proposal is compelling. Please let me know whether you are receptive to one "big bang" PR or which smaller ones you'd prefer. If I can clarify anything, please let me know.

@mikemckiernan
Copy link
Author

@ashb , can you take a peek and let me know if my requests are compelling? If you prefer step-by-step, please let me know what changes you'd like in the first PR. Thanks very much.

@ashb
Copy link
Collaborator

ashb commented Feb 6, 2023

Just seen this point, will take a look

@ashb
Copy link
Collaborator

ashb commented Feb 7, 2023

@mikemckiernan That all looks amazing -- PR in what ever structure you think appropriate. Do we need to bump the minimum sphinx version at all?

@mikemckiernan
Copy link
Author

@mikemckiernan That all looks amazing -- PR in what ever structure you think appropriate. Do we need to bump the minimum sphinx version at all?

The 4.3.2 version seems OK, and my org uses 4.5 at the moment. I don't see a reason to bump the minimum Sphinx version at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants