Re: [PATCH] blame: add blame.showemail config option

On Mon, Apr 27, 2015 at 2:10 PM, Junio C Hamano [off-list ref] wrote:

Quentin Neill [off-list ref] writes:

quoted

Eric Sunshine [off-list ref] writes:

quoted

Quentin Neill [off-list ref] writes:

quoted

-                       if (opt & OUTPUT_SHOW_EMAIL)
+                       if ((opt & OUTPUT_SHOW_EMAIL) || show_email)

The desired behavior is for a configuration setting to provide a
fallback, and for a command-line option to override the fallback. So,
for instance, if blame.showemail is "true", then --no-show-email
should countermand that. Unfortunately, the way this patch is
implemented, it's impossible to override the setting from the
command-line since show_email==true will always win (when
blame.showemail is "true").

I followed the code for blame.showRoot and blame.blankboundary.

If you truly followed this code pattern, I would expect that there
will not be a separate show_email variable introduced to support
this new configuration variable.  The OUTPUT_SHOW_EMAIL bit in the
opt flag word corresponds to show_root and blank_boundary variables,
so the code to read configuration variable would set that bit in the
opt flag word before the command line parser would kick in.  And the
code that checks "opt & OUTPUT_SHOW_EMAIL" currently does not have
to change at all.

Right. Rather than having a separate global 'show_email' variable and
consulting that variable in parallel with OUTPUT_SHOW_EMAIL throughout
the code, instead set the OUTPUT_SHOW_EMAIL bit in git_blame_config().
To do this, take advantage of the "callback data" argument of
git_config(), which will arrive in git_blame_config() as its 'void
*cb' argument. So, for instance, something like this:

    static int git_blame_config(var, value, void *cb)
    {
        ...
        if (!strcmp(var, "blame.showemail")) {
            if (git_config_bool(var, value)) {
                int *output_options = cb;
               *output_options |= OUTPUT_SHOW_EMAIL;
            }
            return 0;
        }
        ...
    }

    int cmd_blame(...)
    {
        ...
        git_config(git_blame_config, &output_options);
        ...
        parse_options(...);
        ...
    }