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 blank line between items in long help #1642

Closed
casey opened this issue Jan 23, 2020 · 25 comments · Fixed by #2058
Closed

Add blank line between items in long help #1642

casey opened this issue Jan 23, 2020 · 25 comments · Fixed by #2058
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue.
Milestone

Comments

@casey
Copy link
Contributor

casey commented Jan 23, 2020

Currently, items in a long help message sometimes have a blank line between them, and sometimes run together. Compare this output from clap:

FLAGS:
    -h, --help
            Print help message

    -u, --unstable
            Enable unstable features. To avoid premature stabilization and excessive version churn,
            unstable features are unavailable unless this flag is set. Unstable features are not
            bound by semantic versioning stability guarantees, and may be changed or removed at any
            time.
    -V, --version
            Print version number

With this output from man man:

       -B     Specify  which  browser  to  use  on  HTML files.  This option overrides the
              BROWSER environment variable. By default, man uses /usr/bin/less-is,

       -H     Specify a command that renders HTML files as text.   This  option  overrides
              the HTMLPAGER environment variable. By default, man uses /bin/cat,

       -S  section_list
              List  is  a  colon separated list of manual sections to search.  This option
              overrides the MANSECT environment variable.

It might be easier to visually parse if the items in a long help message were always separated by a blank line.

@CreepySkeleton
Copy link
Contributor

Not sure about the default behavior, but sounds like we can make a global setting for it.

@casey
Copy link
Contributor Author

casey commented Jan 30, 2020

Hmm, I just thought that, regardless of the default, it should probably be consistent, so if AppSettings::BlankLinesBetweenItems (just making up a name) isn't passed, then there should be no blank lines between items. I think the current behavior depends on if long_help is passed or not. Items with long_help in my help message don't have blank lines after them, and items without long_help have blank lines.

@pksunkara
Copy link
Member

I don't think there should be a setting for this. Blank line between flags and options should exist since it's a standard.

@pksunkara pksunkara added A-help Area: documentation, including docs.rs, readme, examples, etc... D: easy E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue. C-enhancement Category: Raise on the bar on expectations labels Feb 1, 2020
@pksunkara pksunkara added this to the 3.1 milestone Feb 1, 2020
@CreepySkeleton
Copy link
Contributor

For --help - maybe, but what if all of your descriptions are short and fit in one line?

@pksunkara
Copy link
Member

pksunkara commented Feb 2, 2020

Hmm.. Doing enquirer select --help or enquirer select -h is giving the same thing because the long help messages are just one line.

enquirer-select 0.3.0
Prompt that allows the user to select from a list of options

USAGE:
    enquirer select [FLAGS] [OPTIONS] --message <message> [items]...

FLAGS:
    -h, --help     Prints help information
    -p, --paged    Enables paging. Uses your terminal size

OPTIONS:
    -m, --message <message>      Message for the prompt
    -s, --selected <selected>    Specify number of the item that will be selected by default

ARGS:
    <items>...    Items that can be selected

Maybe this is occurring when long help messages are multi line only. Wonder what happens with a mix of them.

@CreepySkeleton
Copy link
Contributor

I propose:

  • For one-line helps only - no blank line by default
  • If at least one of help messages doesn't fit in one line - blank lines between all options just to be consistent
  • AppSetting::AlwaysBlankLIne and AppSetting::NeverBlankLine (I suck at naming things) so developers are able to manually chose the style they want

@pickfire
Copy link
Contributor

pickfire commented Mar 7, 2020

AppSetting::AlwaysBlankLine and AppSetting::NeverBlankLine does not seemed to play well with one-line helps only - no blank line by default.

Shouldn't there be 3 options? I think I propose an additional AppSetting::AutoBlankLine, where it blank lines only if it does not fit, for the one-line helps only. AppSetting::AlwaysBlankLine should always blank line even thought it fits and AppSetting::NeverBlankLine to never blank lines like how it is currently.

As well, how should we define one-line helps? Is it according to the terminal width? Is it according to 80 characters width? Or according to $MANWIDTH (I set this)?

It may looks bad too if the terminal is very wide and one-line helps without blank lines that is too long may hard readability (looking at youtube-dl help).

Having it too short without blank lines is bad too (looking at aria2c help) especially when the short and long option takes so many characters.

you-get help is probably what we want here, it even aligns the description across groups (flags / options).

@pksunkara
Copy link
Member

I agree regarding the auto.

I think we are checking terminal width somewhere in the code.

@pksunkara
Copy link
Member

@pickfire If you want something easy to pick up, this issue should be good to go 😄

@pickfire
Copy link
Contributor

@pksunkara Yes, this issue seemed easy but the discussion is not complete so I am thinking to take it later once things become clearer.

@CreepySkeleton
Copy link
Contributor

@pickfire I believe both @pksunkara and I agree on the design you described in a comment above. Regarding to aligning to the terminal's width - let's just start working on it and see what tools we have available at our disposal.

@pickfire
Copy link
Contributor

pickfire commented May 1, 2020

    struct Flags: u64 {
        const BLANK_LINE_ALWAYS              = 1 << 42;
        const BLANK_LINE_AUTO                = 1 << 43;

Can we only keep two flag since we can use just two flags to represent 3 of the real flags which can save up some bits for the future? Similar this can be applied to COLOR_ALWAYS, COLOR_AUTO, COLOR_NEVER as well.

@CreepySkeleton I planned to change it to BlankLineAlways, BlankLineAuto and BlankLineNever since that can make these in the documentation side by side and easier to discover.

@CreepySkeleton
Copy link
Contributor

I planned to change it to BlankLineAlways, BlankLineAuto and BlankLineNever since that can make these in the documentation side by side and easier to discover.

That would be cool.

Regarding to concrete representation, I truly believe that the decision here should not be based on "let's save a few bits/bytes" considerations. This bitfield is not part of public API, if we decide that u64 is not enough at some point, we can simply bump it to u128. 128 bits ought to be enough for anybody.

I kind of don't know what would be the best solution here. The problem here is: bits are wery suitable for representing binary yes/no values, but the setting in question has three values, yes/no/detect. Bitflags don't really fit. Maybe a separate enum? @kbknapp @pksunkara any ideas?

@kbknapp
Copy link
Member

kbknapp commented May 1, 2020

Perhaps I'm under-thinking it, but I don't think this should be consumer configurable. These rules seem pretty simple and rely on already implemented rules of just determining if multi-line help should be displayed or not:

  • If clap decides something should have a multi-line help, it should insert a blank line directly following the message.
  • If at least one argument in a heading is determined to need mult-line help, all arguments in that heading should have mult-line help (and thus a blank line inserted directly after)

The issue described by @casey seems to only appear if the long_help is over 120 chars, otherwise it doesn't seem to appear and all seems to work (blank lines are where they should be).

So I think this is actually a bug.

While testing for this I found what I would maybe deem a bug when the wrap_help is not enabled (will post a new issue an link here momentarily).

Edit: See the last example of #1891 to see the bug from this issue in action. Make the long_help less than 120 chars, and this issue goes away.

@pickfire
Copy link
Contributor

pickfire commented May 2, 2020

I kind of don't know what would be the best solution here. The problem here is: bits are wery suitable for representing binary yes/no values, but the setting in question has three values, yes/no/detect. Bitflags don't really fit. Maybe a separate enum? @kbknapp @pksunkara any ideas?

That three values could be fitted into two bytes since only one of of the three can be used, so it is either the following options:

  • 10 auto
  • 01 always
  • 00 never

And also, we can just read 2 bytes, of course it is already in memory but I don't see why we do that since it is not possible to have auto but with the other flag set.

@pksunkara pksunkara added C-bug Category: Updating dependencies and removed C-enhancement Category: Raise on the bar on expectations labels May 2, 2020
@pksunkara
Copy link
Member

Since this is now a bug and not an enhancement, we don't need the flags at all.

@pickfire
Copy link
Contributor

pickfire commented May 2, 2020

@pksunkara Err, so what should I do here? Anything that I can help with?

@pksunkara
Copy link
Member

Try to fix the bug? In the test case of first comment of this issue, there should always be an empty line.

@pickfire
Copy link
Contributor

@casey Can we see the source code for the output? Are you perhaps using long_help?

        Arg::new("arg")
            .long_help("help help help help")

Or do you do it?

        Arg::new("arg")
            .long_help("help help help help
")

@casey
Copy link
Contributor Author

casey commented May 16, 2020

I no longer use long_help, but I think this is the last version of my code which had separate help and long_help: casey/intermodal@4fffa77

@pickfire
Copy link
Contributor

I believe the issue is related to long_help. I cannot seemed to reproduce this, is this still reproducible?

fn blank_lines_between_long_help() {
    let app = App::new("myapp").arg(Arg::new("unstable").short('u').about(
        "Enable unstable features. To avoid premature stabilization and
excessive version churn, unstable features are unavailable unless this flag is
set. Unstable features are not bound by semantic versioning stability
guarantees, and may be changed or removed at any time.",
    ));
    assert!(utils::compare_output(
        app,
        "myapp --help",
        ISSUE_1642_LONG_HELP,
        false
    ));
}

@mkantor
Copy link
Contributor

mkantor commented Aug 11, 2020

Any updates on this issue?

@pickfire it sounded like you were working on it, but the discussion here trailed off and I can't find any related pull requests so I'm assuming it's up for grabs? @pksunkara / @kbknapp / @CreepySkeleton is the consensus still that this is a bug and that a fix without a setting would be acceptable?


BTW, the long_help doc is currently incorrect; it describes the behavior as if this issue were fixed.

It says the output for the example code looks like this:

FLAGS:
   --config
        The config file used by the myprog must be in JSON format
        with only valid keys and may not contain other nonsense
        that cannot be read by this program. Obviously I'm going on
        and on, so I'll stop now.

-h, --help
        Prints help information

-V, --version
        Prints version information

But if you try that exact code with clap 2.33.2, what you actually see is this:

FLAGS:
        --config     
            The config file used by the myprog must be in JSON format
            with only valid keys and may not contain other nonsense
            that cannot be read by this program. Obviously I'm going on
            and on, so I'll stop now.
    -h, --help       
            Prints help information

    -V, --version    
            Prints version information

(Without an empty line before -h, --help.)

@pksunkara
Copy link
Member

@mkantor Go ahead and work on the issue. Yes, it's still a bug. Regarding the long_help doc, please check master instead of v2.33 on docs.rs.

@mkantor
Copy link
Contributor

mkantor commented Aug 11, 2020

Bam! #2058

@bors bors bot closed this as completed in f7e2fbf Aug 12, 2020
@pickfire
Copy link
Contributor

pickfire commented Aug 12, 2020

@mkantor I was working on it but I couldn't reproduce it so I stopped working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
6 participants