suggested enhancement to the `flag` module to reduce boilerplate code

[krader1961]

I originally asked this on the discussion channels and decided it warranted an issue given the reply by @xiaq.

I used the flag:parse-getopt function for the first time to implement a which function. I did this because the Windows MSYS2 which command produces output inconsistent with Elvish (e.g., /usr/bin/ls rather than c:\msys64\usr\bin\ls.exe) which is slightly confusing. Using flag:parse-getopt wasn't particularly hard but required more code than I expected. I was hoping for something more like the Fish argparse command. Obviously not in every detail but with regard to directly setting variables in the current context according to the parsed options. So that the latter half of this snippet could be omitted:

fn which {|@args|
    var all = $false
    var search = $false
    var arg-specs = [
      [&short=a &arg-optional=$false &arg-required=$false]
      [&short=s &arg-optional=$false &arg-required=$false]
    ]
    var flags args = (
        flag:parse-getopt $args $arg-specs ^
            &stop-after-double-dash=$true &stop-before-non-flag=$true &long-only=$false
    )

    for flag $flags {
        if (eq $flag[spec][short] a) {
            set all = $true
        }
        if (eq $flag[spec][short] s) {
            set search = $true
        }
    }

@xiaq proposed adding a callback for each option that is run when the option has been successfully parsed. @xiaq also stated that "Setting local variables like fish is not feasible in Elvish it would violate lexical scoping." However, I don't see why that is true if the var has already been declared as in the example above (it is true that the Fish argparse does not require declaring each var corresponding to an option before calling argparse). Nonetheless, given the design of Elvish, and existing precedent, it probably makes more sense to use a callback mechanism than magically setting a var that has already been declared. Which leads to a solution along these lines:

fn which {|@args|
    var all = $false
    var search = $false
    var arg-specs = [
      [&short=a &arg-optional=$false &arg-required=$false
       &set={|value| set all = $value }]
      [&short=s &arg-optional=$false &arg-required=$false
       &set={|value| set search = $value }]
    ]
    var flags args = (
        flag:parse-getopt $args $arg-specs ^
            &stop-after-double-dash=$true &stop-before-non-flag=$true &long-only=$false
    )

That eliminates a lot of boilerplate code and is arguably clearer about the intent while still supporting the existing behavior and idioms for handling options.

P.S., Obviously instead of &set there could be a &var option that takes the name of the variable in the current lexical scope to be set with the value of the parsed option; e.g., &var=all for the first arg-spec above. That would eliminate even more boilerplate code. It would, obviously, throw an exception if the var wasn't already declared in the current scope. In fact, that should be detectable as a compilation error since I can't see a good reason to support dynamic var names (e.g., &var=$var-name). The var name should have to be a bare word.

[krader1961]

Ugh! My "P.S." in my opening comment of this issue is obviously bogus since var arg-specs is defining a map, independent of the behavior of flag:parse-getopt. W don't want to special-case the map key &var. I have no idea what I was thinking when I wrote that comment other than a desire to eliminate even more flag:parse-getopt boilerplate code. While it would be nice if we could avoid the need for a lambda callback that captures a variable that isn't possible given the current design of the flag module. And I don't see a good reason to redesign the flag module solely to eliminate a small amount of boilerplate code beyond what can be achieved by adding an arg-spec &set key.