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 switches from next-line back to same-line format when name/placeholder gets *wider* #4550

Closed
2 tasks done
gnprice opened this issue Dec 13, 2022 · 2 comments
Closed
2 tasks done
Labels
C-bug Category: Updating dependencies

Comments

@gnprice
Copy link

gnprice commented Dec 13, 2022

Please complete the following tasks

Rust Version

rustc 1.65.0 (897e37553 2022-11-02)

Clap Version

master

Minimal reproducible code

Same as #3300.

(That report is on version 3.0.5, but the same behavior is also present at master. I initially ran into it on master, but then found #3300, which provides a nice handy repro recipe.)

Steps to reproduce the bug with the above code

Same as #3300.

Actual Behaviour

The two versions of the application code in the #3300 repro differ by the presence of a value_name field on an argument. One version sets a concise value_name, so that the argument's synopsis is:
--merge-conflict-theirs-diff-header-decoration-style <STYLE_STRING>
The other version leaves out value_name on that argument, so that its placeholder is derived from the name and the synopsis is:
--merge-conflict-theirs-diff-header-decoration-style <MERGE_CONFLICT_THEIRS_DIFF_HEADER_DECORATION_STYLE>

In the version with STYLE_STRING, the -h output is shown in next-line format. By default -h is in same-line format, but the auto-next-line-help logic kicks in: we see that the same-line format would be very wide (around 200 columns), so we switch to next-line in order to better stay within a readable width. Excerpt of the result:

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

        --merge-conflict-ours-diff-header-style <STYLE_STRING>
            Style string for the header above the 'ours' branch merge conflict diff [default:
            normal]

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

        --merge-conflict-theirs-diff-header-style <STYLE_STRING>
            Style string for the header above the 'theirs' branch merge conflict diff [default:
            normal]

In the version with MERGE_CONFLICT_THEIRS_DIFF_HEADER_DECORATION_STYLE, the same-line format is even wider than that. But this time that even-wider output is actually the one we choose to show — we revert to same-line instead of next-line format. Excerpt:

        --merge-conflict-ours-diff-header-decoration-style <STYLE_STRING>                                            Style string for the decoration of the header above the 'ours' merge conflict diff [default: box]
        --merge-conflict-ours-diff-header-style <STYLE_STRING>                                                       Style string for the header above the 'ours' branch merge conflict diff [default: normal]
        --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]
        --merge-conflict-theirs-diff-header-style <STYLE_STRING>                                                     Style string for the header above the 'theirs' branch merge conflict diff [default: normal]

Expected Behaviour

When the help output in same-line format would already be so wide that we decide to switch to next-line format, and then it gets even wider still, that should cause us to stay in next-line format — not to revert back to same-line format.

Additional Context

The key logic is this, in src/output/help_template.rs, as the bool expression determining whether we switch automatically to next-line help format:

            self.term_w >= taken
                && (taken as f32 / self.term_w as f32) > 0.40
                && h_w > (self.term_w - taken)

Specifically, the first line says that if the width taken already occupied by the longest argument synopsis is itself longer than the terminal width self.term_w, we stay in same-line format regardless of the other conditions. Deleting that first comparison fixes this bug.

In #3300, the reporter @dandavison wanted same-line format always, even when very wide; so this behavior, when it applied, acted as an accidental way of getting the behavior he wanted. But, as evidenced by his filing that bug, it's a fragile and counterintuitive way of doing so. There's discussion in that thread (#3300 (comment)) of a more robust way one can get that behavior today, and of possible other ways in the future.

This behavior has been present since the auto-next-line-format logic was first added, in b7793a2 back in 2016-08. (More history described by @epage at #3300 (comment).) I don't see any discussion of this quirk in the original issue thread #597, nor in the related #587.

It's not in the changelog entries added by that commit b7793a2, either. And I don't see any documentation of the auto-next-line feature outside the changelog (e.g. at https://github.com/clap-rs/clap/blob/master/CHANGELOG.md#v2110-2016-08-28 ), so this "give up when even wider" behavior seems not to be documented at all.

As discussed at #3300 (comment) , it's not entirely clear we want this overall auto-next-line-help feature at all. But I think removing this quirk is both an improvement as long as we do keep the overall feature, and a simplifying step that makes it a bit easier to work through removing it if the decision goes in that direction.

Debug Output

No response

@gnprice gnprice added the C-bug Category: Updating dependencies label Dec 13, 2022
@epage
Copy link
Member

epage commented Dec 13, 2022

Could you clarify what about this needs to be a separate conversation than #3300? I do not want to split the conversation on the next-line-help auto-detection between two Issues. Almost all of this issue is spent re-hashing #3300 and if there is something that is unique or different about this that needs to be a separate issue, it was buried in the details.

@epage
Copy link
Member

epage commented Jan 31, 2023

Without further details, I'm closing this out in favor of the other discussions.

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Updating dependencies
Projects
None yet
Development

No branches or pull requests

2 participants