Skip to content

curl: fix option listing in newer versions #560

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

Merged
merged 2 commits into from
May 1, 2023

Conversation

algorythmic
Copy link
Contributor

_parse_help only takes a single parameter (which is then splits) for the options that it invokes the tool with, so curl was being invoked with just --help, not with --help all.

@algorythmic
Copy link
Contributor Author

The test failure seems unrelated, is there something else I should do here?

@scop
Copy link
Owner

scop commented Aug 27, 2021

Rebasing should fix the test failure.

With regards to what to do here, I'd like to see a test suite addition for this. Basically one for which we'd check that an option that is only output with --help all but is common enough to be available in ~all curl is emitted as a completion. For example curl --data-binar and then check that --data-binary is with suggested completions. This would be done in test/t/test_curl.py, and it would be a require_cmd=True one.

Additionally, we've adopted conventional commits comment message conventions recently, and starting from next version it's important we follow that, because it has direct effect on change log and bash-completion version generation. So rewording while rebasing suggested, let's say fix(curl): --help all parsing would be an appropriate first line, the rest of your current commit message is great as is.

@danyspin97
Copy link

Hi! Any update on this PR?

@akinomyoga
Copy link
Collaborator

I need this change before performing the interface change of _parse_help.

@akinomyoga akinomyoga force-pushed the curl-help-category branch from cd54397 to 2edaa32 Compare May 1, 2023 11:01
_parse_help only takes a single parameter (which is then splits) for
the options that it invokes the tool with.
Copy link
Owner

@scop scop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cheers, added the requested test case, I'm good with merging this if/when it passes CI.

@akinomyoga
Copy link
Collaborator

Thank you for adding the test!

@akinomyoga akinomyoga merged commit 92ccf03 into scop:master May 1, 2023
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.

4 participants