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

Retain/keep help arguments sorting? #421

Closed
vn971 opened this issue Aug 8, 2020 · 13 comments
Closed

Retain/keep help arguments sorting? #421

vn971 opened this issue Aug 8, 2020 · 13 comments

Comments

@vn971
Copy link
Contributor

vn971 commented Aug 8, 2020

Hi, thanks for the awesome project!
Is it possible to keep arguments sorting the same as written in the underlying struct? I think there are cases when you want to write arguments in a certain order, and I think it'd be nice to retain this order.

@vn971
Copy link
Contributor Author

vn971 commented Aug 8, 2020

(I tried to think of a MergeRequest, but I can't find any obvious places like a use of HashMap / HashSet. Not sure what's the source of reordering.)

@CreepySkeleton
Copy link
Collaborator

Define "argument-sorting"? I don't understand the question; some examples might help.

@vn971
Copy link
Contributor Author

vn971 commented Aug 9, 2020

I mean in the --help message. For example, given this structopt definition: https://gitlab.memri.io/memri/pod/-/blob/1eb5f671c5270a3cf2a649339d12cd522cf32dca/src/command_line_interface.rs

I get an output like this: https://gitlab.memri.io/memri/pod/snippets/1

As you see, even though "port" is defined first in the source code ( #[structopt(short, long, default_value = "3030")] port: i16, ), it will be in fact listed somewhere in the bottom ( -p, --port <port> Port to listen to [default: 3030] ).

Would it be possible or a better idea to preserve ordering?

@vn971
Copy link
Contributor Author

vn971 commented Aug 9, 2020

( The problem persists if AppSettings::UnifiedHelpMessage is commented out as well )

@TeXitoi
Copy link
Owner

TeXitoi commented Aug 9, 2020

AppSettings::DeriveDisplayOrder

@vn971
Copy link
Contributor Author

vn971 commented Aug 9, 2020

@TeXitoi oh nice, thanks a lot! It works! Is there a reason to not "enable" it by default?

@TeXitoi
Copy link
Owner

TeXitoi commented Aug 9, 2020

That's clap default. But I personally prefer this default, as it makes search in the help message easier.

@CreepySkeleton
Copy link
Collaborator

Is there a reason to not "enable" it by default?

So many men, so many minds. Some prefer alphabetical sorting, some like "as declared" sorting. It's just happened that the former is default, the latter is opt-in, and I also think alphabetical is more common.

@vn971
Copy link
Contributor Author

vn971 commented Aug 9, 2020

@CreepySkeleton that is not alphabetical sort in the first example sort though, is it? https://gitlab.memri.io/memri/pod/snippets/1
I don't recognize the type of sorting at all. When I changed some options around, some options that weren't change actually got swapped, the whole "picture" is moving randomly.

@CreepySkeleton
Copy link
Collaborator

That's... weird. I swear I saw the "alphabetical order" in clap docs, lemme check... nope, I'm not delusional

Displays the arguments and SubCommands in the help message in the order that they were declared in, and not alphabetically which is the default.

Well, I guess you're going to have to live with it until clap 3.0 is out, sorry.

@vn971
Copy link
Contributor Author

vn971 commented Aug 10, 2020

Is it a bug in clap then? I'm a bit lost. It would seem that the latest version of structopt already uses clap 2.33.0, but options are not listed alphabetically in --help.. Am I missing something here?

@CreepySkeleton
Copy link
Collaborator

Yes, this is a bug in clap 2.33. Unfortunately, it's unlikely to ever be fixed because all time we have we put into 3.0; clap 2.x is essentially frozen. You don't seem to be affected by the bug because you use DeriveDisplayOrder, and if you were, you could assign the order manually via display_order = 1, 2, 3... as a workaround.

I'll make sure this is fixed in 3.0.

@vn971
Copy link
Contributor Author

vn971 commented Aug 11, 2020

Thanks, understood, all clear now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants