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

--help output should use line wrapping #11895

Closed
w-flo opened this issue Mar 27, 2023 · 6 comments · Fixed by #12013
Closed

--help output should use line wrapping #11895

w-flo opened this issue Mar 27, 2023 · 6 comments · Fixed by #12013
Assignees
Labels
A-cli-help Area: built-in command-line help A-console-output Area: Terminal output, colors, progress bar, etc. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@w-flo
Copy link

w-flo commented Mar 27, 2023

Problem

In cargo 1.68.1, maybe since the clap v4 upgrade (#11159), help text in --help output is not wrapping. This looks bad for example in cargo init --help, because the help text for --vcs is pretty long and it just wraps at the terminal emulator line end without respecting word boundaries.

Maybe the wrap_help feature of clap should be enabled? AIUI, it pulls in a new dependency, but the possibility to wrap at a fixed width (e.g. 80 chars) was dropped in clap v3. [max_]term_width functions apparently exist, but they don't work when the wrap_help feature is disabled, which it is by default (or maybe I'm misunderstanding this issue).

Proposed Solution

Probably enable the clap wrap_help feature, or maybe downgrade to clap v3 and set a fixed wrap width of e.g. 80 characters if the additional dependency is too heavy / unwanted.

Notes

No response

@w-flo w-flo added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Mar 27, 2023
@weihanglo
Copy link
Member

Thanks for the suggestion! Could you provide some screenshots as references? That would help people compare and evaluate different methods. 🙏

@weihanglo weihanglo added the A-console-output Area: Terminal output, colors, progress bar, etc. label Mar 27, 2023
@w-flo
Copy link
Author

w-flo commented Mar 28, 2023

That's a great idea, @weihanglo! Here's the current situation, using (I believe) the default Ubuntu terminal, which is gnome-terminal, and the default terminal size:
Bildschirmfoto vom 2023-03-28 08-17-40

And this is how it looks with the wrap_help clap feature enabled:
Bildschirmfoto vom 2023-03-28 08-23-57

After enabling this feature, the line length will increase/decrease depending on the terminal width. In full-screen console windows, that might be too much, as it's easier to read text that wraps at maybe column 80 or 100, instead of 200. There's max_term_width(), which can be used to limit text to maybe 80 or 100 columns. For a limit of 100, it looks like this when increasing the terminal width (but of course, like in the previous screenshot, it wraps earlier if the window is smaller than 100 columns):
Bildschirmfoto vom 2023-03-28 08-42-49

According to cargo tree, the clap feature pulls in the additional dependency terminal_size, which depends on rustix (Edit: I'm not sure about other OSes). Cargo already depends on rustix because of the clap->clap_builder->is_terminal->rustix dependency chain, so that's the only new dependency. There appears to be no way in clap4 to enable line wrapping without the terminal_size dependency, i.e. using a hard-coded character limit. The term_width and max_term_width functions exist, but do nothing without the "wrap_help" feature. I'm not sure if cargo can simply pull in new dependencies. If that's an issue, I believe clap3 supported hard-coding the wrap width. Or maybe clap4 could be changed to respect the [max_]term_width() setting for wrapping text without the terminal_size dependency, there is an open github issue for this.

@weihanglo
Copy link
Member

Wow! Really appreciate such a detailed response. Thank you!!

And yep, we pay a bit more attention when pulling new dependencies. Your cargo tree result gives us the correct amount of information. The license of terminal_size is also compatible. It is a relatively small crate as well. And it helps readability of not only wide screens but small screens. Personally I think there is no harm pulling it.

I am not sure if we want to set max_term_width(). If we do, 100 sounds like a proper value to me, as it is also the default max_width set by rustfmt.

The only drawback I am aware of is that people lose the ability to copy the entire description of an option as a sentence without extra steps. They will need to remove leading/trailing whitespaces if enabling the feature.

@weihanglo
Copy link
Member

I am sold on enabling this, but before moving on let's ask @epage first.

@epage, with your knowledge of clap, do you see any potential issue of using wrap_help feature in Cargo?

@weihanglo weihanglo added the A-cli-help Area: built-in command-line help label Mar 28, 2023
@epage
Copy link
Contributor

epage commented Mar 28, 2023

No problem pulling it in. When doing the upgrade, I considered adding wrap_help but I evaluated a good number of commands and thought it unneeded (looks like I missed some...). I'm also surprised I didn't comment on this in the upgrade PR since it was a conscious decision I made.

btw in clap v5, I'm looking to change the fallback terminal width to being a max terminal width, see clap-rs/clap#4295

@weihanglo weihanglo added the S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review label Mar 28, 2023
@heisen-li
Copy link
Contributor

heisen-li commented Apr 3, 2023

@rustbot claim. I'm willing to try to solve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli-help Area: built-in command-line help A-console-output Area: Terminal output, colors, progress bar, etc. C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants