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

Unnecessary name sanitisation #265

Closed
Fiedzia opened this issue Sep 20, 2019 · 11 comments
Closed

Unnecessary name sanitisation #265

Fiedzia opened this issue Sep 20, 2019 · 11 comments
Labels
enhancement We would love to have this feature! Feel free to supply a PR

Comments

@Fiedzia
Copy link

Fiedzia commented Sep 20, 2019

Clap has no problem with subcommand names starting with colon.
I need this to namespace them (users will be able to create their own, so I want standard ones not to clash witht them.

However something in structopt removes them (and anything that's not a letter) from beginning and end of subcommand name. This seems accidental.

@CreepySkeleton
Copy link
Collaborator

CreepySkeleton commented Sep 20, 2019

It seems like influence of rename_all. @Fiedzia could you please try with explicit #[structopt(name = ".whatever")] and see if it disappears? Better yet, past a reproducer here

@TeXitoi
Copy link
Owner

TeXitoi commented Sep 20, 2019

That's a feature, but you can use

#[structopt(rename_all = "verbatim")]

to manage the names by hand.

@CreepySkeleton
Copy link
Collaborator

@TeXitoi That's right, but I never thought that dots . or other punctuation marks should be renamed by this, rename_all meant to rename SomeCase to some-case and the like?

@CreepySkeleton
Copy link
Collaborator

Err, wait, there's no way to start a name with a colon except for explicit name = ":asda". @Fiedzia do you already do this?

@CreepySkeleton
Copy link
Collaborator

From heck docs

Characters not within words (such as spaces, punctuations, and underscores) are not included in the output string except as they are a part of the case being converted to.

Well, it looks like #[structopt(rename_all = "verbatim")] is the correct solution.

@Fiedzia
Copy link
Author

Fiedzia commented Sep 20, 2019

I am explicitely setting name with name=":somevalue", that doesn't work.
rename_all = "verbatim" does work.

This behavior was surprising though, especially that using clap directly doesn't do that.

@TeXitoi
Copy link
Owner

TeXitoi commented Sep 20, 2019

I think we should not change the name if it is setted explicitly, but it's technically a breaking change.

@TeXitoi TeXitoi added the enhancement We would love to have this feature! Feel free to supply a PR label Sep 20, 2019
@CreepySkeleton
Copy link
Collaborator

CreepySkeleton commented Sep 20, 2019

but it's technically a breaking change.

Technically - yes, it is, but I highly doubt that many users has been relying on this behavior. We should really ask on users.rust-lang.org or reddit

@CreepySkeleton
Copy link
Collaborator

@TeXitoi I think this is basically a bug, and semver states that bugfixes does not require major version bump.

I'd like to conduct a survay on users.rust-lang.org, do you mind?

@TeXitoi
Copy link
Owner

TeXitoi commented Oct 9, 2019

I'm OK with doing it even without asking, but feel free to ask if you prefer to be sure.

This was referenced Nov 3, 2019
TeXitoi pushed a commit that referenced this issue Nov 8, 2019
@CreepySkeleton
Copy link
Collaborator

@TeXitoi this is closed too

@TeXitoi TeXitoi closed this as completed Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement We would love to have this feature! Feel free to supply a PR
Projects
None yet
Development

No branches or pull requests

3 participants