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

Make it an error to specify other flags with --help or --list #80

Closed
brettcannon opened this issue Feb 27, 2021 · 8 comments · Fixed by #81
Closed

Make it an error to specify other flags with --help or --list #80

brettcannon opened this issue Feb 27, 2021 · 8 comments · Fixed by #81
Milestone

Comments

@brettcannon
Copy link
Owner

From #77

@gaborbernat
Copy link

Sorry to comment here but #77 is locked, so can't do it there,

the current design only checks the first argument and ignores the rest to pass on to Python

I understand, this is highly unexpected for me at least. No other tool I know does this, so I wasn't aware.

Doesn't rust has some form of parse_known_args? :-) Rather than just the first argument?

@brettcannon
Copy link
Owner Author

Unfortunately the flag for the Launcher is dynamic since you could literally say any Python version (else I would have used an argument parsing library and simplified my life 😉). Unless I do a complete version search upfront and then construct the possible completions I can't know ahead of time what the possible flags are. And doing that seems unnecessarily costly just for this use-case.

@gaborbernat
Copy link

I meant parse known args from the python-launcher POV 👍🏻 We can forward everything else to the resolved interpreter.

@brettcannon
Copy link
Owner Author

Right, but there is no "known args from the Python Launcher POV" without knowing what all potential Python versions are. E.g. the Launcher doesn't know if -3.6 is a valid flag to specify when you want Python 3.6. Same goes for 0.9, 42.1, etc. The only way to know that list of flags would be is to do a complete version search like --list does to calculate all valid version flags. This is why I had to hand-roll CLI flag parsing myself since I can't statically specify it otherwise.

@gaborbernat
Copy link

As far as I understand currently known args are --help, --list and any -x.y where x and y are 1+ digit. That's seems fairly simple to parse and handle, no?

@brettcannon
Copy link
Owner Author

Yep, hence this project exists and functions at all. 😉

My point is it will take more time and effort to support what you're asking for and I don't think it's worth it. Doing --list --help once and reading the error message that says to run --help on its own is a one-time thing if you even happen to even run into it. I don't think increasing the code complexity to let you specify --help in multiple positions when it doesn't make sense to me as to what --list --help would even produce for output is worth the effort in supporting. I would much rather just tell the user "you're not using the flags the way the tool was designed; see py --help for details" then try to support this.

As such, I'm not planning to make any more changes around this beyond what I have already done. I understand your suggestion and I appreciate the input, but it isn't worth it to me to implement this idea and/or the support cost of the resulting code.

@gaborbernat
Copy link

I mean I understand your point, but if just 1000 users run into this and 1000 times you have to tell them to not do it, it will be cheaper to just support it IMHO.

Repository owner locked as resolved and limited conversation to collaborators Mar 3, 2021
@brettcannon
Copy link
Owner Author

I've locked the conversation as I consider this topic resolved.

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

Successfully merging a pull request may close this issue.

2 participants