Allow multiple flags of the same type

[JacobEvelyn]

I'd like to be able to use the same flag more than once in a command, like in Postgres's pg_dump command:

pg_dump --table users --table events

Ideally there would be some sort of :multiple option that turns these arguments into an array. Note that this is different from an argument that accepts multiple values. Thoughts?

[davetron5000]

This is tricky because GLI uses OptionParser under the hood and OptionParser doesn't support this directly. You could tell the flag it's an Array and then use --table users,events, but it's not clear to me right now how to implement this without digging into OptionParser

[JacobEvelyn]

OptionParser doesn't support this directly.

I'm not totally sure I follow. As far as I can tell this works just fine in OptionParser:

require "optparse"

options = Hash.new { |h, k| h[k] = [] }
OptionParser.new do |opts|
  opts.on("-t", "--table TABLE", "Dump the current table") do |table|
    options[:table] << table
  end
end.parse!

p options
p ARGV

> ruby pg_dump.rb foo --table users --table events bar
{:table=>["users", "events"]}
["foo", "bar"]

This is just an example, obviously you could make the array behavior only happen when a :multiple option is passed. Or you could make it available for every flag without needing a :multiple option, so flags can access the last one with options[:table] as usual (since this is both the most common use case and necessary for backwards compatibility), and an array of flags with options[:table_array] or something like that.

What am I missing?

[davetron5000]

OH, I see. Yeah, you are right. That's how this could work.

[JacobEvelyn]

I'd be happy to take a stab at implementing this, though having never looked at the GLI codebase I'm sure I'll need guidance on where to start!

[davetron5000]

Cool! It's a bit hard to follow, but in OptionParserFactory, we create instances of OptionParser, here: https://github.com/davetron5000/gli/blob/gli-2/lib/gli/option_parser_factory.rb#L57 is where the calls to .on happen.

One thing we'll have to figure out is to make this new feature opt-in, or just always on. "Always On" is nicer, but could be construed as breaking backwards compatibility. Opt-in won't do that, but then complicates things as we have to figure out a way to turn it on or off somewhere. Maybe a first stab at the "always on" method would be simplist, and then we can talk out the virtues of making it opt-in?

[JacobEvelyn]

Thanks for the help, @davetron5000! I'm not sure I follow your argument about what would be backwards compatible here. I suppose when you talk about "always on" I'm not totally clear about which of these options you're describing:

1. There's no multiple: true option passed in the flag specification. All flag values are always stored as an array, no matter how many there are.
2. There's no multiple: true option passed in the flag specification. Flag values are only stored as an array if used more than once.
3. There's no multiple: true option passed in the flag specification. Flag values are stored as they are currently (accessible via options[:flag] and options["flag"] as usual), but there's an additional :flag_arr and "flag_arr" key/value pair in the options hash that stores an array of values passed.
4. There is a multiple: true option passed in the flag specification. ("Always on" here is in contrast to having a global switch of some sort.) All flag values are stored as usual, except flags with multiple: true set, which store their values as arrays, even when used zero or one times.
5. Something else.

I think (4) is the right move here, because it:

• Is backwards compatible (unlike option 1).
• Doesn't require programmers to check if option values are arrays or single values to handle different numbers of arguments (unlike option 2).
• Doesn't pollute the options hash with more keys that could potentially collide with others and will generally be seldom-used (unlike option 3).
• Is easy and intuitive to use.

That's what I've been working on, but let me know if I'm missing something! (Or if I'm not being clear.)

[JacobEvelyn]

To make my proposal more concrete, see that implementation. ^ I had a little trouble understanding how best to add tests (looks like tc_flag.rb is the way to go?)—I've never used Cucumber before so I may need a little help in that department.

[davetron5000]

Yeah, I think you can test this using the unit tests and not cucumber. I believe tc_gli.rb has tests for this (e.g. https://github.com/davetron5000/gli/blob/gli-2/test/tc_gli.rb#L693 tests for a custom type conversion). Let me know if that helps or if it's still not clear (these tests were some of the first Ruby code I ever wrote so…they aren't great :).