dnaeon / clingon

Command-line options parser system for Common Lisp
Other
122 stars 7 forks source link

Searching for options in parent commands #3

Closed alessiostalla closed 1 year ago

alessiostalla commented 1 year ago

https://github.com/dnaeon/clingon/blob/2885c241a45b17dbc3fb1ce6890a1f84f9d4953c/src/command.lisp#L747-L748

This makes options in sub-commands always shadow global ones even if not provided. Is there a reason for that? Would it be possible to change the behavior, or at least make it configurable? I know I can subclass command and write my own getopt, but I'd like a less invasive approach – unless that's how you intended this to work.

Thanks for this great library. If I ever make some money off my hobby projects, I'll make sure to donate some to you.

dnaeon commented 1 year ago

Hey @alessiostalla ,

I've documented this behaviour in the README, which is this paragraph here.

Each command in clingon is associated with a context. The context or environment provides the options and their values with respect to the command itself. This means that a parent command and a sub-command may have exactly the same set of options defined, but they will reside in different contexts. Depending on how you use it, sub-commands may shadow a parent command option, but it also means that a sub-command can refer to an option defined in a global command.

I don't recall the full details of this when I initially developed clingon, but the idea was that I might want to provide a flag/option to a sub-command, which does exactly that -- shadowing a global flag/option.

Usually the solution to this "problem" is quite simple -- if you do want to have same flags/options in both -- global and sub-commands -- you just have to "namespace" the keys of these options.

Consider the following example.

my-app --verbose foo bar --verbose

While the top-level and bar sub-command have the same flag (--verbose in this case), you can define the option keys to use "namespaced" keys, e.g.

   (clingon:make-option :flag
                        :description "verbose flag"
                        :long-name "verbose"
                        :key :bar-cmd/verbose)

Then you can do (clingon:getopt cmd :bar-cmd/verbose), or (clingon:getopt cmd :verbose) depending on which one you need.

Here's an example where I use a similar approach, when I do want to have same the string flags (as in --verbose), but also to exist in sub-commands without colliding with the global commands.

Snippet taken from cl-migratum: https://github.com/dnaeon/cl-migratum/blob/master/src/cli/create.lisp#L63

(defun create/options ()
  "Returns the list of options for the `create' command"
  (list
   (clingon:make-option :enum
                        :description "migration kind"
                        :long-name "kind"
                        :required t
                        :items '(("sql" . :sql)
                                 ("lisp" . :lisp))
                        :key :create-cmd/migration-kind)
   (clingon:make-option :string
                        :description "description of the migration"
                        :long-name "description"
                        :required t
                        :key :create-cmd/migration-description)))

Note how the :key for each command is "namespaced".

Let me know if you have other questions.

Thanks, Marin

alessiostalla commented 1 year ago

I know you can namespace them but it becomes awkward if you have flags that you want to be part of every command, e.g. frob --user alessio create project, frob create --user alessio project, and frob create project --user alessio.

Anyway I can write my getopt with my logic, it's just that the documentation says that shadowing is a possibility ("Depending on how you use it, sub-commands may shadow a parent command option") but the code says shadowing is inevitable.

dnaeon commented 1 year ago

Hey @alessiostalla ,

Thanks for explaining your use case. I think having a descend-if-not-set or similar optional argument might help.

Would you consider sending a PR? I'd be happy to review and merge it.

Thanks!

alessiostalla commented 1 year ago

Yes, I'd like to do that. Just I don't know when I'll be able to do it. In the meantime, I wrote my own getopt for those few options where I need that behavior.

dnaeon commented 1 year ago

Just pushed a commit, which adds a :keep-looking keyword arg to CLINGON:GETOPT. This is a breaking change compared to the previous release, but I think this covers your use case.

dnaeon commented 1 year ago

@alessiostalla , actually the more I think about this one, the less I like that behaviour :)

First of all, if we have multiple sub-commands, which have the same option (e.g. --user), then there's nothing wrong with it. However, I still think that namespacing the option keys (e.g. :cmd1/user, :cmd2/user, :cmd3/user) is the way to go, as it clearly specifies which particular option you want to get.

On the other hand, your use case makes sense as well, and you may not want to namespace your keys, but just use the same option and re-use it anywhere in the command's tree.

For that reason I'll be introducing CLINGON:GETOPT* which will return the value of an option from the first command, which provides the requested option, and the option is set. That should cover your use case as well.

dnaeon commented 1 year ago

BTW, just as a heads up. As part of next release of clingon I will be introducing persistent options, which are options that get persisted across sub-commands. That would allow for an option to be defined on a parent command and be propagated to all sub-commands.