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

#[arg(value_delimiter = ':')] Delimiter isnt displayed in the help message. #5392

Open
2 tasks done
max-ishere opened this issue Mar 11, 2024 · 8 comments · May be fixed by #5817
Open
2 tasks done

#[arg(value_delimiter = ':')] Delimiter isnt displayed in the help message. #5392

max-ishere opened this issue Mar 11, 2024 · 8 comments · May be fixed by #5817
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing

Comments

@max-ishere
Copy link

Please complete the following tasks

Clap Version

4.5.2

Describe your use case

Most of the arguments are already documented by clap, I didnt even have to do anything (nice). However the delimiter doesnt show up in the help message.

    /// Colon-separated list of lock types
    #[arg(required = true, value_delimiter = ':')]
    #[clap(long)]
    what: Vec<InhibitTarget>,
Options:
      --what <WHAT>  Colon-separated list of lock types [possible values: shutdown, sleep, ...]

Describe the solution you'd like

The delimiter should be shown like this:

Options:
      --what <WHAT>  [`:` separated list of: shutdown, sleep, ...]

Alternatives, if applicable

No response

Additional Context

No response

@max-ishere max-ishere added the C-enhancement Category: Raise on the bar on expectations label Mar 11, 2024
@epage epage added A-help Area: documentation, including docs.rs, readme, examples, etc... S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing labels Mar 11, 2024
@epage
Copy link
Member

epage commented Mar 11, 2024

For #4812, we are looking at including it within the example usage. I'd be interested in exploring ways of doing that here as well.

@max-ishere max-ishere changed the title `#[arg(value-delimiter = ':')] Delimiter isnt displayed in the help message. `#[arg(value_delimiter = ':')] Delimiter isnt displayed in the help message. Mar 12, 2024
@max-ishere max-ishere changed the title `#[arg(value_delimiter = ':')] Delimiter isnt displayed in the help message. #[arg(value_delimiter = ':')] Delimiter isnt displayed in the help message. Mar 12, 2024
Aethelflaed added a commit to Aethelflaed/clap that referenced this issue Nov 14, 2024
Aethelflaed added a commit to Aethelflaed/clap that referenced this issue Nov 14, 2024
Aethelflaed added a commit to Aethelflaed/clap that referenced this issue Nov 14, 2024
Aethelflaed added a commit to Aethelflaed/clap that referenced this issue Nov 14, 2024
@Aethelflaed
Copy link

Placing it in the usage can be similar to what was done for #1052, but then that's only visible when there's more than one value name.

I'm proposing to also add it in the help message of the option/argument

@epage
Copy link
Member

epage commented Nov 14, 2024

that's only visible when there's more than one value name.

Not quite as that is conflating a couple of features which I realized in seeing #5817

In that PR,

            arg!(-f --fake <s> "some help")
                .required(true)
                .required(true)
                .value_names(["some", "val"])
                .action(ArgAction::Set)
                .value_delimiter(':'),
        )

gets rendered as

Usage: test [OPTIONS] --fake <some>:<val> --birthday-song <song> --birthday-song-volume <volume>

Options:
  -f, --fake <some>:<val>  some help [value delimiter: ':']

Seeing that is reminding of the weird interplay of

  • value_names
  • num_args: gets defaulted by value_names
  • value_delimiter

So --fake actually accepts --fake some1:some2:some3 val1:val2:val3. This was likely not intended for that test but slipped through the cracks as features changed through v3. The test was likely trying to get --fake some:val but now we only support unbounded value delimiters.

We have the idea of extra_values

let mut extra_values = false;
extra_values |= val_names.len() < num_vals.max_values();
if self.is_positional() && matches!(*self.get_action(), ArgAction::Append) {
extra_values = true;
}
if extra_values {
rendered.push_str("...");
}

If we used something like that, we could get

  -f, --fake <some>:... <val>:...  some help [value delimiter: ':']

or

  -f, --fake <some>[:...] <val>[:...]  some help [value delimiter: ':']

@epage
Copy link
Member

epage commented Nov 14, 2024

I'm proposing to also add it in the help message of the option/argument

Currently we do this for

Seeing two of those rendered together, we get

Usage: default [OPTIONS]

Options:
      --arg <argument>  Pass an argument to the program. [default: "\n"] [possible values: normal, "
                        ", "\n", "\t", other]
  -h, --help            Print help
  -V, --version         Print version

Overall, I'm not thrilled with these and don't feel like they compose well together, and worry about extending them more. Also, if we can find ways to include them in the usage, it becomes more natural (like I want to do with possible values).

btw a good test case for one, the other, or both options is to see what Cargo's output would look like with changing this.

@Aethelflaed
Copy link

So --fake actually accepts --fake some1:some2:some3 val1:val2:val3. This was likely not intended for that test but slipped through the cracks as features changed through v3. The test was likely trying to get --fake some:val but now we only support unbounded value delimiters.

From what I gathered, there used to be an option to require the delimiter, but without it we're left with allowing both the delimiter and the space separated values. I didn't think that an issue.

Your idea with the extra_values sounds great, and we could even indicate the delimiter as optional with something like:

  -f, --fake <some>[ : | ' ' ]... <val>[ : | ' ' ]...  some help

Although it gets a bit hard to read...

  -f, --fake <some>[[ : | ' ' ]...]

About the [help message] part, if it doesn't impact the direct usage (like a default value), then it could only be display in the long--help version to remove?

@Aethelflaed
Copy link

btw a good test case for one, the other, or both options is to see what Cargo's output would look like with changing this.

Any particular cargo command I can try to see interesting result?

@epage
Copy link
Member

epage commented Nov 19, 2024

From what I gathered, there used to be an option to require the delimiter, but without it we're left with allowing both the delimiter and the space separated values. I didn't think that an issue.

That setting was because of a weird interaction between features and became redundant. Just setting a value delimiter makes it so you only use the value delimiter. You have to explicitly opt-in to num_args)1..) or similar to have them both active.

-f, --fake <some>[ : | ' ' ]... <val>[ : | ' ' ]... some help

If you have both num_args(1..) and value_delimiter(':'), I'm not sure how the value_names should be integrated in, whether they should be shown as part of the first std::env::args_os entry (kind of like you showed though yours is also interleaved) or one value_name per entry (like what I showed)

@epage
Copy link
Member

epage commented Nov 19, 2024

Any particular cargo command I can try to see interesting result?

Hmm, the only delimited value I'm remember is --features and we hand-implement that because we have two delimiters. If we had it showing in the usage, I'd change it so one of the two was done through clap so it would be rendered.

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-enhancement Category: Raise on the bar on expectations S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants