Re: [PATCH] scalar: accept -C and -c options before the subcommand

[Derrick Stolee <stolee@gmail.com>]

On 1/26/2022 9:55 PM, Ævar Arnfjörð Bjarmason wrote:
> 
> 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