-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Builder and clap_derive
should be WYSIWYG
#2808
Comments
I really do want options alphabetized by default, even if I only have a few of them. For small, quick tools, that'll avoid having options in a haphazard order. For larger tools with more options, I'd want to decide some kind of logical grouping of options, and in that case I might consider turning on DeriveDisplayOrder, but I would want that to be a deliberate choice. |
@joshtriplett thats an interesting use case to keep in mind! This presents a bit of a tension. Unorganized prototypes might want things alphabetized and these benefit most from defaults. In contrast, I know a lot of my CLIs have logical ordering but I forget to set the display order to derive in them. I'd be curious what other precedence there is. I know |
Wouldn't you notice this when reviewing the help (e.g. to polish, or to prepare screenshots or screencasts or manual pages)? |
|
I've been thinking about this issue on and off for a while; about making DeriveDisplayOrder the default. @joshtriplett makes a great point, but if I may make a counter point to get some people's opinions. When building a CLI, small or not, upon first running Contrast that with our current behavior of sorting them by default; if that's what you wanted it's no big deal but if that's not the desired behavior there is no clear solution unless you already know about It's anecdotal but in my experience in the wild I've run into more people who either expect the WYSIWYG ordering and are somewhat surprised by the sorted args, or don't care either way. One last comment about alphabetizing is that with So I'm leaning towards DeriveDisplayOrder should be the default, but I'm open to being swayed by more opinions and discussion. |
I've updated the issue with a summary of the conversations and included my survey of different tools For cases like |
For completeness sake, it'd be possible for us to always sort Unfortunately for us, it's difficult to tell if those tools (that use clap) that are using alphabetical sort currently are doing so because that's what they prefer, or simply because that's what clap v2 did by default. Likewise, it'd be interesting to see if any of those tools also have their declarations in alphabetical order or not. I'm not suggesting we hunt down that info, just musing on the subject 😜 |
Yes, in the Issue, I intentionally referred to this as "Current Sorting Implementation" so we kept in mind that this was changeable. I brought up the custom comparator because it felt like I was seeing a lot of different sort policies and I worry that it'd be hard to have "one true policy". For example, I know |
Talking about discoverable paths forward (whether we support derive-only or just make derive the default), in addition to manually ordering items, people can also override the display order position. In particular, we sort all items within the same display order position. I was curious about providing "sort items of same display position" as a way of allowing sorting to discover we already do it :) |
FWIW, I am okay with changing the default for this in v4 if the user feedback we collect doesn't prefer keeping the current sorting. |
I ended up on a rabbit trail and as part of it, was reading through #1365. Remembering this issue, I realized we can drop * It will no longer be globally configured but sorting would still be available via:
We do this by
By assigning |
I am not sure why we need that if we are allowing them to assign the same display order value and then clap sorts them. |
I was exploring ideas to make it easier to disable the display order en-masse in case concern was raised over having to set it one-by-one, much like we have Whether to do this depends on
A counter idea for this is to have them use #2976 to bulk set it. #2976 doesn't work as well for EDIT: the downside with bulk editing is that users of |
I was considering whether we can implement this now, and just key the The reason we "can't" is ordering. We can't guarantee "can't" is quoted because this is software, there is always something we could do. For example, we could special case this setting like we special case |
In clap 4.0, we will make `DeriveDisplayOrder` the default and this is how you'll disable it. This is part of clap-rs#2808
#3414 added That just leaves (all for clap4)
|
Force sorting with `next_display_order(None)` Fixes clap-rs#2808
Force sorting with `next_display_order(None)` Fixes clap-rs#2808
Force sorting with `next_display_order(None)` Fixes clap-rs#2808
Rust Version
1.55.0
Affected Version of clap
v3.0.0-beta.4
Expected Behavior Summary
Args are ordered as I declare them
Actual Behavior Summary
Args are sorted, without any exception (exceptions are common)
Context
DeriveDisplayOrder
is a very popular flag to turn on. When further researching this, it is popular across CLIs and even when args are sorted, its one manually to allow exceptions.rg
is one major exception for thisI chalk this up to
rg
being an exceptional case in its scope and design and being limited by clap2's lack ofhelp_heading
.Trade offs
Current Sorting Implementation
help_heading
(ierg
)--no-
flags with their positive versions--help
and--version
which are generally left unsortedDeriving Order
#[clap(flatten)]
Survey of CLIs
Sorted
rg
--help
and--version
(uses clap, author confirmed its intentional)tokei
--help
and--version
(uses clap)ls
--help
/--version
df
--help
/--version
less
no-
variantsCustom
cargo
bat
fd
delta
hyperfine
rsync
vim
chmod
no-
variants would not be sortedmount
curl
wget
iptables
traceroute
grep
find
(mostly sorted but there are exceptions(List of commands was inspired by prior Rust command list, some off the top of my head, and a random 50 linux command list I found. I only did a random sampling of the GNU CoreUtils though)
The text was updated successfully, but these errors were encountered: