discussion: how to convert parseopt style to cligen

[c-blake]

discussion: how to convert parseopt style to cligen

[c-blake]

At a high level, I think converting explicit control flow parsing that has logic too complex to just "wrap it in a proc" and pass that proc to cligen is probably never going to be very "ergonomic", at least the conceptual level. It's like converting procedural language to a declarative one..the concerns are just very distinct and that bubbles up in a bunch of places.

At a more fine-grained level, just relating to pass, conf, and maybe some new environment variable source, have you seen mergeParams()? That has a very ergonomic way to slurp a config file, environment variable, and command line, merge them together and have that be what gets parsed. That feature could use test/example code that uses parsecfg, too. And it would not be too hard (just some namespacing issues) to have the parser-dispatcher directly callable, though I'm not sure that's needed with mergeParams() and a resolution to just #49 .

At the level of the exact question...Yeesh. nimfind does all this import ../compiler stuff and re-uses all this complex code specifically oriented around the control flow of the nim compiler's command-line language. Heck, it might even support .nims and nim.cfg and all that. While I applaud the, erm, "unbridled ambition", it seems like an end-point in conversion pain/extreme mismatch between the old explicit and proposed new generated way. I mean, maybe shoot-for-the sky was your point.

Something like nimble would be a lot simpler. Last I looked at it, nimble code base was like 1/3 command parsing that could be collapsed to maybe 10% just declarative help={} sections with cligen as well as making nimble more library like, though I doubt dom96 would want the bootstrapping issue of depending on something not in the stdlib. I don't use nimble much. { Personally, I dislike all the pip/cargo/nimble language package managers. I already have an OS package manager. At a minimum, every such lang-packager should provide N scripts to convert to .rpm, .deb, .ebuild, ... so OS package guys could just run something to add easily any given lang package to their repos, but there's probably a lot of feature mismatching, too. Oh well. }

Anyway, I'd be happy to contribute cligen and parseopt3 to the stdlib and be the "code owner", but I met with a lot of resistance getting a minor POSIX feature into the stdlib parseopt. See nim-lang/Nim#7297 . So, I doubt it would be welcome. The testing structure would also need an overhaul to run out of the continuous integration stuff at the minimum. I just update Nim from devel every week or so and run test.sh when I do.

[c-blake]

Also, Nim has as many layers of scope as you want, not just global and local. If there is a "global"/outer scope vars way of doing it, I'd say just do that.

[c-blake]

On this topic of converting imperative/control flow-oriented parser-interpreters to the declarative style of cligen (where cligen just re-uses the existing declarative syntax for declaring a Nim proc that any Nim programmer already knows), it bears mentioning the new setByParse (see RELEASE_NOTES.md or docs for cligen.dispatchGen).

You can't quite do everything with setByParse. For convenience it is a Table rather than a seq. So, for example, it would be hard to recreate a CLI language that "applied" certain optional behaviors only to command arguments after a certain option or argument (e.g. a compiler that took warning or path flags and applied them to source files listed after the flag).

These kinds of command languages start to get quite distant from the syntax/semantics of a Nim proc call, though they certainly do exist in the wild out there. You haven't really been "discussing back" since you opened this. You could probably convince me that a seq[tuple[paramName, passedValue: string]] might be a better type for setByParse. I could probably export an in so terse constructs like "foo" in setByParse still worked.

The broader problem is, of course, that CLI authors end up re-implementing the parsing/conversion as they try to reconstruct certain behaviors. It's not clear to me how "gradually more painful" that can/should be. If we really got the Variant value type stuff to work we might be able to have setByParse be a seq[tuple[string, Variant]]. That's probably as close as we could get.

[c-blake]

Also, in terms of the general pattern of "parsing what you know locally and deferring the rest for later processing", it might be possible to assist some. There could be a new mode where rather than erroring out with an "unknown option", we add to some seq the CLI author passes us. The user could then use the new mergeParams() to incorporate those unparsed results into some argument for the "next stage". Or that tuple[string, Variant] in my last comment could be could be tuple[string, bool, Variant] where the bool indicates "already handled/was able to parse". Then this very same seq serves both purposes, but to use it to pass on you might need a one-liner to filter out the true/false tuples to collect a new seq[string] (or cligen could provide a helper proc to do that). So, then the name to parsed value in some sequence stuff is all done automagically, but if the command language depends on order that one aspect can be handled.

This might sound great, but there are probably complexities lurking. E.g., parseopt3 needs to know if a parameter takes a value or is a boolean flag -- just to define the syntax. If you have short flags -abc and you understand -ab as bools but not -c then what should happen? There is no "long version of -c" since you don't know the parameter name. I guess you could "promote" it to its own element of the collected seq and add "-c". I feel like there may be other "bool gotchas" lurking. It might only "really" work in requireSeparator mode.

[c-blake]

Alternatively, setByParse could be a seq[tuple[paramName, unparsedVal: string; handledAlready: bool]], and we could add a parameter like tolerateUnknown=false to dispatchGen to make it forge onward rather than raising a ParseError exception. Then a CLI-only author (who might also want to consider order of issued arguments) would have all they might need. If they wanted to "pass on unrecognized stuff", they could just loop over and collect a seq[string] with --paramName=unparsedVal. It is probably just impossible to handle "mixed short options" like -abcd where a and b are understood as bool by first pass parser while c and d are not. I don't see how even a parsopt3 approach could resolve that. There's no way to know if it would mean --cd or -c -d until later, but the use case would probably have few users doing that and in tolerateUnknown=true mode we could just issue a warning message.

Whatever it is that is supposed to receive unknown options needs to be able to accept some seq[string] as a command line, but that may well be fundamental to that use case anyhow. I don't think the Nim stdlib lets you (as some command parsing libraries do) "permute" or otherwise modify what os.comandLineParams() will give to other callers, but a lot fo things will just take a seq[string] parameter defaulting to os.comandLineParams().

About the only thing "hidden" from the CLI-only author then would be the source of seq[string] elements "pre-mergeParams()". Honestly, though, someone treating flags from config vs flags from environ vs flags from the user cmd more differently than priority/ordering is probably being hostile to command end users. And mergeParams() gives them control over ordering those batches.

So, I am thinking a setByParse: seq[tuple[paramName, unparsedVal: string; handledAlready: bool]] with tolerateUnknown=false covers all the cases that are desirable to cover. What do you think? There are quite a few features piling up in this release and so I'd like to nail down the setByParse interface soon, if possible. Thanks for any input.

[c-blake]

As I sit to code this up, I am leaning toward this:

type
  ClStatus* = enum clBadKey,                        ## Unknown long key
                   clBadVal,                        ## Unparsable value
                   clNonOption,                     ## Unexpected non-option
                   clMissingMandatory,              ## >=1 mandatory missing
                   clHelpOnly, clVersionOnly, clOk  ## Non-errors
  ClParse* = tuple[paramName,         ##Param name/long opt key
                   unparsedVal,       ##Unparsed value("" for missing mandatory
                   message: string;   ##A decent default error message
                   status: ClStatus]  ##Parse status for this parameter
dispatchGen(..., setByParse: seq[ClParse]=addr myVar, ...) =

The only condition where the parser would currently raise without an associated command parameter is if some mandatory/required parameters are missing. So, we provide those with a dummy value since they're missing. If and only if the CLI author passes some non-nil setByParse pointer, then the parser will return (or maybe press onward and return just before dispatching?) instead of raise.

[c-blake]

I've committed and pushed a test/DetectSet2.nim to give a flavor for how this could look. We could also add a parseOnly parameter to dispatchGen to make the generated parser-dispatcher always return before calling even on success if that were wanted for some reason.

Perhaps none of this is all quite so clean as a complete parsing/dispatching separation where fancy CLI authors call the parser 3 times instead of using mergeParams, but I don't see how such a separated system could be nice to use for more vanilla "just define some new argParse/argHelps and go CLI authors unless there was also an clean "Any" type. Maybe we could have a seq[tuple[param, pointer]] and force fancy CLI authors to cast[](pointer) as needed, but to auto-generate any needed such casts or something like that? I'm not sure. I'm also not sure what payoff in such separation there would be besides porting various not so great existing CLI behavior to the very different, more declarative cligen model. It might be better to force such ports to drop less uniform behavior (like treating config file options differently from the command line and other sorts of irregularities) that pop up easily when the interpreter logic is all strewn about. Also, manually doing the cast[](pointer)s would be only a little less work as manually calling argParse if they need a real typed value, though admittedly that comparison only makes sense if they are defining their own argParse on new types. So, we might also be able to do a closed Variant and only store those values in a seq and require the argParse call otherwise, only easing the pain for standard out of the box supported types.

I am sure that it would be easy to finish off the arc of this work just changing raise's to append to setByP and maybe-raise/maybe return/maybe call the wrapped proc and that this would be all backward compatible with the easy to use current default modes and provide a way to collect "unhandled long options" to pass through to some other seq[string] consumer. That seems probably enough. A fancy CLI author can always manually call argParse on a string, after all, and this setByParse is fundamentally a very manual mode. I just probably won't finish this today and thought I'd see if you have some early feedback.

[c-blake]

Well, you should now be able to do something sort of like collect unrecognized parameters into a new seq[string] after a dispatchFoo to pass onward to some other part of the program. See the new test/DetectSet2.nim which should probably be renamed. I doubt this is sufficient to fully port all of nimfinds behavior, but this idea of skipping that which is unrecognized and doing something else with it definitely arises in other contexts.

[c-blake]

I think, with all my recent changes, that if we added a parseOnly=false flag to generated dispatchers then backwardly compatible ambitious CLI authors could, in theory, resurrect almost any behavior from a more classical interpretive parser. They could dispatchGen(foo, setByParse=addr fooParse), then dispatchFoo(cmd, parseOnly=true) (potentially multiple times with adjusted cmds), collect whatever out of fooParse and do whatever. I can't say that it would be pleasant, efficient, or well-advised programming or worth anyone's time to do or reasonable as PRs to some existing tool, though. Those are all much more subjective questions.

[timotheecour]

(sorry, not ignoring this, it's on my TODO list to get back to this issue)

[c-blake]

Well, I'd just like to finalize this setByParse thing and punch a new release because there's an awful lot that's happened since the last one.

It seems from our other threads that we may want two kinds of clOk - one for option keys and one for positional arguments. Right now, you can glean that from/parse that out of my stylized "positional $1" spelling of the option key in ClParse.paramName.

It would maybe be cleaner to do a clOkOption, clOkPositional, clOk = {clOkOption,clOkPositional}. No need for a ClParse.positionalNumber, though. They know that programmatically just from iteration order over the seq.

[c-blake]

About the only thing I can think missing from this parse everything we can into a setByParse mode is that errors from argParse get dumped to stderr instead of maybe stuffed into an ArgCvtParams variable to be copied into setByParse.

In some ways this expert mode is also interesting because rather than "failing fast" you could do a complete parse of the command and print out all errors, not just the first one. setByParse is basically an AST but the simplified context of seq[string] -> seq[assignments] parsing makes it more like an array than a tree. So, one thing you could do is ignore all errors and build up seq[string]s of both ignored and non-ignored. Still, I think few would want to use this unless they absolutely had to.

Anyway, except for the error printing, the only thing preventing porting any CLI to this framework I can think of would be if there were no way to pass a synthesized seq[string] downstream to a next stage. In that case the downstream would also probably have to "ignore unrecognized constructs" as well as the upstream and both would just parse the whole command. That's an awful lot of coordination/cross-concerning assumption about which stage does what with which arguments. It's not impossible to imagine, but I don't think supporting that level of interplay is easy.

[c-blake]

Well, I renamed test/DetectSet2.nim to test/ParseOnly.nim. Between that and mergeParams(), I think you might be able to port most things over to cligen if you had to. I'm not sure of the ease or wisdom of that. E.g., I kinda doubt the Nim core guys would want to depend on cligen for something like nimfind. But give that test file a look. I'd like to get a release stamped out soon as there is a slew of new stuff. (And here I thought a few weeks ago that spell correcting long option keys was going to feature prominently.) If we think there are going to be major changes to the parseOnly/setByParse then I should at least mark those as very experimental.

[c-blake]

This seems about as covered as I can see making it - besides a known-to-cligen-only Variant or ptr-oriented parsed value feature. I am really unsure such a parsed value can be made any more ergonomic than setByParse users just doing their own string->type conversion, though.