Support repeated flags

[dtolnay]

Something like:

gflags::define! {
    -f, --file <PATH>...
}

where it would accept the same flag multiple times (-f path1 -f path2 -f path3) and expose them all in a vec or slice to the application.

[nikclayton-dfinity]

Been thinking about this a bit, and poking at the code.

Trying to think about the best way of representing this in the low level Flag type in state.rs.

Couple of options I can see:

Either

a) The underlying storage is always a Vec, and the code in state.rs for setting/getting the value is conditional on whether or not the flag repeats.

b) There are two storage fields, one as a Vec, one as not, and it selects which one to use based on whether or not the flag repeats.

dispatch.rs is where command line argument parsing and setting the flag value happens. Ideally don't want to have to change that -- if a flag doesn't repeat and set() is called repeatedly it should just store the most recent value (e.g., by pushing to the Vec).

Instances of the low-level Flag type are created in define.rs, which would need to be modified to pass a boolean indicating whether or not the flag can hold multiple values.

Suspect that option (a) above makes most sense. Setting a flag value can always append to the Vec, and getting the value either returns the last value in the Vec (if the flag does not repeat) or the whole Vec (if it does).

This would replace the use of the core::sync::atomic types in state.rs with an Arc<Mutex<Vec<T>>> to hold the values.

This also means that the repeat count functionality changes to just return the length of the Vec.

Alternative approach.

What happens if the user is allowed to write something like:

gflags::define! {
    --foo: Vec<&'static str>
}

and make the Vec explicit?

There's a UX benefit to this, Vec is immediately obvious that it's a collection, probably more so than a custom ... somewhere in the flag definition (although ... is consistent with how this is typically shown in help output).

Value would need to be implemented for a range of Vec<T> types (in the same way it's implemented for &'static str, &'static Path, etc at the moment).

An IS_REPEAT or similar member would be needed on the Value trait to indicate that it is a repeating value. Then the code in dispatch.rs would need a new block (in parse()) to push the value on to the Vec instead of setting it.

I think this still needs an Arc<Mutex<Vec<T>>> to store the values, not the existing atomic pointer type -- the existing code assumes that the value will be completely replaced with an atomic store, not modified. Although can AtomicPtr::get_mut() be used safely here to get a mutable ref to the Vec and push on to it?

Other questions

How does this interact with the ability to set default values?

Does the user write something like this?

gflags::define! {
    --foo: Vec<&str> = ["foo", "bar", "baz"]
}

---

Thoughts?