{"thread":{"id":"44927","subject":"Does it make sense to show tips on blame?","startedAt":"2017-01-21T05:23:14Z","lastAt":"2017-01-22T22:59:18Z","messageCount":3,"participants":["Edmundo Carmona Antoranz","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"309883","messageId":"CAOc6etZ9QeJAf1CKh9Eraigwy45yR6eO-7Th+bZRnTa71BoPew@mail.gmail.com","threadId":"44927","inReplyTo":null,"subject":"Does it make sense to show tips on blame?","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2017-01-21T05:23:04Z","receivedAt":"2017-01-21T05:23:14Z","isPatch":false,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"Hi!\n\nI'm having fun with difflame. I added support for an option called\n'tips' which would display 1-line summary of a block of added lines\nthat belong to the same revision, in order to provide context while\nreading difflame output without having to run an additional git show\ncommand.\n\nOutput is like this (from README.txt, taken from difflame on git\nitself, sorry if it's too wide):\n\ndiff --git a/fast-import.c b/fast-import.c\nindex f561ba833..64fe602f0 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2218,13 +2218,17 @@ static uintmax_t do_change_note_fanout(\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2218)\nchar *fullpath, unsigned int fullpath_len,\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2219)\nunsigned char fanout)\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2220) {\n-02d0457eb4 (Junio C Hamano 2017-01-10 15:24:26 -0800 2221)     struct\ntree_content *t = root->tree;\n        405d7f4af fast-import: properly fanout notes when tree is imported\n+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2221)      struct\ntree_content *t;\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2222)      struct\ntree_entry *e, leaf;\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2223)\nunsigned int i, tmp_hex_sha1_len, tmp_fullpath_len;\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2224)\nuintmax_t num_notes = 0;\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2225)\nunsigned char sha1[20];\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2226)      char\nrealpath[60];\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2227)\n        405d7f4af fast-import: properly fanout notes when tree is imported\n+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2228)      if (!root->tree)\n+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2229)\n load_tree(root);\n+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2230)      t = root->tree;\n+405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2231)\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2232)      for (i\n= 0; t && i < t->entry_count; i++) {\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2233)\ne = t->entries[i];\n2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2234)\ntmp_hex_sha1_len = hex_sha1_len + e->name->str_len;\n\nThe tip that was used for revision 405d7f4af on both \"blocks\" of added\nlines can be seen.\n\nThe question is if you think it makes sense to add this option to\ngit-blame itself.\n\nSomething like:\ngit blame --tips README.txt\n\nThanks in advance\n"},{"id":"309949","messageId":"xmqqefzuvijl.fsf@gitster.mtv.corp.google.com","threadId":"44927","inReplyTo":"CAOc6etZ9QeJAf1CKh9Eraigwy45yR6eO-7Th+bZRnTa71BoPew@mail.gmail.com","subject":"Re: Does it make sense to show tips on blame?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-22T22:25:34Z","receivedAt":"2017-01-22T22:25:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:\n\n> Output is like this (from README.txt, taken from difflame on git\n> itself, sorry if it's too wide):\n\nIt is not just too wide but it is line-wrapped and cannot see what\nyou wanted to say out of it.\n\nWhat does the word \"tip\" mean in this context?  The word is often\nused to mean the commits directly pointed at by branches (i.e. the\ntip of history), but I do not think that is what you are interested\nin showing in this butchered \"diff\" output.\n\n> 2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2227)\n>         405d7f4af fast-import: properly fanout notes when tree is imported\n> +405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2228)      if (!root->tree)\n> +405d7f4af6 (Mike Hommey   2016-12-21 06:04:48 +0900 2229)\n>  load_tree(root);\n> ...\n\nDo you mean the one-line summary for the commit?  The commits that\nare shown will not be at the tip most of the time (the \"+\" ones may\nbe if you happen to be running \"git show\" on the tip of a branch,\nbut that is minority if you also want to do \"git log -p\"), so I am\nnot sure it makes sense to call them \"tips\" in the above output.\n\nIf I were doing the above, I would probably do them as footnotes to\nkeep the body of the patch text (slightly) more readable.  E.g.\n\n\t@@ -l,k +m,n @@\n         2a113aee9b  \n        +405d7f4af6   if (!root-tree)    \n        +405d7f4af6       load_tree(root)\n         ...\n        #2a113aee9b \"fast-import: Proper notes tree manipulation\", 2009-12-07\n        #405d7f4af6 \"fast-import: properly fanout notes when tree is imported\", 2016-12-21\n\n> The question is if you think it makes sense to add this option to\n> git-blame itself.\n>\n> Something like:\n> git blame --tips README.txt\n\nI do not think I would use it personally, as it would make it hard\nto pipe the output of \"git blame\" to less and then /search things,\nas extra cruft added by the option will get in the way.  IOW, I do\nnot think we want it for human-oriented output.\n\nOn the other hand, when output from \"blame\" is used by scripts (or\nGUI frontends), having the one-line summary would be very useful.\nBut I think the authors of \"blame\" already thought about that\nusecase and included the \"summary\" field in the \"--porcelain\" output\nthat are meant to be consumed by scripts, so...\n"},{"id":"309962","messageId":"CAOc6etbCLju8nUcU5SMOJwzAcjk_LM041Tak897XrXUpFWtCiA@mail.gmail.com","threadId":"44927","inReplyTo":"xmqqefzuvijl.fsf@gitster.mtv.corp.google.com","subject":"Re: Does it make sense to show tips on blame?","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2017-01-22T22:59:12Z","receivedAt":"2017-01-22T22:59:18Z","isPatch":false,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Sun, Jan 22, 2017 at 4:25 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:\n>\n> What does the word \"tip\" mean in this context?  The word is often\n> used to mean the commits directly pointed at by branches (i.e. the\n> tip of history), but I do not think that is what you are interested\n> in showing in this butchered \"diff\" output.\n\n>\n> Do you mean the one-line summary for the commit?  The commits that\n> are shown will not be at the tip most of the time (the \"+\" ones may\n> be if you happen to be running \"git show\" on the tip of a branch,\n> but that is minority if you also want to do \"git log -p\"), so I am\n> not sure it makes sense to call them \"tips\" in the above output.\n>\n\nYeah, it would be the 1-line summary for the revision that is related\nto the lines that\nwill be printed right after the summary, as a way to provide\nadditional context about\nthose lines without having to resort to an additional git show\n--summary command.\nPerhaps a name more appropriate name than \"tips\" in the context of git would be\nbetter suited to avoid confusion.\n\n> If I were doing the above, I would probably do them as footnotes to\n> keep the body of the patch text (slightly) more readable.  E.g.\n>\n>         @@ -l,k +m,n @@\n>          2a113aee9b\n>         +405d7f4af6   if (!root-tree)\n>         +405d7f4af6       load_tree(root)\n>          ...\n>         #2a113aee9b \"fast-import: Proper notes tree manipulation\", 2009-12-07\n>         #405d7f4af6 \"fast-import: properly fanout notes when tree is imported\", 2016-12-21\n>\n\nNow, this is a different topic altogether (difflame related) and I\nthink it was raised from the fact that I\nincluded output from difflame (which starts from diff output) as my\nexample. When talking about\nblame it would be no-patches, just plain blame output writing the\n1-line summary for blocks of lines\nrelated to the same revision. I already sent a patch today with the\nchanges needed to see what I mean in action.\nCould you give it a test drive? Consider it's a rough draft (and I\nalso raise the question of \"aggregating\" output\non the mail thread)\n\nSomething like this, hope this doesn't get butchered (at least not\nthat hard, i'm truncating the lines):\n\n        3d426842: README.txt\n3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600   1) difflame\n3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600   2)\n3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600   3) Copyright 2017\n3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600   4) Released unde\n3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600   5)\n       0660083e: Add params to blame and diff, adjust README.txt\n0660083e (Edmundo Carmona Antoranz 2017-01-18 20:51:12 -0600   6) Show the outp\n0660083e (Edmundo Carmona Antoranz 2017-01-18 20:51:12 -0600   7)\n       c869edc9: README.txt - stick (or at least, at least try) to 80 columns\nc869edc9 (Edmundo Carmona Antoranz 2017-01-20 23:39:51 -0600   8) Example outp\nc869edc9 (Edmundo Carmona Antoranz 2017-01-20 23:39:51 -0600   9) params to cha\n\n>> The question is if you think it makes sense to add this option to\n>> git-blame itself.\n>>\n>> Something like:\n>> git blame --tips README.txt\n>\n> I do not think I would use it personally, as it would make it hard\n> to pipe the output of \"git blame\" to less and then /search things,\n> as extra cruft added by the option will get in the way.  IOW, I do\n> not think we want it for human-oriented output.\n\nDoing a search through less, it would make sense to leave the 1-line summary out\nbut when looking at blame output through OSI layer 8, I'd definitely\nlike to get that 1-line summary \"as an option\" every so often (more\noften than not\nin my case, probably).\n"}]}