Add Commit Summary in blame?

Add Commit Summary in blame?

[Aleks Todorov]

Hi folks,

Would people be interested in seeing a patch that adds an option to
see the commit message summary line in the git blame output? I imagine
I'm not the first person to come up with this, is there a reason why
it wasn't already implemented? The porcelain already has it, so the
change is trivial.

Thanks

Re: Add Commit Summary in blame?

[Junio C Hamano]

Aleks Todorov <aleks.todorov.1337@gmail.com> writes:

> Would people be interested in seeing a patch that adds an option to
> see the commit message summary line in the git blame output? I imagine
> I'm not the first person to come up with this, is there a reason why
> it wasn't already implemented? The porcelain already has it, so the
> change is trivial.

I personally do not.  The blame output on the left-hand-side is
already wide enough to be even annoying.

A patch that might interest me personally would be one that takes
the --format option (copy from the log family plus add some that are
only useful in "git blame", like "line number") to allow customizing
the left-hand-side of the output, and allows us to reimplement ugly
hacks^W^W ad-hoc options (like -l, -t, --date, -f, -n, -s, -e) in
terms of that.  With such a feature, of course your "commit summary"
will come almost for free in the form of --format=%s I would think.

A change that adds even more ad-hoc options is probably not welcome,
I would have to say.

Re: Add Commit Summary in blame?

[Aleks Todorov]

I know it's been a while, but thank you for your reply! I was quite interested
in this format option and have wanted to implement it since. I finally have
some free time.

I've written a barebones implementation just to play around with the code and
see it work.

Before I continue, I wanted to know, should we re-implement the git log
implementation or refactor it to make it reusable? I lean into the latter,
however if the implementation is shared, that means we need a way to sanitize
user input to ensure they are not using flags which are not allowed, and we
will also need to pollute the implementation with length specifiers everywhere
since each field in the blame output needs to be width-aligned.

So far, this is what I've been able to output:

  > ./git-blame builtin/blame.c -F '%h fname (%an %ad line) '

  d48be35ca6f fname (Elijah Newren Tue Mar 21 06:26:07 2023 +0000
line) #include "write-or-die.h"
  cee7f245dca fname (Junio C Hamano Thu Oct 19 16:00:04 2006 -0700 line)
  ce41720cad7 fname (Alex Henrie Thu Apr 2 15:26:56 2015 -0600 line)
static char blame_usage[] = N_("git blame [<options>] [<rev-opts>]
[<rev>] [--] <file>");
  df8738116f7 fname (Ævar Arnfjörð Bjarmason Thu Oct 13 17:39:20 2022
+0200 line) static char annotate_usage[] = N_("git annotate
[<options>] [<rev-opts>] [<rev>] [--] <file>");
  5817da01434 fname (Pierre Habouzit Tue Jul 8 15:19:34 2008 +0200 line)
  5817da01434 fname (Pierre Habouzit Tue Jul 8 15:19:34 2008 +0200
line) static const char *blame_opt_usage[] = {
  5817da01434 fname (Pierre Habouzit Tue Jul 8 15:19:34 2008 +0200
line)  blame_usage,
  5817da01434 fname (Pierre Habouzit Tue Jul 8 15:19:34 2008 +0200 line)  "",
  9c9b4f2f8b7 fname (Alex Henrie Tue Jan 13 00:44:47 2015 -0700 line)
   N_("<rev-opts> are documented in git-rev-list(1)"),
  5817da01434 fname (Pierre Habouzit Tue Jul 8 15:19:34 2008 +0200 line)  NULL
  5817da01434 fname (Pierre Habouzit Tue Jul 8 15:19:34 2008 +0200 line) };
  cee7f245dca fname (Junio C Hamano Thu Oct 19 16:00:04 2006 -0700 line)

This is the code I have, reusing the log impl (avoided formatting existing code
for brevity):

  diff --git a/builtin/blame.c b/builtin/blame.c
  index c470654c7e..7a7be7b36d 100644
  --- a/builtin/blame.c
  +++ b/builtin/blame.c
  @@ -66,6 +66,7 @@ static int xdl_opts;
   static int abbrev = -1;
   static int no_whole_file_rename;
   static int show_progress;
  +static char *user_format = NULL;
   static char repeated_meta_color[COLOR_MAXLEN];
   static int coloring_mode;
   static struct string_list ignore_revs_file_list = STRING_LIST_INIT_DUP;
  @@ -506,6 +507,17 @@ static void emit_other(struct blame_scoreboard
*sb, struct blame_entry *ent, int
                          putchar('?');
                  }

  +               if (user_format) {
  +                       struct strbuf sb = STRBUF_INIT;
  +                       struct pretty_print_context context = {
  +                               .fmt = CMIT_FMT_USERFORMAT,
  +                               .abbrev = abbrev,
  +                       };
  +                       repo_format_commit_message(the_repository,
  +                                                  suspect->commit,
user_format,
  +                                                  &sb, &context);
  +                       printf("%s", sb.buf);
  +               } else {
                  printf("%.*s", (int)(length < GIT_MAX_HEXSZ ? length
: GIT_MAX_HEXSZ), hex);
                  if (opt & OUTPUT_ANNOTATE_COMPAT) {
                          const char *name;
  @@ -546,6 +558,7 @@ static void emit_other(struct blame_scoreboard
*sb, struct blame_entry *ent, int
                          printf(" %*d) ",
                                 max_digits, ent->lno + 1 + cnt);
                  }
  +               }
                  if (reset)
                          fputs(reset, stdout);
                  do {
  @@ -905,6 +918,7 @@ int cmd_blame(int argc,
                  OPT_BIT('t', NULL, &output_option, N_("show raw
timestamp (Default: off)"), OUTPUT_RAW_TIMESTAMP),
                  OPT_BIT('l', NULL, &output_option, N_("show long
commit SHA1 (Default: off)"), OUTPUT_LONG_OBJECT_NAME),
                  OPT_BIT('s', NULL, &output_option, N_("suppress
author name and timestamp (Default: off)"), OUTPUT_NO_AUTHOR),
  +               OPT_STRING('F', "format", &user_format,
N_("format"), N_("print blame entries in the given <format>")),
                  OPT_BIT('e', "show-email", &output_option, N_("show
author email instead of name (Default: off)"), OUTPUT_SHOW_EMAIL),
                  OPT_BIT('w', NULL, &xdl_opts, N_("ignore whitespace
differences"), XDF_IGNORE_WHITESPACE),
                  OPT_STRING_LIST(0, "ignore-rev", &ignore_rev_list,
N_("rev"), N_("ignore <rev> when blaming")),

Re: Add Commit Summary in blame?

[Junio C Hamano]

Aleks Todorov <aleks.todorov.1337@gmail.com> writes:

> ...
> however if the implementation is shared, that means we need a way to sanitize
> user input to ensure they are not using flags which are not allowed, and we
> will also need to pollute the implementation with length specifiers everywhere
> since each field in the blame output needs to be width-aligned.

I was imagining that the aligning is responsibility of the end-users
who give whatever random format strings to the command, if you are
to reuse the existing helper functions out of the implementation of
the "git log" family of commands, as %</%> and its friends would be
available for free.  I admit that it has been forever since I looked
at the code paths that implement wrapping and padding specifiers
last time, and I do not know offhand how cleanly they are written
and how reusable they are, though.

If you are doing this to feed some GUI implementation, however, I
suspect that a more productive way would be to make the GUI tool
read from the "--incremental" format.  But as I do not know your
motivation...