[PATCH] blame: add option to print tips (--tips)

[PATCH] blame: add option to print tips (--tips)

[Edmundo Carmona Antoranz]

---
 builtin/blame.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5..4bc449f40 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1884,6 +1884,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
 #define OUTPUT_NO_AUTHOR       0200
 #define OUTPUT_SHOW_EMAIL	0400
 #define OUTPUT_LINE_PORCELAIN 01000
+#define OUTPUT_SHOW_TIPS      02000
 
 static void emit_porcelain_details(struct origin *suspect, int repeat)
 {
@@ -1939,14 +1940,18 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 	struct commit_info ci;
 	char hex[GIT_SHA1_HEXSZ + 1];
 	int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
+	int revision_length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
 
 	get_commit_info(suspect->commit, &ci, 1);
 	sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
 
+	if (opt & OUTPUT_SHOW_TIPS)
+		printf("\t%.*s: %s\n", revision_length, hex, ci.summary.buf);
+
 	cp = nth_line(sb, ent->lno);
 	for (cnt = 0; cnt < ent->num_lines; cnt++) {
 		char ch;
-		int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
+		int length = revision_length;
 
 		if (suspect->commit->object.flags & UNINTERESTING) {
 			if (blank_boundary)
@@ -2609,6 +2614,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		{ OPTION_CALLBACK, 'C', NULL, &opt, N_("score"), N_("Find line copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback },
 		{ OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
 		OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
+		OPT_BIT(0, "tips", &output_option, N_("Show tips before content lines"), OUTPUT_SHOW_TIPS),
 		OPT__ABBREV(&abbrev),
 		OPT_END()
 	};
-- 
2.11.0.rc1

Re: [PATCH] blame: add option to print tips (--tips)

[Edmundo Carmona Antoranz]

Hello, everybody!

So, this is a draft of what I mean by "adding tips to blame".

Example output (sample from builtin/blame.c):

15:32 $ ./git blame --tips -L 1934,1960 builtin/blame.c
       cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1934)
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1935) static void emit_other(struct scoreboard *sb,
struct blame_entry *ent, int opt)
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1936) {
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1937)  int cnt;
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1938)  const char *cp;
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1939)  struct origin *suspect = ent->suspect;
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1940)  struct commit_info ci;
       d59f765ac9: use sha1_to_hex_r() instead of strcpy
d59f765ac9 builtin/blame.c   (Jeff King                2015-09-24
17:08:03 -0400 1941)  char hex[GIT_SHA1_HEXSZ + 1];
       cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1942)  int show_raw_time = !!(opt &
OUTPUT_RAW_TIMESTAMP);
       f2aea1696f: blame: add option to print tips (--tips)
f2aea1696f builtin/blame.c   (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1943)  int revision_length = (opt &
OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
       cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1944)
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1945)  get_commit_info(suspect->commit, &ci, 1);
       f2fd0760f6: Convert struct object to object_id
f2fd0760f6 builtin/blame.c   (brian m. carlson         2015-11-10
02:22:28 +0000 1946)  sha1_to_hex_r(hex,
suspect->commit->object.oid.hash);
       cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1947)
       f2aea1696f: blame: add option to print tips (--tips)
f2aea1696f builtin/blame.c   (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1948)  if (opt & OUTPUT_SHOW_TIPS)
f2aea1696f builtin/blame.c   (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1949)          printf("\t%.*s: %s\n", revision_length,
hex, ci.summary.buf);
f2aea1696f builtin/blame.c   (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1950)
       cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1951)  cp = nth_line(sb, ent->lno);
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1952)  for (cnt = 0; cnt < ent->num_lines; cnt++) {
cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
16:00:04 -0700 1953)          char ch;
       f2aea1696f: blame: add option to print tips (--tips)
f2aea1696f builtin/blame.c   (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1954)          int length = revision_length;
       b11121d9e3: git-blame: show lines attributed to boundary
commits differently.
b11121d9e3 builtin-blame.c   (Junio C Hamano           2006-12-01
20:45:45 -0800 1955)
b11121d9e3 builtin-blame.c   (Junio C Hamano           2006-12-01
20:45:45 -0800 1956)          if (suspect->commit->object.flags &
UNINTERESTING) {
       e68989a739: annotate: fix for cvsserver.
e68989a739 builtin-blame.c   (Junio C Hamano           2007-02-06
01:52:04 -0800 1957)                  if (blank_boundary)
e68989a739 builtin-blame.c   (Junio C Hamano           2007-02-06
01:52:04 -0800 1958)                          memset(hex, ' ',
length);
       7ceacdffc5: "blame -c" should be compatible with "annotate"
7ceacdffc5 builtin-blame.c   (Junio C Hamano           2008-09-05
00:57:35 -0700 1959)                  else if (!(opt &
OUTPUT_ANNOTATE_COMPAT)) {
       4c10a5caa7: blame: -b (blame.blankboundary) and --root (blame.showroot)
4c10a5caa7 builtin-blame.c   (Junio C Hamano           2006-12-18
14:04:38 -0800 1960)                          length--;


Does it look "worthy"? And if so, would it be better to think of
something like an "aggregate" option (or something like that) that
would include the common information as tips, something like:

15:32 $ ./git blame --tips -L 1934,1960 builtin/blame.c
       cee7f245dc: builtin-pickaxe.c (Junio C Hamano
2006-10-19 16:00:04 -0700) git-pickaxe: blame rewritten.
1934
1935 static void emit_other(struct scoreboard *sb, struct blame_entry
*ent, int opt)
1936 {
1937  int cnt;
1938  const char *cp;
1939  struct origin *suspect = ent->suspect;
1940  struct commit_info ci;
       d59f765ac9: builtin/blame.c   (Jeff King
2015-09-24 17:08:03 -0400) use sha1_to_hex_r() instead of strcpy
1941  char hex[GIT_SHA1_HEXSZ + 1];
       cee7f245dc: builtin-pickaxe.c (Junio C Hamano
2006-10-19 16:00:04 -0700) git-pickaxe: blame rewritten.
1942  int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
       f2aea1696f: builtin/blame.c   (Edmundo Carmona Antoranz
2017-01-22 15:23:31 -0600) blame: add option to print tips (--tips)
1943  int revision_length = (opt & OUTPUT_LONG_OBJECT_NAME) ?
GIT_SHA1_HEXSZ : abbrev;
       cee7f245dc: builtin-pickaxe.c (Junio C Hamano
2006-10-19 16:00:04 -0700) git-pickaxe: blame rewritten.
1944
1945  get_commit_info(suspect->commit, &ci, 1);

Best regards!

On Sun, Jan 22, 2017 at 3:28 PM, Edmundo Carmona Antoranz
<eantoranz@gmail.com> wrote:
> ---
>  builtin/blame.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 126b8c9e5..4bc449f40 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1884,6 +1884,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
>  #define OUTPUT_NO_AUTHOR       0200
>  #define OUTPUT_SHOW_EMAIL      0400
>  #define OUTPUT_LINE_PORCELAIN 01000
> +#define OUTPUT_SHOW_TIPS      02000
>
>  static void emit_porcelain_details(struct origin *suspect, int repeat)
>  {
> @@ -1939,14 +1940,18 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
>         struct commit_info ci;
>         char hex[GIT_SHA1_HEXSZ + 1];
>         int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
> +       int revision_length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
>
>         get_commit_info(suspect->commit, &ci, 1);
>         sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
>
> +       if (opt & OUTPUT_SHOW_TIPS)
> +               printf("\t%.*s: %s\n", revision_length, hex, ci.summary.buf);
> +
>         cp = nth_line(sb, ent->lno);
>         for (cnt = 0; cnt < ent->num_lines; cnt++) {
>                 char ch;
> -               int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
> +               int length = revision_length;
>
>                 if (suspect->commit->object.flags & UNINTERESTING) {
>                         if (blank_boundary)
> @@ -2609,6 +2614,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
>                 { OPTION_CALLBACK, 'C', NULL, &opt, N_("score"), N_("Find line copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback },
>                 { OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
>                 OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
> +               OPT_BIT(0, "tips", &output_option, N_("Show tips before content lines"), OUTPUT_SHOW_TIPS),
>                 OPT__ABBREV(&abbrev),
>                 OPT_END()
>         };
> --
> 2.11.0.rc1
>

Re: [PATCH] blame: add option to print tips (--tips)

[Junio C Hamano]

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

> So, this is a draft of what I mean by "adding tips to blame".
>
> Example output (sample from builtin/blame.c):
> ...
> 15:32 $ ./git blame --tips -L 1934,1960 builtin/blame.c
>        cee7f245dc: git-pickaxe: blame rewritten.
> cee7f245dc builtin-pickaxe.c (Junio C Hamano           2006-10-19
> 16:00:04 -0700 1934)

This is unfortunately unreadable to guess what you wanted to
achieve, not because the lines are too wide, but because they are
line-wrapped.

> Does it look "worthy"? And if so, would it be better to think of
> something like an "aggregate" option (or something like that) that
> would include the common information as tips, something like:

What is the target audience?  If you are trying to write a script
that reads output by "git blame", you are strongly discouraged
unless you are reading from "git blame --porcelain" which is more
compact and has this information already IIRC.

Re: [PATCH] blame: add option to print tips (--tips)

[Edmundo Carmona Antoranz]

On Sun, Jan 22, 2017 at 4:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> What is the target audience?  If you are trying to write a script
> that reads output by "git blame", you are strongly discouraged
> unless you are reading from "git blame --porcelain" which is more
> compact and has this information already IIRC.

I wrote this for human consumption, actually. Maybe I overestimated the
need for this feature (I definitely find it handy, but it might be just me).

Re: [PATCH] blame: add option to print tips (--tips)

[Junio C Hamano]

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

> I wrote this for human consumption, actually. 

I see.

> Maybe I overestimated the
> need for this feature (I definitely find it handy, but it might be just me).

That is too early to tell.  At this point we only know there are me
who won't use it and you who will, among all the other people in the
world.

Re: [PATCH] blame: add option to print tips (--tips)

[Pranit Bauva]

Hey Junio,

On Mon, Jan 23, 2017 at 5:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> That is too early to tell.  At this point we only know there are me
> who won't use it and you who will, among all the other people in the
> world.

We can probably make it useful with some extended efforts. I use
git-blame and I sometimes find that I don't need things like the name
of the author, time, timezone and not even the file name and I have to
use a bigger terminal. If we could somehow remove those fields then
maybe this would be a useful feature.

Idea: Make git-blame understand `format`

git-log has a format option in which we can configure what all things
we need and what we don't. We could probably do the same here also.
After carefully using the format specification with git-log each
person can get a good look at however he seems to view. But I am not
sure whether this is worth the effort. I personally find this `format`
feature useful.

Regards,
Pranit Bauva

Re: [PATCH] blame: add option to print tips (--tips)

[Junio C Hamano]

Pranit Bauva <pranit.bauva@gmail.com> writes:

> We can probably make it useful with some extended efforts. I use
> git-blame and I sometimes find that I don't need things like the name
> of the author, time, timezone and not even the file name and I have to
> use a bigger terminal. If we could somehow remove those fields then
> maybe this would be a useful feature.

I admit that I didn't recall the option until somebody else told me,
but I think "blame -s" or something like that for that purpose ;-)

Re: [PATCH] blame: add option to print tips (--tips)

[Pranit Bauva]

Hey Junio,

On Tue, Jan 24, 2017 at 12:06 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
>> We can probably make it useful with some extended efforts. I use
>> git-blame and I sometimes find that I don't need things like the name
>> of the author, time, timezone and not even the file name and I have to
>> use a bigger terminal. If we could somehow remove those fields then
>> maybe this would be a useful feature.
>
> I admit that I didn't recall the option until somebody else told me,
> but I think "blame -s" or something like that for that purpose ;-)

Ah! Thanks a lot!

Regards,
Pranit Bauva