Re: [PATCH 0/8] builtin/config: introduce subcommands

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Mar 06, 2024 at 06:46:33PM -0500, Taylor Blau wrote:
>> On Wed, Mar 06, 2024 at 09:06:08AM -0800, Junio C Hamano wrote:
>> > Patrick Steinhardt <ps@pks.im> writes:
>> > >   - `git config --get-urlmatch` -> `git config get-urlmatch`.
>> >
>> > ... is a Meh to me, personally.  I'd not actively push it
>> > enthusiastically, but I'd passively accept its existence.
>> 
>> I don't have strong feelings about this, but I wonder if `--urlmatch`
>> (or `--url-match`) might be an argument to the "get" mode of this
>> sub-command instead. Something like `git config get --urlmatch` feels
>> much more natural to me than `git config get-urlmatch`.
>
> Agreed. I allude to this somewhere in the patch series that I only see
> this as a first incremental step, and that some of the subcommands
> really should be converted to become options eventually. Specifically:
>
>     - `git config get-urlmatch` -> `git config get --urlmatch`
>
>     - `git config get-regexp` -> `git config get --regexp`
>
>     - `git config get-all` -> `git config get --all`
>  
>     - `git config set-all` -> `git config set --all`
>
>     - `git config unset-all` -> `git config unset --all`
>
> I didn't yet do it as part of this patch series because I didn't want to
> make functional changes at the same time (well, except of course for the
> introduction of subcommands). But if we all agree that this patch series
> is something we want _and_ that we don't want to have an intermediate
> step with the above somewhat weird subcommands then I would be happy to
> pile onto the series.
>
> I wouldn't think that keeping this intermediate step would be too bad
> though. While we would eventually omit the above subcommands, the infra
> to keep them around needs to stay anyway to support old syntax like
> `--get-all`. Thus, we can keep the subcommands themselves almost for
> free even if we eventually decide to hide them and replace them with
> flags.

The "more involved" changes along the lines Taylor suggested take
some thought in designing the end-user experience, so even if you do
not initially plan to support them in the implementation, it would
be better to know how the final command line should look like before
starting, so that we won't surprise users, if anything.

The trickies I anticipate offhand are:

 * The urlmatch thing wants name and URL.  In the new syntax, I
   think the URL part should become an option parameter to the --url
   option.  E.g. a request with the current UI

    $ git config --get-urlmatch SECTION.VARIABLE URL

   would become

    $ git config get --url=URL SECTION.VARIABLE

 * The color thing takes the default value to be used when
   unconfigured.  That should also become an option parameter to the
   --default option.

    $ git config --get-color SECTION.VARIABLE DEFAULT

   would become

    $ git config get --type=color --default=DEFAULT SECTION.VARIABLE

   Similarly for --tty=true/false used in --get-colorbool.

 * Now, these extended ones with options introduce possibilities to
   combine them in ways that weren't possible before.  For example,
   "--get" and "--get-all" are distinct, and there is no way to tell
   "--get-urlmatch" to give all values that would match, but it is
   plausible that we may want to handle

    $ git config get --all --url=URL SECTION.VARIABLE

   to do so.  A more interesting one is the --default=DEFAULT
   option.  What we currently write

    $ git config --get no.such.variable || echo 'default value'

   can become

    $ git config get --default='default value' no.such.variable

HTH.