Change the list file argument expansion extension and add a new option

[magicDGS]

• First commit solves Change the list file argument expansion extension from ".list" to ".args" #94
• Second commit adds an option to switch off the list parsing.

[magicDGS]

@cmnbroad - do you want to review this one?

[codecov-io]

Codecov Report

Merging #95 into master will not change coverage.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master      #95   +/-   ##
=========================================
  Coverage     75.68%   75.68%           
- Complexity      576      577    +1     
=========================================
  Files            22       22           
  Lines          2159     2159           
  Branches        447      447           
=========================================
  Hits           1634     1634           
  Misses          350      350           
  Partials        175      175

Impacted Files	Coverage Δ	Complexity Δ
...titute/barclay/help/DefaultDocWorkUnitHandler.java	72.56% <100%> (ø)	77 <0> (ø)	⬇️
...ute/barclay/argparser/CommandLineParserConfig.java	100% <100%> (ø)	2 <2> (?)
...e/barclay/argparser/CommandLineArgumentParser.java	82.35% <100%> (ø)	176 <0> (ø)	⬇️
...ute/barclay/argparser/CommandLineProgramGroup.java	0% <0%> (ø)	0% <0%> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7839ad8...5d07ecd. Read the comment docs.

[lbergelson]

If we're going to do this maybe we should just go all the way and let you specify a list of extension that will be expanded this way. Default to .args but allow people to set empty list for off, or several extensions if they want to support their own set of things?

[magicDGS]

What about a CommandLineParserOptions interface to have also user-implementations of other options in the parser? The idea is similar to the java.nio.file.OpenOption:

• Interface for the CommandLineParserOption
• An enum for switch options (e.g. APPEND_TO_COLLECTIONS) that does not require any specific behaviour implementation-dependent.
• Sub-interfaces for other options, as this one. For example, a ExpandCollectionExtensionOption with a Collection<String> getExtensions() method to return them. For solving Picard interval list files with .list extension can't be used with -L gatk#3555, GATK may return .args (as simple as () -> Collections.singleton(".args")), and other toolkits may implement their own.

This can be also useful for broadinstitute/gatk#2596 to specify some syntax checks/enforcements on arguments (e.g., conversion to kebab-case syntax, or just check after a refactor of all arguments is done). What do you think, @lbergelson and @cmnbroad?

[cmnbroad]

@magicDGS We'd certainly take a look at a PR if you want to propose something.

[magicDGS]

@cmnbroad - I implemented the changes in the next commit to show the idea (slightly modified from the previous description.
@lbergelson - Now there is a way to set none, different, or several extensions.

Back to you!

[cmnbroad]

@magicDGS Thanks for proposing these changes. See my inline comments - I think now might be a good time to change the way we're handling options.

[cmnbroad]

Since we now want to have more complex options for things like custom expansion file extensions, I think it would be cleaner to switch over to using a configuration object that is passed in by clients. Barclay can have a default implementation, with methods like "List getExpansionFileExtensions()", and custom implementations be provided by the clients.

[magicDGS]

So do you mean to use a BarclayConf interface (or mutable object) to pass to the parser? I think that passing the options as single elements makes clear which behaviour is changing/requested...

If we are going to that direction, I can either make another commit here or open a new PR, whatever you prefer...

[cmnbroad]

I meant that instead of the CommandLineParserOption enum or interface, Barclay should have a CommandLineParserConfig class, with methods like "List getExpansionFileExtensions()" and "boolean getAppendToCollections()", with default implementations. Any customizations would be implemented by passing a config object to the constructor which is a subclass of that default implementation. Barclay would just query it; hopefully the object itself won't need any mutators.

When we just had options that were toggles, it would have been overkill, but with the need to return a custom list, this is cleaner, and will eliminate the need for type casting, etc. Reusing this PR is fine, or make a new one if its easier - either way. Thanks!

[magicDGS]

@cmnbroad - I added a commit with the CommandLineParserConfig implementation that you suggested.

[magicDGS]

Closing in favor of #126