Skip to content

manpage: Split out options by argument group.#35

Merged
praiskup merged 1 commit intopraiskup:masterfrom
wwood:argument-group-sections
Jun 9, 2021
Merged

manpage: Split out options by argument group.#35
praiskup merged 1 commit intopraiskup:masterfrom
wwood:argument-group-sections

Conversation

@wwood
Copy link
Copy Markdown

@wwood wwood commented Jun 7, 2021

Hi again,

Here I introduce some opinion, that the manpage should separate the options by argument group rather than putting them all under the OPTIONS heading. For programs with lots of options it helps to organise them e.g. like https://wwood.github.io/CoverM/coverm-genome.html

Thoughts?

@wwood
Copy link
Copy Markdown
Author

wwood commented Jun 7, 2021

Seems this is failing tests - will fix.

@wwood wwood force-pushed the argument-group-sections branch from 64997e4 to 133eae2 Compare June 8, 2021 01:04
@wwood
Copy link
Copy Markdown
Author

wwood commented Jun 8, 2021

OK so fixed the failing tests, mostly by changing the expected output.

Now .SH OPTIONS is .SH ACTIONS because that is the title of the parser.

Also, options not in an argument group are put below subcommands. Is that bad?

Thanks.

@praiskup
Copy link
Copy Markdown
Owner

praiskup commented Jun 8, 2021

Thank you for the PR!

See the example resalloc.1 in tests; what I dislike is that the main command options (--connection and --version) are moved to the bottom of the manual page. They are now below all the sub-commands and theirs options, can we keep them first?

Also, the word ACTIONS sort of replaces the Sub-commands header. So we could keep only ACTIONS?

@wwood wwood force-pushed the argument-group-sections branch from 133eae2 to aaa8105 Compare June 8, 2021 22:00
@wwood
Copy link
Copy Markdown
Author

wwood commented Jun 8, 2021

Fixed re global options movement.

ACTIONS comes from the title of the subparser defined when ading the subparsers. Are you suggesting to delete this bit:

        lines = [
            '.SS',
            self._bold('Sub-commands'),
        ]

?

@praiskup
Copy link
Copy Markdown
Owner

praiskup commented Jun 9, 2021

Yes, either keep "Sub-commands", or "ACTIONS". If I understand your change correctly,
the "Sub-commands" header would always follow immediately after the "ACTIONS" header.

@wwood wwood force-pushed the argument-group-sections branch 2 times, most recently from 46ec60c to 405a702 Compare June 9, 2021 06:16
@wwood
Copy link
Copy Markdown
Author

wwood commented Jun 9, 2021

I kept the title (here .SH ACTIONS) if the subparsers was instantiated with a title, otherwise defaulting to .SS Sub-commands.

I'll let you decide if you want to make changes to that they are both .SH or .SS and commit that.

I've created a few more commits in this branch you might be interested in too https://github.com/wwood/argparse-manpage/tree/bird_opinions - most are self-contained in single commits if you wanted to take them. Some border more closely align with my opinions though so may not be interesting.

@wwood
Copy link
Copy Markdown
Author

wwood commented Jun 9, 2021

Also note I changed the instantiation of the copr so that the default (without title) was being tested, intentionally.

@praiskup praiskup merged commit d41d0a4 into praiskup:master Jun 9, 2021
@praiskup
Copy link
Copy Markdown
Owner

praiskup commented Jun 9, 2021

Thank you, I briefly checked the bird_opinions changes and they look good. I would prefer to have a PR for them, though, and I may have some questions, but nice!

@wwood
Copy link
Copy Markdown
Author

wwood commented Jun 9, 2021

I'm happy enough with my contributions so far, particularly since I've a special use-case for these man pages and don't require the full code base to be functional meaning I'll have to maintain a fork anyway. Still, I'm happy to answer questions - just shoot me an email or tag me here or something.

Thanks.

@praiskup
Copy link
Copy Markdown
Owner

praiskup commented Jun 9, 2021

I've a special use-case for these man pages and don't require the full code base to be functional meaning I'll have to maintain a fork anyway.

Accepted. Thank you anyway!

praiskup added a commit that referenced this pull request Nov 28, 2021
After this patch we generated the manual pages differently on systems
with Python >= 3.10.  This is a temporary revert, till we have a proper
fix.

This reverts commit d41d0a4.
Relates: #35
praiskup added a commit that referenced this pull request Nov 28, 2021
After this patch we generated the manual pages differently on systems
with Python >= 3.10.  This is a temporary revert, till we have a proper
fix.

This reverts commit d41d0a4.
Relates: #35
@praiskup
Copy link
Copy Markdown
Owner

@wwood, there was some problem I don't yet understand, but your patch was breaking
builds with python3.10+. Would you take a look and reopen this PR please?

@praiskup
Copy link
Copy Markdown
Owner

I hope I finally understood the hierarchy, so I rather made a bit more changes to make this correct.
Proposal is in #42.

praiskup added a commit that referenced this pull request Nov 28, 2021
The `add_argument_group()` can be used to add additional
title/description for sub-set of arguments.

Follow-up: #35, #37
Complements: 31f695f
Replaces: d41d0a4
praiskup added a commit that referenced this pull request Nov 28, 2021
The `add_argument_group()` can be used to add additional
title/description for sub-set of arguments.

Follow-up: #35, #37
Complements: 31f695f
Replaces: d41d0a4
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

Successfully merging this pull request may close these issues.

2 participants