Skip to content

Re-enable command specific help messages #732

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
Mar 7, 2019
Merged

Re-enable command specific help messages #732

merged 2 commits into from
Mar 7, 2019

Conversation

hakanf
Copy link

@hakanf hakanf commented Mar 5, 2019

This makes sure that the global help message wont be printed instead of the command specific one unless we fail to read the configuration file

Currently if you try, for example:

gitlab project list --help

you only get the global argument help message

This makes sure that the global help message wont be printed instead of the command spedific one unless we fail to read the configuration file
@xarx00
Copy link
Contributor

xarx00 commented Mar 5, 2019

Good, it seems to work (within argparse limits).

This certainly fixes the main problem of the CLI help - that you weren't able to obtain a proper advice on available parameters. But when you use wrong arguments (e.g. gitlab group list -x), you still won't obtain a proper help (for gitlab group list) parameters, but only for gitlab parameters. And the help itself is very ugly and confusing.

Unfortunately, I think these problems are built into argparse. I was playing with argparse a little, but I was not able to make it work satisfactorily. I think that argparse is not suitable for handling two levels of subparsers properly, and neither such an number of subparsers (27 rows of subparsers listed in the usage).

I suggest to replace it with Click. I was playing with click too, and I was able to make it work very good for a script similar to gitlab.

@xarx00 xarx00 mentioned this pull request Mar 5, 2019
@hakanf
Copy link
Author

hakanf commented Mar 6, 2019

Sure, I can see your point. And yes, Click seems like a nice fit...
Makes it a bit of a bigger project though. Is your intention that we move to Click instead of this quick patch?

@hakanf
Copy link
Author

hakanf commented Mar 6, 2019

What I meant to say is that I could help out with moving to Click if you want, just that perhaps we separate that out into another patch, so that we dont delay this one...

@xarx00
Copy link
Contributor

xarx00 commented Mar 6, 2019

I am not a maintainer of the project, so I cannot decide about the move to Click. I have created issue #733 for this purpose.

@max-wittig
Copy link
Member

@xarx00 @hakanf What do you think, if we merge this one after testing and move to click later. Is someone willing to contribute that?

@hakanf
Copy link
Author

hakanf commented Mar 6, 2019

@max-wittig , sure, I can work with @xarx00 to move to click.

@max-wittig max-wittig self-assigned this Mar 6, 2019
@max-wittig max-wittig added the cli label Mar 6, 2019
gitlab/cli.py Outdated
@@ -150,6 +147,9 @@ def main():
options.config_file
)
except gitlab.config.ConfigError as e:
if "--help" in sys.argv or "-h" in sys.argv:
parser.print_help()
exit(0)
Copy link
Member

Choose a reason for hiding this comment

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

please use sys.exit(0) like below

@xarx00
Copy link
Contributor

xarx00 commented Mar 6, 2019

@xarx00 @hakanf What do you think, if we merge this one after testing and move to click later. Is someone willing to contribute that?

I definitly agree.

Copy link
Member

@max-wittig max-wittig left a comment

Choose a reason for hiding this comment

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

please use sys.exit(0) like below

Copy link
Member

@max-wittig max-wittig left a comment

Choose a reason for hiding this comment

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

please use sys.exit(0) like below

@max-wittig max-wittig assigned hakanf and unassigned max-wittig Mar 6, 2019
@hakanf
Copy link
Author

hakanf commented Mar 6, 2019

OK, done... (also fixed another location I found using exit instead of sys.exit)

@max-wittig max-wittig merged commit a6e10f9 into python-gitlab:master Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants