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 --help output more consistent and narrower #10556

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

keszybz
Copy link
Contributor

@keszybz keszybz commented Jul 2, 2022

No description provided.

"-t TIMEOUT_MULTIPLIER, --timeout-multiplier TIMEOUT_MULTIPLIER" made the output
very wide. Let's use METAVAR with short names to reduce output width, and
also provide a hint to the user what type of value is expected, e.g.
"-C DIRECTORY" is better than "-C WD".
Argparse doesn't use dots at the end, so some --help lines had a
dot, and some didn't. So drop them everywhere for consistency.
Also, sentences are capitalized and some texts are reworded a bit.
@codecov
Copy link

codecov bot commented Jul 2, 2022

Codecov Report

Merging #10556 (9876acd) into master (f0e9a44) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #10556      +/-   ##
==========================================
- Coverage   68.82%   68.80%   -0.02%     
==========================================
  Files         406      406              
  Lines       87954    87954              
  Branches    19540    19540              
==========================================
- Hits        60536    60521      -15     
- Misses      22850    22864      +14     
- Partials     4568     4569       +1     
Impacted Files Coverage Δ
mesonbuild/mtest.py 77.21% <100.00%> (ø)
mesonbuild/dependencies/dev.py 56.77% <0.00%> (-1.83%) ⬇️
mesonbuild/dependencies/ui.py 68.45% <0.00%> (-1.35%) ⬇️
mesonbuild/dependencies/cmake.py 83.54% <0.00%> (-1.03%) ⬇️
mesonbuild/dependencies/base.py 85.91% <0.00%> (-0.56%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51dad83...9876acd. Read the comment docs.

@eli-schwartz
Copy link
Member

Argparse doesn't use dots at the end, so some --help lines had a
dot, and some didn't. So drop them everywhere for consistency.

Hmm, but why not add them everywhere for consistency? e.g. now you end up with some --help lines containing multiple sentences where only the first sentence has a dot.

@keszybz
Copy link
Contributor Author

keszybz commented Jul 3, 2022

Argparse doesn't use dots at the end, so some --help lines had a
dot, and some didn't. So drop them everywhere for consistency.

Hmm, but why not add them everywhere for consistency? e.g. now you end up with some --help lines containing multiple sentences where only the first sentence has a dot.

Two reasons: 1. there's an autogenerated messages for --help itself which use a dot and is provided implicitly by argparse. 2. the dots are not that useful here. They take up space and make the text *feel" longer. Help outputs like this are almost always without help. The few places where there are multiple sentences still read OK without the dot.

@eli-schwartz
Copy link
Member

there's an autogenerated messages for --help itself which use a dot and is provided implicitly by argparse.

But this:

-h, --help                               show this help message and exit

also refrains from capitalizing the first word, while you went ahead and capitalized all the manually defined option entries, so the autogenerated --help is still inconsistent. So I don't see the problem with making this one autogenerated message be inconsistent about the dot as well.

@keszybz
Copy link
Contributor Author

keszybz commented Jul 3, 2022

Yes, that's true. But consistency is just one of the reasons. The other reason is that it's just better to not have the dots, as described above. Most --help outputs don't.

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.

None yet

2 participants