Skip to content

feat(cli): add a custom help formatter #2121

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 28, 2022
Merged

feat(cli): add a custom help formatter #2121

merged 1 commit into from
Jul 28, 2022

Conversation

weakcamel
Copy link
Contributor

@weakcamel weakcamel commented Jul 4, 2022 •
edited by nejch
Loading

Add a custom argparse help formatter which overrides output format
for long (over 10 elements) list of choices.

Formatter is derived from argparse.HelpFormatter with minimal necessary
changes.

Closes #456

@weakcamel weakcamel force-pushed the fix_cli_help_output branch from 561c933 to 76e8b27 Compare July 4, 2022 08:49
@weakcamel
Copy link
Contributor Author

This is a proposal of how to fix #456

It isn't great - I had to copy a lot of the original code into the custom help formatter only to disable 2 lines of validation. It does seem to work well though.

What do you think?

$ gitlab --help
Usage: gitlab [-h] [--version] [-v] [-d] [-c CONFIG_FILE] [-g GITLAB] [-o {json,legacy,yaml}] [-f FIELDS] [--server-url SERVER_URL] [--ssl-verify SSL_VERIFY]
              [--timeout TIMEOUT] [--api-version API_VERSION] [--per-page PER_PAGE] [--pagination PAGINATION] [--order-by ORDER_BY] [--user-agent USER_AGENT]
              [--private-token PRIVATE_TOKEN | --oauth-token OAUTH_TOKEN | --job-token JOB_TOKEN]
              { application, application-appearance, application-settings, audit-event, broadcast-message, current-user, current-user-email, current-user-gpg-key,
              current-user-key, current-user-status, deploy-key, deploy-token, dockerfile, event, feature, generic-package, geo-node, gitignore, gitlabciyml, group,
              group-access-request, group-access-token, group-audit-event, group-badge, group-billable-member, group-billable-member-membership, group-board,
              group-board-list, group-cluster, group-custom-attribute, group-deploy-token, group-descendant-group, group-epic, group-epic-award-emoji,
              group-epic-discussion-note, group-epic-issue, group-epic-note, group-epic-note-award-emoji, group-epic-resource-label-event, group-export, group-hook,
              group-import, group-issue, group-issues-statistics, group-label, group-member, group-member-all, group-merge-request, group-milestone,
              group-notification-settings, group-package, group-project, group-runner, group-subgroup, group-variable, group-wiki, hook, issue, issues-statistics,
              key, ldap-group, license, merge-request, namespace, notification-settings, pages-domain, personal-access-token, project, project-access-request,
              project-access-token, project-additional-statistics, project-approval, project-approval-rule, project-artifact, project-audit-event, project-badge,
              project-board, project-board-list, project-branch, project-cluster, project-commit, project-commit-comment, project-commit-discussion,
              project-commit-discussion-note, project-commit-status, project-custom-attribute, project-deploy-token, project-deployment,
              project-deployment-merge-request, project-environment, project-event, project-export, project-file, project-fork, project-hook, project-import,
              project-issue, project-issue-award-emoji, project-issue-discussion, project-issue-discussion-note, project-issue-link, project-issue-note,
              project-issue-note-award-emoji, project-issue-resource-label-event, project-issue-resource-milestone-event, project-issue-resource-state-event,
              project-issues-statistics, project-job, project-key, project-label, project-member, project-member-all, project-merge-request,
              project-merge-request-approval, project-merge-request-approval-rule, project-merge-request-approval-state, project-merge-request-award-emoji,
              project-merge-request-diff, project-merge-request-discussion, project-merge-request-discussion-note, project-merge-request-note,
              project-merge-request-note-award-emoji, project-merge-request-pipeline, project-merge-request-resource-label-event,
              project-merge-request-resource-milestone-event, project-merge-request-resource-state-event, project-merge-train, project-milestone, project-note,
              project-notification-settings, project-package, project-package-file, project-pages-domain, project-pipeline, project-pipeline-bridge,
              project-pipeline-job, project-pipeline-schedule, project-pipeline-schedule-variable, project-pipeline-test-report,
              project-pipeline-test-report-summary, project-pipeline-variable, project-protected-branch, project-protected-environment, project-protected-tag,
              project-push-rules, project-registry-repository, project-registry-tag, project-release, project-release-link, project-remote-mirror, project-runner,
              project-service, project-snippet, project-snippet-award-emoji, project-snippet-discussion, project-snippet-discussion-note, project-snippet-note,
              project-snippet-note-award-emoji, project-storage, project-tag, project-trigger, project-user, project-variable, project-wiki, runner, runner-job,
              snippet, starred-project, todo, topic, user, user-activities, user-custom-attribute, user-email, user-event, user-gpg-key, user-impersonation-token,
              user-key, user-membership, user-personal-access-token, user-project, user-status, variable } ...

GitLab API Command Line Interface

optional arguments:
  -h, --help            show this help message and exit
  --version             Display the version.
  -v, --verbose, --fancy
                        Verbose mode (legacy format only) [env var: GITLAB_VERBOSE]
  -d, --debug           Debug mode (display HTTP requests) [env var: GITLAB_DEBUG]
  -c CONFIG_FILE, --config-file CONFIG_FILE
                        Configuration file to use. Can be used multiple times. [env var: PYTHON_GITLAB_CFG]
  -g GITLAB, --gitlab GITLAB
                        Which configuration section should be used. If not defined, the default selection will be used.
  -o {json,legacy,yaml}, --output {json,legacy,yaml}
                        Output format (v4 only): json|legacy|yaml
  -f FIELDS, --fields FIELDS
                        Fields to display in the output (comma separated). Not used with legacy output
  --server-url SERVER_URL
                        GitLab server URL [env var: GITLAB_URL]
  --ssl-verify SSL_VERIFY
                        Whether SSL certificates should be validated. [env var: GITLAB_SSL_VERIFY]
  --timeout TIMEOUT     Timeout to use for requests to the GitLab server. [env var: GITLAB_TIMEOUT]
  --api-version API_VERSION
                        GitLab API version [env var: GITLAB_API_VERSION]
  --per-page PER_PAGE   Number of entries to return per page in the response. [env var: GITLAB_PER_PAGE]
  --pagination PAGINATION
                        Whether to use keyset or offset pagination [env var: GITLAB_PAGINATION]
  --order-by ORDER_BY   Set order_by globally [env var: GITLAB_ORDER_BY]
  --user-agent USER_AGENT
                        The user agent to send to GitLab with the HTTP request. [env var: GITLAB_USER_AGENT]
  --private-token PRIVATE_TOKEN
                        GitLab private access token [env var: GITLAB_PRIVATE_TOKEN]
  --oauth-token OAUTH_TOKEN
                        GitLab OAuth token [env var: GITLAB_OAUTH_TOKEN]
  --job-token JOB_TOKEN
                        GitLab CI job token [env var: CI_JOB_TOKEN]

resource:
  {
    application,
    application-appearance,
    application-settings,
...
    user-event,
    user-gpg-key,
    user-impersonation-token,
    user-key,
    user-membership,
    user-personal-access-token,
    user-project,
    user-status,
    variable
  }
                        The GitLab resource to manipulate.

@weakcamel weakcamel force-pushed the fix_cli_help_output branch 2 times, most recently from c6156e2 to 9916e22 Compare July 4, 2022 09:57
@nejch nejch force-pushed the fix_cli_help_output branch 2 times, most recently from 319a41d to 3b3bcce Compare July 26, 2022 21:23
@nejch
Copy link
Member

nejch commented Jul 26, 2022 •
edited
Loading

Hey @weakcamel this is great (even if you had to hack argparse - we always knew argparse was never really made to handle bigger CLI apps). I like the idea here as we've kind of given up on argparse and were going to wait until switching to click or so at some point. So thanks a lot for the argparse research and for working on this!

I took the liberty of playing around with this and made changes to make our CI pass. Here's a summary of changes I did on top of yours:

  • cli_internal.py -> _formatter.py as PEP8 suggests that for private/internal modules.
  • made the minimal amount of config to ignore _formatter.py from our linters because we're essentially vendoring stdlib code.
  • removed the add_usage method as we can reuse the base class one by adding from gettext import gettext as _.
  • simplified the formatter by removing brackets and just unconditionally vertically listing the items. I think that's more consistent anyway. I added a bit of padding only at the beginning to ensure consistent indentation.
  • reverted some tiny changes to reflect use of the original base code from argparse etc.
  • squashed the commits to also make our commitlint happy, you're still the author of the commit.

You can see a comparison of my changes on top of yours here: https://github.com/python-gitlab/python-gitlab/compare/a3802ba8fe493ea214805f9f2270a7d59a277d42..319a41d8cf9b272bd3485ea6cc8f34f1c04b6b52

I hope you're ok with me making these changes here, our linters can be quite difficult to get right sometimes.

@nejch nejch marked this pull request as ready for review July 26, 2022 21:34
@nejch
Copy link
Member

nejch commented Jul 26, 2022

@JohnVillalovos assigning this over to you because I made some changes on top of it. I think it's not worth running some of our linters on top of it as this is stdlib.

To test it.. gitlab --help and gitlab project --help 😁

@nejch nejch changed the title add a custom help formatter feat(cli): add a custom help formatter Jul 26, 2022
JohnVillalovos
JohnVillalovos reviewed Jul 27, 2022
View reviewed changes
Copy link
Member

@JohnVillalovos JohnVillalovos left a comment

Choose a reason for hiding this comment

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

Would be nice to have a test which tests this is doing what it says it is.

Is that a reasonable thing to ask for or is it too difficult to test?

@nejch
Copy link
Member

nejch commented Jul 27, 2022

Would be nice to have a test which tests this is doing what it says it is.

Is that a reasonable thing to ask for or is it too difficult to test?

Was hoping to avoid spending time on argparse but added something basic here 😁 8491d4a

Not the most robust test, but should do for now I hope

@nejch nejch requested a review from JohnVillalovos July 27, 2022 07:51
@JohnVillalovos
Copy link
Member

So I created another way of doing this in #2191

@nejch
Copy link
Member

nejch commented Jul 27, 2022

So I created another way of doing this in #2191

@JohnVillalovos that seems to format differently from the current PR and adds more code other than the override, could you add your review here on why you don't like the current approach? :)

@nejch nejch force-pushed the fix_cli_help_output branch from 8491d4a to ef10d37 Compare July 28, 2022 07:46
@nejch
Copy link
Member

nejch commented Jul 28, 2022 •
edited
Loading

@JohnVillalovos I like your idea, to avoid looking at 2 PRs I cherry-picked it here and squashed with co-authored trailers. I just changed the output to match the tests and to bring some variables & terminology back to that used natively in HelpFormatter. Could you take another look? (Edit: the indent is not quite the same anymore with just format_help, but I think we can live with that to stop blocking it)

@weakcamel @JohnVillalovos @nejch
feat(cli): add a custom help formatter
dc08093 
Add a custom argparse help formatter that overrides the output
format to list items vertically.

The formatter is derived from argparse.HelpFormatter with minimal changes.

Co-authored-by: John Villalovos <john@sodarock.com>
Co-authored-by: Nejc Habjan <nejc.habjan@siemens.com>
@nejch nejch force-pushed the fix_cli_help_output branch from ef10d37 to dc08093 Compare July 28, 2022 07:59
@JohnVillalovos JohnVillalovos merged commit 005ba93 into python-gitlab:main Jul 28, 2022
@weakcamel
Copy link
Contributor Author

weakcamel commented Jul 28, 2022 •
edited
Loading

@nejch

simplified the formatter by removing brackets and just unconditionally vertically listing the items. I think that's more consistent anyway. I added a bit of padding only at the beginning to ensure consistent indentation.

IIRC I did the distinction because I noticed that for shorter lists, some outputs were misalligned somewhat so that was yet another temporary hack of the proof of concept. If it now works as expected with your changes - that's even better.

I hope you're ok with me making these changes here, our linters can be quite difficult to get right sometimes.

Absolutely! Thank you so much for doing all the facelifting; I'm not familiar much with python-gitlab's design and plans so you taking over and turning my proof-of-concept into the actual code which fits the rest is great.

Edit: Actually, now that I caught up on the rest ot this PR, I can see that the code wasn't used in the end :D but that's just as well. As long as formatting is better now - I'm happy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnVillalovos JohnVillalovos JohnVillalovos approved these changes

@nejch nejch nejch approved these changes

Assignees

@JohnVillalovos JohnVillalovos

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CLI: List objects vertically
3 participants
@weakcamel @nejch @JohnVillalovos