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

prefix_command documentation is confusing and should be reworded #1052

Closed
mwestphal opened this issue Jun 11, 2024 · 9 comments
Closed

prefix_command documentation is confusing and should be reworded #1052

mwestphal opened this issue Jun 11, 2024 · 9 comments

Comments

@mwestphal
Copy link

We read the prefix_command documentation:

App *     prefix_command (bool allow=true)
     Do not parse anything after the first unrecognized option and return. 
bool     prefix_command_ {false}
     If true, return immediately on an unrecognized option (implies allow_extras) INHERITABLE. 

There is an incoherence. The method param name is "allow" implying that setting to true would "allow" unrecognized option but the param doc is they other way around.

On testing, it looks like the param doc is incorrect, setting prefix_command to true indeed "allow" to have unrecognized option.

@henryiii
Copy link
Collaborator

That is what it does, though; it causes parsing to stop as soon as it hits an unrecognized option, which puts them in the "unparsed" options. This "allows" unrecognized options; otherwise if it didn't stop, it would try to recognize them and fail.

@mwestphal
Copy link
Author

I just tested, setting it to true does not cause the parsing to stop, setting it to false cause the parsing to stop.

@henryiii
Copy link
Collaborator

What do you mean by "parsing to stop"? Just checking to make sure we are referring to the same thing.

@mwestphal
Copy link
Author

What I mean by "parsing to stop" is that, when setting allow to false, when encountering an unrecognized option, the parsing stop and the parsing method returns immediatly.

@henryiii
Copy link
Collaborator

Okay, that's what allow=true does, it causes parsing to stop and returns, giving access to all unrecognized options. allow=false cases parsing to continue, which breaks on an option not registered with CLI11. If you call .prefix_command() (which is allow=true), then prefix_command_ is true which returns immediately on unrecognized options.

@mwestphal
Copy link
Author

I'm getting confused a bit now but I think its a vocabulary issue.

in any case, we got confused by the documentation when using this in our app (ParaView).

Indeed, when I hear "it causes parsing to stop", I definitely do not understand "it does not cause an error and give access to unrecognized option".

Nothing critical, but I think this doc could be improved to explain a bit more how to use CLI11 to do a partial parsing of the CLI options (which is the usecase I had).

@henryiii
Copy link
Collaborator

Okay, I think this should be reworded to avid "parsing to stop". Internally, that's what's happening, but that's not a good description for readers. I think we can use the same text for both, since they are the same thing.

@henryiii henryiii changed the title prefix_command documentation is wrong and should be updated prefix_command documentation is confusing and should be reworded Jul 11, 2024
@phlptp phlptp mentioned this issue Jul 27, 2024
@phlptp
Copy link
Collaborator

phlptp commented Jul 27, 2024

@mwestphal this was clarified in #1059, I will leave it open for a day or so if you have any further comment

@mwestphal
Copy link
Author

Lgtm

phlptp added a commit that referenced this issue Jul 29, 2024
Fix docstring related to #1052 
Fix config_to_string with defaults #1007

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@phlptp phlptp closed this as completed Aug 5, 2024
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

3 participants