Re: [PATCH v8] var: support broken-down idents, signing key, multiple args, and -z

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

"Andrew Pleeter via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Andrew Pleeter <andrewpleeter@gmail.com>
>
> While 'git var' exposes GIT_AUTHOR_IDENT and GIT_COMMITTER_IDENT,
> extracting individual components (name, email, or date) currently
> requires callers to manually parse the composite string. Furthermore,
> there is no way to query the resolved commit signing key through
> 'git var', and the command only accepts a single variable at a time.
>
> Teach 'git var' to expose individual identity components and commit
> signing configuration, and allow querying multiple variables with
> optional NUL-termination:

The huge laundary list (below) strongly tells us that this single
patch is doing too many things at once and it is better done as a
multi-patch series.  Also, some descriptions seem to hint how the
implementation evolved during the development of this patch, which
nobody is interested when they read "git log" output (which is the
ultimate target audience we write our commit log messages for).

I would say this should be split into at least 3 patches.

 (1) Add "-z" output mode.

     To allow reading scripts to unambiguously parse output from
     "git var (-l | <var>)" command, implement a NUL terminated
     output mode, similar to how "git config -l -z" shows list of
     configuration variables and their values.  When showing the
     value of a single variable this only makes difference for
     variable with multiple values, but in the next step in this
     series, we will introduce a mode where multiple variables are
     queried.

 (2) Add (2 <= argc) mode that displays like "var -l" mode on top.

     To allow reading values for multiple variables with a single
     command invocation, teach "var" to take more than one variable,
     and show output the same way as "git var -l [-z]", giving list
     of "var=value" but only for variables requested by the user.

 (3) Add new variables.

     Scripts reading from "git var GIT_AUTHOR_IDENT" needs to parse
     the output if they want to extract only the author name.  

     To allow scripts to easily access broken-out fields of
     GIT_{AUTHOR,COMMITTER}_IDENT, add a GIT_AUTHOR_NAME variable
     and its friends, as well as GIT_SIGNING_KEY.

After you receive a review, you should respond and try to engage in
a dialog with reviewers, before sending a new iteration of a patch.

When your new iteration is different from what reviewer suggested,
without such an exchange beforehand, reviewers cannot tell if that
is merely due to miscommunication, or you had a good reason to do
things differently.  Don't make reviewers feel as if they are
talking to silent machine that takes an earlier iteration of the
patch with their input and spits out a new iteration.

Thanks.

> - Add GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and GIT_AUTHOR_DATE.
> - Add GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL, and GIT_COMMITTER_DATE.
> - Add GIT_SIGNING_KEY to resolve the key that would be used to sign
>   the resulting commit if you were to run 'git commit' right now.
> - Allow passing multiple variable arguments (e.g., 'git var
>   GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL').
> - When a single variable is requested, print its bare value for backward
>   compatibility.
> - When multiple variables are requested, model the output after
>   'git var -l' by printing 'VARIABLE=value' pairs (or 'VARIABLE\nvalue\0'
>   when '-z' is given).
> - Format multi-valued variables in multi-variable mode as repeated
>   'VARIABLE=value' entries (or 'VARIABLE\nvalue\0' with '-z'),
>   eliminating stream ambiguity without extra trailing delimiters.
> - When querying multiple variables, omit any variable that has no value,
>   continue processing remaining variables, and exit with code 1.
> - Support '-z' to terminate variable outputs with NUL bytes.
> - Format 'git var -l -z' using the same convention as 'git config
>   list -z' (newline separating key and value, NUL separating entries).
> - Use parse_options() to strictly require options before arguments.
> - Update Documentation/git-var.adoc and t/t0007-git-var.sh.