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

Default display for String is awkward, ie [default: ] #4976

Open
2 tasks done
nathan-at-least opened this issue Jun 19, 2023 · 3 comments
Open
2 tasks done

Default display for String is awkward, ie [default: ] #4976

nathan-at-least opened this issue Jun 19, 2023 · 3 comments
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

Comments

@nathan-at-least
Copy link

nathan-at-least commented Jun 19, 2023

Please complete the following tasks

Rust Version

rustc 1.69.0 (84c898d65 2023-04-16)

Clap Version

4.3.4

Minimal reproducible code

use clap::Parser;

#[derive(Parser)]
struct Opts {
    #[clap(long, default_value_t)]
    text: String,
}

fn main() {
    Opts::parse();
}

Steps to reproduce the bug with the above code

$ cargo run -- --help

Actual Behaviour

Usage: foo [OPTIONS]

Options:
      --text <TEXT>  [default: ]
  -h, --help         Print help

Expected Behaviour

I do not think this "should" happen given the current release, but IMO it would be more ideal:

Usage: foo [OPTIONS]

Options:
      --text <TEXT>  [default: '']
  -h, --help         Print help

In this proposed feature enhancement, all Display values would be disambiguated via single quote ', so some examples:

  • field type String displays [default '']
  • field type i64 displays [default '0']
  • field type bool displays [default 'false']

Additional Context

The display of default_value_t defaults just literally uses Display, which can be confusing for a variety of defaults:

  • empty string (like in this example)
  • strings containing ']' or other symbols.

unlikely / pathological cases:

  • newlines

In my "expected behavior" I'm imagining consistent shell-like escaping. This need not be literal shell syntax (especially because users can vary which shells they use and across platforms), but only "unambiguous to typical commandline users".

Simply surrounding the Display value with single ticks ' handles the empty string case, but introduces an escaping problem with Display values that contain '. So this complication would make this feature improvement a bit more messy.

Perhaps \'-style escapes would be simple enough?

#[clap(long, default_value = "val: 'blah'")]
text: String,

-would produce:

[default: 'val: \'blah\'']

Debug Output

n/a

@nathan-at-least nathan-at-least added the C-bug Category: Updating dependencies label Jun 19, 2023
@nathan-at-least
Copy link
Author

I filed this as a bug, because I consider UX issues which may cause confusion to users as bugs. I could also see categorizing this as a feature/enhancement.

@epage
Copy link
Member

epage commented Jun 19, 2023

Currently, we only check for to decide if we should use quoting. When we quote, we are currently using Rust's debug representation of a string.

I think checking for empty string and quoting in that case is reasonable.

We also have the code spread out a bit. We should probably consolidate these before quoting for empty strings.

See

@epage epage added A-help Area: documentation, including docs.rs, readme, examples, etc... E-easy Call for participation: Experience needed to fix: Easy / not much labels Jun 19, 2023
@zippy-dice
Copy link
Contributor

Is it ideal to quote for String that is empty or contain spaces?
As a solution, I suggest creating a function to wrap a string by '' or "" and use it in help_template.rs and possible_value.rs.
In that case, I believe there would be no need to change format.rs, as it will always enclose a string by '', regardless of whitespace or empty strings.

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
Projects
None yet
Development

No branches or pull requests

3 participants