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

Add clap cosmetic argument names #915

Merged
merged 2 commits into from
Jan 16, 2022

Conversation

dandavison
Copy link
Owner

No description provided.

@dandavison dandavison force-pushed the 891-drop-deprecated-options-help-text-arg-names branch from 917863d to 8c741cc Compare January 16, 2022 21:08
@dandavison dandavison force-pushed the 891-drop-deprecated-options-help-text-arg-names branch from 8c741cc to aba42d7 Compare January 16, 2022 21:18
@th1000s
Copy link
Collaborator

th1000s commented Jan 16, 2022

I for my part find these SCREAMING_SNAKE_CASE default specifiers visually jarring and yes, I know Pythons argparse and now clap use them by defaults. Its ok when it is just a short N or NUM, but these longer ones... but I see you try to shorten them to reduce the --for-bar <FOO_BAR> cases.

@dandavison
Copy link
Owner Author

dandavison commented Jan 16, 2022

I for my part find these SCREAMING_SNAKE_CASE default specifiers visually jarring

Agreed. What do you think the best solution is?

  1. Disable them entirely
    I'm not sure if it's possible. E.g. value_name can't be set to None. And if we could, I worry that it would be a problem that user's wouldn't know which options required associated values.
  2. Lower case
    I worry that they're a bit hard to visually separate from the (important) option name
  3. Use a different color
    It may be possible but I haven't found how yet.

Also using upper case allows us to make this sort of informal distinction where here we are giving the allowed variants literally:

--paging <auto|always|never>

I think it's it at least a bit better now in this PR -- before they were screaming back the option name verbatim.

@dandavison
Copy link
Owner Author

Incidentally, check out the concise help via delta -h in this branch -- I think it's going to be useful.

@dandavison
Copy link
Owner Author

Merging this one which uses a smaller set of shorter names (e.g. STYLE_STRING many times over) but I'd be happy if we can identify further improvements.

@dandavison dandavison merged commit 4fa97a6 into master Jan 16, 2022
@dandavison dandavison deleted the 891-drop-deprecated-options-help-text-arg-names branch January 16, 2022 21:44
@th1000s
Copy link
Collaborator

th1000s commented Jan 17, 2022

informal distinction

That's quite formal already and a good reason. I can also see that these are easier to visually separate.

I'd probably replace

STYLE_STRING => STYLE
FORMAT_STRING and other *FORMAT* => FMT
NUMBER_OF_SPACES / WIDTH => N

check out the concise help via delta -h

I couldn't help but notice this breaking the terminal width :)

    --merge-conflict-theirs-diff-header-decoration-style <MERGE_CONFLICT_THEIRS_DIFF_HEADER_DECORATION_STYLE>    Style string for the decoration of the header above the 'theirs' merge conflict diff [default: box]

@dandavison
Copy link
Owner Author

I'd probably replace

OK, I think I like those. People just have to understand once, but they are forever after condemned to look at them...

I couldn't help but notice this breaking the terminal width :)

Right, I found I couldn't change that one for some reason. I reported a bug against clap: clap-rs/clap#3300 but it's of course entirely possible I'm missing something.

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.

2 participants