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

3 leading hyphens for options (---long) #3309

Open
2 tasks done
tertsdiepraam opened this issue Jan 18, 2022 · 6 comments
Open
2 tasks done

3 leading hyphens for options (---long) #3309

tertsdiepraam opened this issue Jan 18, 2022 · 6 comments
Labels
A-builder Area: Builder API C-enhancement Category: Raise on the bar on expectations M-breaking-change Meta: Implementing or merging this will introduce a breaking change. S-waiting-on-decision Status: Waiting on a go/no-go before implementing
Milestone

Comments

@tertsdiepraam
Copy link
Contributor

Please complete the following tasks

  • I have searched the discussions
  • I have searched the existing issues

Clap Version

3.0.7

Describe your use case

In uutils/coreutils, we are trying to move to clap 3. Some of the original GNU utilities have long options with 3 leading hyphens. This is done because these flags are undocumented and only used for testing purposes. Two examples are:

rm ---presume-input-tty file1
split ---io-blksize=1 file1

We would like to support these with clap 3.

Describe the solution you'd like

The ideal solution would be if I could use a leading hyphen in the string passed to Arg::long, however, all leading hyphens are stripped here:

pub fn long(mut self, l: &'help str) -> Self {
    self.long = Some(l.trim_start_matches(|c| c == '-'));
    self
}

Therefore, I would like to propose that only the first 2 hyphens would be stripped. So this App:

App::new("cli")
    .arg(Arg::new("a").long("a"))
    .arg(Arg::new("b").long("--b"))
    .arg(Arg::new("c").long("---c"))

would be executed with

cli --a --b ---c

This would technically be a breaking change, but I think that there isn't much (if any) code that passes long arguments with more than 2 leading hyphens.

Alternatives, if applicable

I think I have found a workaround using alias, which does not strip leading hyphens. So the workaround looks like this:

Arg::new("presume-input-tty")
    .long("presume-input-tty")
    .alias("-presume-input-tty")

However, this supports both 2 and 3 leading hyphens and is a bit awkward.

Additional Context

Finally, I just want to say that clap 3 has been fantastic so far and the transition is going very smoothly!

@tertsdiepraam tertsdiepraam added the C-enhancement Category: Raise on the bar on expectations label Jan 18, 2022
epage added a commit that referenced this issue Jan 18, 2022
@epage
Copy link
Member

epage commented Jan 18, 2022

Some extra context for anyone else, this is an upgrade from clap2 to clap3 and clap2 seemed to allow arbitrary number of leading - for long arguments

use clap::{App, Arg};

fn main() {
    let matches = build_app().get_matches();
    dbg!(matches);
}

fn build_app() -> App<'static, 'static> {
    App::new("myapp")
        .version("3.0")
        .about("Tests completions")
        .arg(
            Arg::with_name("file")
                .long("file")
                .takes_value(true)
                .help("some input file"),
        )
}
$ cargo run -- -----file foo
   Compiling test-clap v0.1.0 (/home/epage/src/personal/test-clap)
    Finished dev [unoptimized + debuginfo] target(s) in 0.42s
     Running `target/debug/test-clap -----file foo`
[src/main.rs:5] matches = ArgMatches {
    args: {
        "file": MatchedArg {
            occurs: 1,
            indices: [
                2,
            ],
            vals: [
                "foo",
            ],
        },
    },
    subcommand: None,
    usage: Some(
        "USAGE:\n    test-clap [OPTIONS]",
    ),
}

@epage
Copy link
Member

epage commented Jan 18, 2022

Any change to our handling of leading hyphens would be a breaking change and would have to wait until clap 4.

We also have plans to pull out the lexer (#2915) and allow plugging in custom lexers (#1210).

However, this supports both 2 and 3 leading hyphens and is a bit awkward.

This implies with clap2 you support exclusively 3 hyphens for some arguments. Have a small, reproducible example of how you did it? I've tried some ideas but haven't.

I'd overall recommend this approach for the short term as anything else is unlikely to be available until clap 4.

@tertsdiepraam
Copy link
Contributor Author

Some extra context for anyone else, this is an upgrade from clap2 to clap3 and clap2 seemed to allow arbitrary number of leading - for long arguments

Ah yeah, that's important context indeed. That was how we were supporting it before.

This implies with clap2 you support exclusively 3 hyphens for some arguments.

Sorry for being unclear. We don't have that right now. So in that sense, we can actually model this behavior a bit better in clap 3 than in clap 2.

wait until clap 4.

That makes sense. We'll stick with the workaround for now! Thank you for your response!

@epage
Copy link
Member

epage commented Jan 18, 2022

Trying to decide what we should do with this issue moving forward. So far, the options seem to be

  • Closing in favor of alias
  • Close in favor of Support for single hyphen in long arguments #1210 (though doing 3 - on only some args will require some hard coding but getting clap2 behavior should be easy)
  • Keeping open for considering the conditional trim in long
    • Just unsure how confusing that will be to users if we start taking meaning from the number of - especially since what does one - mean?
  • Keeping it open for some not yet discussed solution

Thoughts?

@tertsdiepraam
Copy link
Contributor Author

what does one - mean?

That is indeed a weird thing about my proposal. But I think it makes sense to reject a single -, because it implies support for #1210, but won't work like that in practice.

Alternatively, the trimming of - could be removed altogether, like already happened with short, since it now takes a char instead of &str. This makes the API more consistent and (in my opinion) more predictable, although it will probably break a lot of code. The transition for short was easier, because the type checker would complain about the usage of &str.

Regarding the options you present:

  • Closing in favor is alias feels strange, because I would intuitively expect long and alias to behave the same and I only found this difference when inspecting the source code.
  • Issue Support for single hyphen in long arguments #1210 seems to be fundamentally about something else, although there could be a solution that solves both issues.

Nevertheless, I understand if you want to close this because there is a fairly viable workaround and there is no short term solution.

getting clap2 behavior should be easy

As a side note: I think it's good that the clap 2 behavior is gone and wouldn't want it back unless it's optional.

@epage
Copy link
Member

epage commented Jan 18, 2022

Note that for #1210 I'm more calling out the generalized solution I proposed in the comments (was just being lazy).

I agree about it being weird that long and alias are inconsistent. I was surprised to see that alias worked for you.

If we removed trimming and made long more like alias, we'd need to do it in phases. We'd first remove trimming in clap 4, asserting when leading - are present, and then in clap 5 open up to leading -, so we could help people transition through this. This would also be contingent on the feedback we received. I wish there was a better way for us to receive feedback about future changes than "do it and see who complains".

Nevertheless, I understand if you want to close this because there is a fairly viable workaround and there is no short term solution.

I'm less concerned with "no short term solution" and more with "is any new solution within scope". As long as we feel there is room for a solution within the scope of clap, I'm fine leaving an issue open even if it is longer in the resolving.

@epage epage added A-builder Area: Builder API S-waiting-on-decision Status: Waiting on a go/no-go before implementing M-breaking-change Meta: Implementing or merging this will introduce a breaking change. labels Feb 2, 2022
@epage epage added this to the 5.0 milestone May 4, 2022
epage added a commit to epage/clap that referenced this issue May 4, 2022
This is a step towards clap-rs#3309.  We want to make longs and long aliases
more consistent in how they handle leading dashes.  There is more
flexibility offered in not stripping and it matches the v3 short
behavior of only taking the non-dash form.  This starts the process by
disallowing it completely so people will catch problems with it and
remove their existing leading dashes.  In a subsequent breaking release
we can remove the debug assert and allow triple-leading dashes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-builder Area: Builder API C-enhancement Category: Raise on the bar on expectations M-breaking-change Meta: Implementing or merging this will introduce a breaking change. S-waiting-on-decision Status: Waiting on a go/no-go before implementing
Projects
None yet
Development

No branches or pull requests

2 participants