Support user-defined model transformations after initialization and before parsing

[MarkoMackic]

Proposes API for #1259

[codecov-io]

Codecov Report

Merging #1266 (a126fa1) into master (abb4559) will increase coverage by 0.00%.
The diff coverage is 95.00%.

@@            Coverage Diff            @@
##             master    #1266   +/-   ##
=========================================
  Coverage     93.82%   93.82%           
- Complexity      461      467    +6     
=========================================
  Files             2        2           
  Lines          6851     6869   +18     
  Branches       1842     1845    +3     
=========================================
+ Hits           6428     6445   +17     
- Misses          138      139    +1     
  Partials        285      285           

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/picocli/CommandLine.java	93.65% <95.00%> (+<0.01%)	323.00 <7.00> (+6.00)

---

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 abb4559...a126fa1. Read the comment docs.

[remkop]

From a functionality perspective, this proposal seems too limited:
all that any preprocessor can do is prevent a subcommand from being added.

What if there are applications that want to dynamically add a subcommand, or add an option, or remove a subcommand from the parent and add it to a different subcommand, or make any other change to the model?

From a design perspective, I prefer interfaces to classes, and ideally an interface with just a single abstract method (to allow the use of lambdas and Groovy closures in various scenarios.)

[MarkoMackic]

Let's do a more generic one :)

[MarkoMackic]

@remkop What do you think about this approach ?

[MarkoMackic]

There is one test failing :) I'll fix that

[remkop]

Thank you for the update, I like the new interface signature very much.
I had some feedback, can you take a look?

On a separate note, I am starting to think perhaps there is a better name than "preprocessor"...

I don't have any ideas yet, open to suggestions.
I am not sure if the name should emphasize the timing, the fact that this is something that is applied between constructing the model and using the model to parse command line args. Or whether the name should emphasize that this is something that allows applications to modify/transform the model.
Still thinking, suggestions welcome!

[MarkoMackic]

@remkop I'll fix the codestyle. We could call it modelPreprocessor. That name empasizes it's done at model build time.

[remkop]

IModelTransformer?

[MarkoMackic]

That makes sense. I'll refactor tomorrow evening.

[MarkoMackic]

@remkop Please replace the X.X in this branch with the release this is going to get into. And read the comment about this after build transformation.

[MarkoMackic]

And also PR title should be changed, because the functionality impact is wider, but I don't have good words to define it

[MarkoMackic]

@remkop Please review, CommandSpec is not final anymore because it's replaced by preprocessors/transformers. This would allow preprocessor/transformer to return totally new command spec ( maybe you want only to have methods implemented in CommandSpec that are able to modify it, than interface signature would be void(CommandSpec) and it could remain final )

[remkop]

Looks good, thank you!
I think we are getting close.
I added some feedback for minor adjustments, can you take a look?

We should also mention this feature in the user manual (index.adoc).
Perhaps add a small section called Model Transformations, between @spec annotation and [Custom Factory].(https://picocli.info/#_custom_factory). If you can provide an initial draft, perhaps with a usage example, then I take care of the rest.

[remkop]

Can we add public getter and setter accessors for this attribute to CommandSpec?
Following the naming convention I use here, that would be

/** Some javadoc here
  * @since 4.6 */
public IModelTransformer modelTransformer() { return modelTransformer; }

/** Some javadoc here
  * @since 4.6 */
public CommandSpec modelTransformer(IModelTransformer modelTransformer) { this.modelTransformer = modelTransformer; return this; }

[MarkoMackic]

That's ok, than we need to add public API to CommandLine to manually trigger the transformation ? I'll see into this.

[MarkoMackic]

@remkop Actually you don't need programmatic API for this :) because you're constructing/editing command line/model on the fly ( i.e. your code does construction ) , but maybe it can serve some purpose. I've left a single test empty to cover this functionality, you could probably write that usecase.

[MarkoMackic]

I've done it.

[MarkoMackic]

@remkop This is going with 4.6 ? I'll handle requested changes today, and write some docs

[MarkoMackic]

@remkop Could you please write the docs ? You have a wider understanding of CommandSpec API. I could only write about filtering subcommands :)

[remkop]

This looks good, thank you very much!

I added some minor feedback, would be nice if you can take care of them, if not, I can do it.
I will update the user manual, no worries.

[MarkoMackic]

@remkop All good :) Please make one final review, add this to user manual and we're ready to go.

[remkop]

Looks good, many thanks!

[MarkoMackic]

@remkop When is 4.6 going to be released ?

[remkop]

I am taking next week off from work. That is when I am planning to merge approved PRs and complete other outstanding work for 4.6 and do the release.

[remkop]

Merged and added documentation.
Feedback welcome!