Discuss the design of flags

[cdlm]

Discussion started at pillar-markup/pillar#440

@cdlm:
If flags have side-effects then their order of execution must be clear; however there is no obvious order that would be valid in all situations. Sometimes the order of declaration in the specification makes sense, sometimes it's the order of invocation on the command line, and it could also be some arbitrary order depending on the application. Some flags can also be repeated.

I didn't want to assume how and when an app should use its flags. So they are best seen as optional parameters that you use when you need their value, instead of an imperative API to a stateful builder. The meaning blocks are there to lift the string passed on the command line into an object value that's useful to the app. We tried a version where the block had a side-effect, but then in the meaning block it looked like (args at: #all) value. with no immediate indication of what effect this has.

For commands the situation is different because each subcommand selects a narrower subset of the app's behavior. In the end you have one and only one most-specific subcommand, so that makes a convenient entry point.

---

@guillep:

If flags have side-effects then their order of execution must be clear

Yes, and Clap could define an order :). Even more, it could have a default one "definition order" for example. And then accept other strategies like "invocation order", and each command could define the order of flags with a strategy, no?

I didn't want to assume how and when an app should use its flags. So they are best seen as optional parameters that you use when you need their value, instead of an imperative API to a stateful builder. The meaning blocks are there to lift the string passed on the command line into an object value that's useful to the app.

I understand. But to me, we should take a stand and favour one usage over the other :).
And maybe give hooks to the user to override those decisions.
We will never make a choice that pleases everybody, I'd take that outside the table :P.
My heuristic here is that we should favour most common usages, and leave the freedom to do less common usages.

We are now using a full-grown framework to parse command line arguments, so we should extract the juice from it, no?

I'd like to discuss alternatives, tell me what you think:

1. flags with effect. The command executes the flag meaning automatically. This works naturally with optional flags. If some flag is not there, the building method is not called.

myBuilder := MyBuilder new.
ClapCommand new
   add: ((ClapFlag id: #x) meaning: [ myBuilder withX ]);
   add: ((ClapFlag id: #y)
      beIntegerValue; "clap does the parsing and validation for me ;)"
      meaning: [ :anInteger | myBuilder withY: anInteger ]);
   add: ((ClapFlag id: #z) meaning: [ :aString | myBuilder withZ: aString asUppercase ]);
   meaning: [ myBuilder build ].

2. flags with value as explicit argument. The command executes the flag automatically and sends the associated values as argument of the command meaning. Here the meaning of flags defines actually how to "marshall" the flag, what is their value when used from the command line (otherwise having meaning have no sense, no?...).
   => But, as a side effect, all flags become mandatory, because they are arguments in the block.

ClapCommand new
   add: ((ClapFlag id: #x) meaning: [ SomeObject new ]);
   add: ((ClapFlag id: #y) meaning: [ 17 ]);
   add: ((ClapFlag id: #z) meaning: [ :value | value asInteger ]);
   meaning: [ :x :y :z | MyObject withX: x withY: y withZ: z ].

3. flags with value in dictionary. The command executes the flag automatically and just stores the associated values in a dictionary. Here the meaning of flags, again, defines how to "marshall" the flag, what is their value when used from the command line.
   In this version, what we have now IIUC, it's the user responsibility to have conditional code to check for the arguments, which I don't like...

ClapCommand new
   add: ((ClapFlag id: #x) meaning: [ SomeObject new ]);
   add: ((ClapFlag id: #y) meaning: [ 17 ]);
   add: ((ClapFlag id: #z) meaning: [ :value | value asInteger ]);
   meaning: [ :args | MyObject withX: (args at: #x) withY: (args at: #y) withZ: (args at: #z ifAbsent: [0]) ].

I prefer solution 1. I think it is easy to understand and that defining a clear order of execution of flags can simplify the task of the developer (I think definition order is the best one because the developer can foresee what will happen more easily).

Solution 2 is too limiting, and solution 3 too verbose.

But please note that solutions 1 and 3 are somewhat compatible ;). We could use solution 1 and, those people wanting fine grained control could use solution 3.

We tried a version where the block had a side-effect, but then in the meaning block it looked like (args at: #all) value. with no immediate indication of what effect this has.

I did not understand this. My point is that nobody should call (args at: #all) value. at all :). It seems superfluous. It should be called automatically.
Otherwise, why should I do value?

ClapCommand new
   add: ((ClapFlag id: #x) meaning: [ SomeObject new ]);
   meaning: [ :args | (args at: #x) value ].

If the evaluation could be done automatically when accessing the argument:

ClapCommand new
   add: ((ClapFlag id: #x) meaning: [ SomeObject new ]);
   meaning: [ :args | args at: #x ].

or even simpler, because people do not need to understand the semantics of meaning:

ClapCommand new
   add: (ClapFlag id: #x);
   meaning: [ :args | args at: #x ifPresent: [ SomeObject new ] ].

I don't know, tell me what you think about these ideas. I am really convinced that clap could do more :)

[cdlm]

Okay, first impressions:

1. Closes over the builder, which means the specification must be rebuilt since it's specific to each invocation. In my mind the specifications are constants that are available at image startup; rebuilding them on demand was just the easy way. However the builder can be instanciated in the main command's meaning block and passed to flag meanings via aFlag value: builder.

NB. Meaning blocks receive the match as 1st argument and whatever's passed to #value: as second argument.

2. What I don't like about it is that it looks like the bad kind of implicit code: if you reorder flag declarations you have to reorder block arguments accordingly, for instance. Currently the first argument to the meaning block provides access to the same information, except in an explicit way (which is good because at that point you're dealing with user input; IMHO conditional code is not that bad as it sounds in that case).

NB. That version doesn't make all flags mandatory; what it does require is that all flags have a value even when omitted, which you ensure by defining both the #meaning: and the #implicitMeaning: block. Certainly a meaning setter that expects both blocks at once makes sense here.

3. #value is necessary because matches are like nodes in an AST. Besides what the flag means at the app domain level, they hold information that you don't want to drop : position in the match tree and on the command line, string representation as typed on the command line, etc. All that's necessary for error reporting (I want application code to be able to explain syntax or validation errors by printing the command back with wrong parts in red, for instance). Also think of the --help flag, which is basically a singleton, so it needs to navigate to its parent match and document that; --help by itself doesn't have any meaningful value.

Now I'm aware it's a little verbose, but at least it's clear. We can think of convenience accessors, maybe a collective way of accessing values like match values at: #child. You can also definitely use the conditional accessors like #at:ifPresent:ifAbsent:, I would even encourage that as soon as you have more than a simple value or conversion to do.

[cdlm]

About automatically evaluating flags, again, I see flags as parameters. With messages, you don't expect parameters to be executed so that they modify the callee context. Instead, the callee uses whichever parameter it needs at that point.

But to enable effectful flags, I think I would favor some explicit iteration in the main block, like args applyInvokedFlagsTo: applicationModel.

Declaration order is under developer control, but I don't think it matches well with evaluation order.
If you split declarations into several methods, declaration order becomes less obvious.
It also makes sense to group declarations by theme (e.g. in the order they should appear in documentation) rather than in execution order, and that still works if you split declarations across several methods.

Conversely, the user has control over command-line order and will expect it to have meaning, at least in some cases: usually, later flags override or compliment earlier ones.

Then you have to consider what object should be affected (applicationModel above), and how to build it in the first place.

myBuilder := MyBuilder new.
ClapCommand new
   add: ((ClapFlag id: #x) meaning: [ myBuilder withX ]);
   …

The command specification is something that wants to be static, but it closes over the builder which is dynamic (specific to the command execution, or at least to the image session in the current state of things). Of course we have a flexible notion of static in Pharo, but still… I'd like to see a motivating example that relies on building the command specification depending on the context…

For me the builder should be instanciated in the command's meaning block, so flags cannot be evaluated before that, but they have to be before the block returns because that means the command has finished running and the image must quit.

[cdlm]

@guillep @Ducasse @LaurineDargaud