Subject: Re: [PATCH 1/2] blame: Make --show-name negatable

From: Junio C Hamano <[email-masked]>
Date: Mon, 12 Sep 2022 09:30:19 -0700

Øystein Walle <[email-masked]> writes:

> That was me! :-)
>
> Not long after I sent you a new link with the patch below, but I suppose
> you missed it. My first rough attempt was incomplete, changing the
> behavior from on/auto on to auto/off instead of the intended
> on/auto/off.

Not clear to this bystander what exactly is going on.  Is the patch
Rene relayed is your rough attempt that was incomplete?

> The docs changes look good to me (but should perhaps be squashed into the impl
> patch?). Not sure how many of the negatable long options that exist are
> actually documented as such, but imho. they should be.
>
> There are no added tests, but there are no tests for -f/--show-name in the
> first place and I just didn't get around to writing one...

Thanks.  Let's see if we can have an cleaned-up version with tests,
then.

>
> Øsse
>
> ---
>  builtin/blame.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index a9fe8cf7a6..cbaae91a8f 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -49,6 +49,7 @@ static int blank_boundary;
>  static int incremental;
>  static int xdl_opts;
>  static int abbrev = -1;
> +static int show_name = -1;
>  static int no_whole_file_rename;
>  static int show_progress;
>  static char repeated_meta_color[COLOR_MAXLEN];
> @@ -621,7 +622,8 @@ static void find_alignment(struct blame_scoreboard
> *sb, int *option)
>          if (compute_auto_abbrev)
>              auto_abbrev = update_auto_abbrev(auto_abbrev, suspect);
>          if (strcmp(suspect->path, sb->path))
> -            *option |= OUTPUT_SHOW_NAME;
> +            if (show_name == -1)
> +                *option |= OUTPUT_SHOW_NAME;
>          num = strlen(suspect->path);
>          if (longest_file < num)
>              longest_file = num;
> @@ -867,7 +869,7 @@ int cmd_blame(int argc, const char **argv, const
> char *prefix)
>          OPT_BOOL(0, "show-stats", &show_stats, N_("show work cost
> statistics")),
>          OPT_BOOL(0, "progress", &show_progress, N_("force progress
> reporting")),
>          OPT_BIT(0, "score-debug", &output_option, N_("show output
> score for blame entries"), OUTPUT_SHOW_SCORE),
> -        OPT_BIT('f', "show-name", &output_option, N_("show original
> filename (Default: auto)"), OUTPUT_SHOW_NAME),
> +        OPT_BOOL('f', "show-name", &show_name, N_("show original
> filename (Default: auto)")),
>          OPT_BIT('n', "show-number", &output_option, N_("show original
> linenumber (Default: off)"), OUTPUT_SHOW_NUMBER),
>          OPT_BIT('p', "porcelain", &output_option, N_("show in a
> format designed for machine consumption"), OUTPUT_PORCELAIN),
>          OPT_BIT(0, "line-porcelain", &output_option, N_("show
> porcelain format with per-line commit information"),
> OUTPUT_PORCELAIN|OUTPUT_LINE_PORCELAIN),
> @@ -943,6 +945,9 @@ int cmd_blame(int argc, const char **argv, const
> char *prefix)
>      revs.diffopt.flags.follow_renames = 0;
>      argc = parse_options_end(&ctx);
>
> +    if (show_name == 1)
> +        output_option |= OUTPUT_SHOW_NAME;
> +
>      prepare_repo_settings(the_repository);
>      the_repository->settings.command_requires_full_index = 0;