Does it make sense to show tips on blame?

Does it make sense to show tips on blame?

[Edmundo Carmona Antoranz]

Hi!

I'm having fun with difflame. I added support for an option called
'tips' which would display 1-line summary of a block of added lines
that belong to the same revision, in order to provide context while
reading difflame output without having to run an additional git show
command.

Output is like this (from README.txt, taken from difflame on git
itself, sorry if it's too wide):

diff --git a/fast-import.c b/fast-import.c
index f561ba833..64fe602f0 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2218,13 +2218,17 @@ static uintmax_t do_change_note_fanout(
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2218)
char *fullpath, unsigned int fullpath_len,
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2219)
unsigned char fanout)
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2220) {
-02d0457eb4 (Junio C Hamano 2017-01-10 15:24:26 -0800 2221)     struct
tree_content *t = root->tree;
        405d7f4af fast-import: properly fanout notes when tree is imported
+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2221)      struct
tree_content *t;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2222)      struct
tree_entry *e, leaf;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2223)
unsigned int i, tmp_hex_sha1_len, tmp_fullpath_len;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2224)
uintmax_t num_notes = 0;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2225)
unsigned char sha1[20];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2226)      char
realpath[60];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2227)
        405d7f4af fast-import: properly fanout notes when tree is imported
+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2228)      if (!root->tree)
+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2229)
 load_tree(root);
+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2230)      t = root->tree;
+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2231)
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2232)      for (i
= 0; t && i < t->entry_count; i++) {
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2233)
e = t->entries[i];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2234)
tmp_hex_sha1_len = hex_sha1_len + e->name->str_len;

The tip that was used for revision 405d7f4af on both "blocks" of added
lines can be seen.

The question is if you think it makes sense to add this option to
git-blame itself.

Something like:
git blame --tips README.txt

Thanks in advance

Re: Does it make sense to show tips on blame?

[Junio C Hamano]

Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:

> Output is like this (from README.txt, taken from difflame on git
> itself, sorry if it's too wide):

It is not just too wide but it is line-wrapped and cannot see what
you wanted to say out of it.

What does the word "tip" mean in this context?  The word is often
used to mean the commits directly pointed at by branches (i.e. the
tip of history), but I do not think that is what you are interested
in showing in this butchered "diff" output.

> 2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2227)
>         405d7f4af fast-import: properly fanout notes when tree is imported
> +405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2228)      if (!root->tree)
> +405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2229)
>  load_tree(root);
> ...

Do you mean the one-line summary for the commit?  The commits that
are shown will not be at the tip most of the time (the "+" ones may
be if you happen to be running "git show" on the tip of a branch,
but that is minority if you also want to do "git log -p"), so I am
not sure it makes sense to call them "tips" in the above output.

If I were doing the above, I would probably do them as footnotes to
keep the body of the patch text (slightly) more readable.  E.g.

	@@ -l,k +m,n @@
         2a113aee9b  
        +405d7f4af6   if (!root-tree)    
        +405d7f4af6       load_tree(root)
         ...
        #2a113aee9b "fast-import: Proper notes tree manipulation", 2009-12-07
        #405d7f4af6 "fast-import: properly fanout notes when tree is imported", 2016-12-21

> The question is if you think it makes sense to add this option to
> git-blame itself.
>
> Something like:
> git blame --tips README.txt

I do not think I would use it personally, as it would make it hard
to pipe the output of "git blame" to less and then /search things,
as extra cruft added by the option will get in the way.  IOW, I do
not think we want it for human-oriented output.

On the other hand, when output from "blame" is used by scripts (or
GUI frontends), having the one-line summary would be very useful.
But I think the authors of "blame" already thought about that
usecase and included the "summary" field in the "--porcelain" output
that are meant to be consumed by scripts, so...

Re: Does it make sense to show tips on blame?

[Edmundo Carmona Antoranz]

On Sun, Jan 22, 2017 at 4:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:
>
> What does the word "tip" mean in this context?  The word is often
> used to mean the commits directly pointed at by branches (i.e. the
> tip of history), but I do not think that is what you are interested
> in showing in this butchered "diff" output.

>
> Do you mean the one-line summary for the commit?  The commits that
> are shown will not be at the tip most of the time (the "+" ones may
> be if you happen to be running "git show" on the tip of a branch,
> but that is minority if you also want to do "git log -p"), so I am
> not sure it makes sense to call them "tips" in the above output.
>

Yeah, it would be the 1-line summary for the revision that is related
to the lines that
will be printed right after the summary, as a way to provide
additional context about
those lines without having to resort to an additional git show
--summary command.
Perhaps a name more appropriate name than "tips" in the context of git would be
better suited to avoid confusion.

> If I were doing the above, I would probably do them as footnotes to
> keep the body of the patch text (slightly) more readable.  E.g.
>
>         @@ -l,k +m,n @@
>          2a113aee9b
>         +405d7f4af6   if (!root-tree)
>         +405d7f4af6       load_tree(root)
>          ...
>         #2a113aee9b "fast-import: Proper notes tree manipulation", 2009-12-07
>         #405d7f4af6 "fast-import: properly fanout notes when tree is imported", 2016-12-21
>

Now, this is a different topic altogether (difflame related) and I
think it was raised from the fact that I
included output from difflame (which starts from diff output) as my
example. When talking about
blame it would be no-patches, just plain blame output writing the
1-line summary for blocks of lines
related to the same revision. I already sent a patch today with the
changes needed to see what I mean in action.
Could you give it a test drive? Consider it's a rough draft (and I
also raise the question of "aggregating" output
on the mail thread)

Something like this, hope this doesn't get butchered (at least not
that hard, i'm truncating the lines):

        3d426842: README.txt
3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600   1) difflame
3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600   2)
3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600   3) Copyright 2017
3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600   4) Released unde
3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600   5)
       0660083e: Add params to blame and diff, adjust README.txt
0660083e (Edmundo Carmona Antoranz 2017-01-18 20:51:12 -0600   6) Show the outp
0660083e (Edmundo Carmona Antoranz 2017-01-18 20:51:12 -0600   7)
       c869edc9: README.txt - stick (or at least, at least try) to 80 columns
c869edc9 (Edmundo Carmona Antoranz 2017-01-20 23:39:51 -0600   8) Example outp
c869edc9 (Edmundo Carmona Antoranz 2017-01-20 23:39:51 -0600   9) params to cha

>> The question is if you think it makes sense to add this option to
>> git-blame itself.
>>
>> Something like:
>> git blame --tips README.txt
>
> I do not think I would use it personally, as it would make it hard
> to pipe the output of "git blame" to less and then /search things,
> as extra cruft added by the option will get in the way.  IOW, I do
> not think we want it for human-oriented output.

Doing a search through less, it would make sense to leave the 1-line summary out
but when looking at blame output through OSI layer 8, I'd definitely
like to get that 1-line summary "as an option" every so often (more
often than not
in my case, probably).