Re: [PATCH v2 07/13] builtin/config: introduce "get" subcommand

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 3650 bytes --]

On Tue, Mar 12, 2024 at 11:11:07PM -0400, Eric Sunshine wrote:
> On Mon, Mar 11, 2024 at 7:20 PM Patrick Steinhardt <ps@pks.im> wrote:
> > Introduce a new "get" subcommand to git-config(1). Please refer to
> > preceding commits regarding the motivation behind this change.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> > @@ -80,6 +76,12 @@ COMMANDS
> > +get::
> > +       Get value for one or more config options. Values can be filtered by
> > +       regexes and URLs.Returns error code 1 if the key was not found and the
> > +       last value if multiple key values were found. If `--all` is set, then
> > +       all values will be shown.
> 
> s/URLs.Returns/URLs. Returns/
> 
> It's not a new problem with this description (since you're mostly just
> relocating existing text), but I find the discussion of what is
> returned quite confusing and difficult to interpret. Breaking it down
> into simpler sentences might help:
> 
>     Emits the value of the specified key. If key is present
>     multiple times in the configuration, emits the last
>     value. If `--all` is specified, emits all values
>     associated with key. Returns error code 1 if key
>     is not present.
> 
> But, doing so may be outside the scope of this patch series and can be
> tackled at a later date (or not at all).

I like this description, so let's just go with it.

> > @@ -93,22 +95,16 @@ OPTIONS
> > +--all::
> > +       With "get", Return all values for a multi-valued key.
> 
> s/Return/return/
> s/"get"/`get`/
> 
> > +---regexp::
> > +       With "get", interpret the name as a regular expression. Regular
> > +       expression matching is currently case-sensitive and done against a
> > +       canonicalized version of the key in which section and variable names
> > +       are lowercased, but subsection names are not.
> 
> s/"get"/`get`/
> 
> > @@ -286,7 +271,7 @@ Valid `<type>`'s include:
> >  --default <value>::
> > -  When using `--get`, and the requested variable is not found, behave as if
> > +  When using `get`, and the requested variable is not found, behave as if
> >    <value> were the value assigned to the that variable.
> 
> Not a fault of this patch (and need not be fixed by this series): "to
> the that" should be either "to the" or "to that".

This was fixed via 86f9ce7dd6 (docs: fix typo in git-config `--default`,
2024-03-16).

> > @@ -506,25 +509,25 @@ you have to provide a regex matching the value of exactly one line.
> >  To query the value for a given key, do
> >
> >  ------------
> > -% git config --get core.filemode
> > +% git config get core.filemode
> >  ------------
> >
> >  or
> >
> >  ------------
> > -% git config core.filemode
> > +% git config get core.filemode
> >  ------------
> 
> Meh. We only need to retain one of these examples now, not both, right?

Oh, of course.

Patrick

> > diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> > @@ -17,9 +17,15 @@ do
> >  case "$mode" in
> >  legacy)
> >         mode_prefix="--"
> > +       mode_get=""
> > +       mode_get_all="--get-all"
> > +       mode_get_regexp="--get-regexp"
> >         ;;
> >  subcommands)
> >         mode_prefix=""
> > +       mode_get="get"
> > +       mode_get_all="get --all"
> > +       mode_get_regexp="get --regexp --all --show-names"
> >         ;;
> >  *)
> >         echo "unknown mode $mode" >&2
> 
> The variables added by this patch to the `case` arms invalidate the
> suggested simplification in my review of [6/13].

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]