Re: [PATCH] scalar: accept -C and -c options before the subcommand
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Jan 27, 2022, 14:46 UTC
- Message-ID
- <0f8d5d04-e86c-48e2-fea0-32c25c3f9325@gmail.com>
- In-Reply-To
- <220127.86v8y5dgus.gmgdl@evledraar.gmail.com>
On 1/26/2022 9:55 PM, Ævar Arnfjörð Bjarmason wrote:
Show 45 quoted lines
> > On Wed, Jan 26 2022, Johannes Schindelin via GitGitGadget wrote: > >> From: Johannes Schindelin <johannes.schindelin@gmx.de> >> >> The `git` executable has these two very useful options: >> >> -C <directory>: >> switch to the specified directory before performing any actions >> >> -c <key>=<value>: >> temporarily configure this setting for the duration of the >> specified scalar subcommand >> >> With this commit, we teach the `scalar` executable the same trick. >> >> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> >> --- >> scalar: accept -C and -c options >> >> This makes the scalar command a bit more handy by offering the same -c >> <key>=<value> and -C <directory> options as the git command. >> >> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1130%2Fdscho%2Fscalar-c-and-C-v1 >> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1130/dscho/scalar-c-and-C-v1 >> Pull-Request: https://github.com/gitgitgadget/git/pull/1130 > > I think it would help for context to note that this patch had at least 6 > submissions on the ML already as part of early versions of the scalar > series. > > Here's the CL of the iteration that ejected it: > https://lore.kernel.org/git/pull.1005.v7.git.1637158762.gitgitgadget@gmail.com/ > > Where you summarized: > > * The patch that adds support for -c <key>=<value> and -C <directory> was > moved to its own add-on patch series: While it is obvious that those > options are valuable to have, an open question is whether there are other > "pre-command" options in git that would be useful, too, and I would like > to postpone that discussion to that date. > > Having been involved in those discussions I can't remember what the > pre-command options you're referring to there are, but it seems "that > date" is probably upon us.
My understanding was that this was ejected so we could find the right time to lib-ify handle_options() (as Taylor suggested), but we didn't want to do that while Scalar was still in a tentative state (in contrib/ with a plan to move it out after more is implemented).
The biggest benefits of using handle_options() is for other pre-command options such as --exec-path, which I use on a regular basis when testing new functionality.
There are other options in handle_options() that might not be appropriate, or might be incorrect if we just make handle_options() non-static. For example, `scalar --list-cmds=parseopt` wouldn't show the scalar commands and would instead show the git commands.
So my feeling is that we should continue to delay this functionality until Scalar is more stable, perhaps even until after it moves out of contrib/. The need to change handle_options() to work with a new top-level command is novel enough to be worth careful scrutiny, but that effort is only valuable if the Git community is more committed to having Scalar in the tree for the long term.
Thanks, -Stolee