Skip to content
This repository has been archived by the owner on Jan 1, 2022. It is now read-only.

Better than display_order and DeriveDisplayOrder argument positioning #153

Open
2 tasks done
epage opened this issue Dec 6, 2021 · 8 comments
Open
2 tasks done

Comments

@epage
Copy link
Owner

epage commented Dec 6, 2021

Issue by I60R
Saturday Apr 11, 2020 at 14:06 GMT
Originally opened as clap-rs/clap#1807


Make sure you completed the following tasks

Describe your use case

The same as for Arg::display_order(usize) and DeriveDisplayOrder but should be easier to apply

Describe the solution you'd like

  • Works well with structopt/clap_derive flattening
  • Doesn't move both arguments to top or to the bottom like .display_order (because 999 is default for every argument).
  • DeriveDisplayOrder shouldn't override it

Alternatives, if applicable

Keep using Arg::display_order(usize) and DeriveDisplayOrder

Additional context


@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by kbknapp
Wednesday Apr 29, 2020 at 03:32 GMT


This might be pretty hairy to implement, especially if we end up trying to detect loops (A says display after B, and B says display after A).

I'm leaning towards closing this feature, as it'd significantly impact the code used to generate help messages for (my subjectively) little benefit.

@I60R could you elaborate on why doesn't DeriveDisplayOrder work in your case? Or Arg::help_heading doesn't fit your use case?

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by I60R
Wednesday May 06, 2020 at 06:42 GMT


This might be pretty hairy to implement, especially if we end up trying to detect loops (A says display after B, and B says display after A).

Okay, I understand.

@I60R could you elaborate on why doesn't DeriveDisplayOrder work in your case?

I actually use it. The problem was that it don't play well with structopt flattening, more specifically it doesn't allow to move argument into separate struct while keeping its order in --help message. This could be easily structopt problem, however it also seems that it occurs because current display_order functionality in clap is somehow limited.

Arg::display_after in clap was simply the most straightforward solution here that I was able to imagine

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by kbknapp
Monday May 11, 2020 at 18:41 GMT


I actually use it. The problem was that it don't play well with structopt flattening, more specifically it doesn't allow to move argument into separate struct while keeping its order in --help message. This could be easily structopt problem, however it also seems that it occurs because current display_order functionality in clap is somehow limited.

Aaah ok, I understand. Yeah to me that sounds like either a feature/bug we could work on as part of clap_derive (what was structopt).

I'm open to others opinions as well, but even when flattening the order should be preserved. That sounds like a new issue to me, or perhaps we re-name this issue and update the OP with this new constraint/requirement.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Monday May 11, 2020 at 18:43 GMT


@I60R Could you provide us a sample code where it doesn't work? AFAIK, even flattening should make it work.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by I60R
Monday May 18, 2020 at 20:11 GMT


I'm open to others opinions as well, but even when flattening the order should be preserved. That sounds like a new issue to me, or perhaps we re-name this issue and update the OP with this new constraint/requirement.

Done. I also think that I've found what argument positioning I want:

  1. With DeriveDisplayOrder we increment each arg display_order starting from 1000 but only for those that had it unspecified. Then, if multiple args sharing the same display_order they would be sorted alphabetically (default behavior).

  2. Instead or alongside with we may provide display_all method for App and ArgGroup which would be used like this:

    display_all = &[
        ("FLAGS", &[
            "first",
            "second",
            "x",
            ...
        ]),
        ("OPTIONS", &[
            "nth",
            "nth+1",
            ...
        ]),
        ...,
        ("CUSTOM SECTION", &[
            ...,
            "last-1",
            "last"
        ])
    ],

it could substitute display_order and display functions, plus add more styling options like custom section headers, separators, etc.

@kbknapp is it possible to be implemented?

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Monday May 18, 2020 at 20:22 GMT


custom section headers

We have them. help_heading.

is it possible to be implemented?

You are not exactly giving us a sample of the code where DeriveDisplayOrder is not working in clap_derive. As I said before, flattening the struct should work with the option well.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by I60R
Monday May 18, 2020 at 20:44 GMT


We have them. help_heading.

The whole point isn't in having them, but in having them in the same place and not using them explicitly.

You are not exactly giving us a sample of the code where DeriveDisplayOrder is not working in clap_derive. As I said before, flattening the struct should work with the option well.

That's actually the problem e.g. if you have this unflattened struct:

#[derive(StructOpt)]
#[structopt(global_settings=&[DeriveDisplayOrder])]
struct Opt {
    a: bool,
    b: bool,
    c: bool,
    ...
}

and then refactors it to flattened

#[derive(StructOpt)]
#[structopt(global_settings=&[DeriveDisplayOrder])]
struct Opt {
    b: bool,
    #[structopt(flatten)]
    flattened: Flattened,
    ...
}

#[derive(StructOpt)]
struct Flattened {
    a: bool,
    c: bool,
}

you may find that the original order is messed up (b-a-c) and there's no straightforward way to fix it

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Tuesday May 19, 2020 at 08:39 GMT


With DeriveDisplayOrder we increment each arg display_order starting from 1000 but only for those that had it unspecified. Then, if multiple args sharing the same display_order they would be sorted alphabetically (default behavior).

Yes, that can be implemented.

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

No branches or pull requests

1 participant