{"thread":{"id":"42609","subject":"[PATCH] blame: improve diagnosis for \"--reverse NEW\"","startedAt":"2016-06-16T06:55:52Z","lastAt":"2016-06-16T06:55:52Z","messageCount":3,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"289231","messageId":"xmqqtwgvadru.fsf@gitster.mtv.corp.google.com","threadId":"42609","inReplyTo":null,"subject":"[PATCH] blame: improve diagnosis for \"--reverse NEW\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-14T17:20:05Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git blame --reverse OLD..NEW -- PATH\" tells us to start from the\ncontents in PATH at OLD and observe how each line is changed while\nthe history developers up to NEW, and report for each line the\nlatest commit up to which the line survives in the original form.\n\nIf you say \"git blame --reverse NEW -- PATH\" by mistake, we complain\nabout the missing OLD, but we phrased it as \"No commit to dig down\nto?\" In this case, however, we are digging up from OLD, so say so.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/git-blame.txt | 2 +-\n builtin/blame.c | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\nindex ba54175..16323eb 100644\n--- a/Documentation/git-blame.txt\n+++ b/Documentation/git-blame.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n [verse]\n 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]\n \t [-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]\n-\t [--progress] [--abbrev=] [ | --contents | --reverse ]\n+\t [--progress] [--abbrev=] [ | --contents | --reverse ..]\n \t [--] \n \n DESCRIPTION\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 21f42b0..281f372 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2473,7 +2473,7 @@ static char *prepare_initial(struct scoreboard *sb)\n \t\tfinal_commit_name = revs->pending.objects[i].name;\n \t}\n \tif (!final_commit_name)\n-\t\tdie(\"No commit to dig down to?\");\n+\t\tdie(\"No commit to dig up from?\");\n \treturn xstrdup(final_commit_name);\n }\n \n-- \n2.9.0-290-g8794d48\n"},{"id":"289232","messageId":"xmqqporjackh.fsf@gitster.mtv.corp.google.com","threadId":"42609","inReplyTo":"xmqqtwgvadru.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2] blame: improve diagnosis for \"--reverse NEW\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-14T17:46:06Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git blame --reverse OLD..NEW -- PATH\" tells us to start from the\ncontents in PATH at OLD and observe how each line is changed while\nthe history developers up to NEW, and report for each line the\nlatest commit up to which the line survives in the original form.\n\nIf you say \"git blame --reverse NEW -- PATH\" by mistake, we complain\nabout the missing OLD, but we phrased it as \"No commit to dig down\nto?\" In this case, however, we are digging up from OLD, so say so.\n\nSigned-off-by: Junio C Hamano \n---\n\n * There was another instance of \"dig down to\" in the same function.\n\n Documentation/git-blame.txt | 2 +-\n builtin/blame.c | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\nindex ba54175..16323eb 100644\n--- a/Documentation/git-blame.txt\n+++ b/Documentation/git-blame.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n [verse]\n 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]\n \t [-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]\n-\t [--progress] [--abbrev=] [ | --contents | --reverse ]\n+\t [--progress] [--abbrev=] [ | --contents | --reverse ..]\n \t [--] \n \n DESCRIPTION\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 21f42b0..a027b8a 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2466,14 +2466,14 @@ static char *prepare_initial(struct scoreboard *sb)\n \t\tif (obj->type != OBJ_COMMIT)\n \t\t\tdie(\"Non commit %s?\", revs->pending.objects[i].name);\n \t\tif (sb->final)\n-\t\t\tdie(\"More than one commit to dig down to %s and %s?\",\n+\t\t\tdie(\"More than one commit to dig up from, %s and %s?\",\n \t\t\t revs->pending.objects[i].name,\n \t\t\t final_commit_name);\n \t\tsb->final = (struct commit *) obj;\n \t\tfinal_commit_name = revs->pending.objects[i].name;\n \t}\n \tif (!final_commit_name)\n-\t\tdie(\"No commit to dig down to?\");\n+\t\tdie(\"No commit to dig up from?\");\n \treturn xstrdup(final_commit_name);\n }\n \n-- \n2.9.0-290-g8794d48\n"},{"id":"289239","messageId":"xmqqziqn8vg8.fsf_-_@gitster.mtv.corp.google.com","threadId":"42609","inReplyTo":"xmqqporjackh.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] blame: dwim \"blame --reverse OLD\" as \"blame --reverse OLD..\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-14T18:41:11Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Instead of always requiring both ends of a range, we could DWIM\n\"OLD\", which could be a misspelt \"OLD..\", to be a range that ends at\nthe current commit.\n\nSigned-off-by: Junio C Hamano \n---\n\n * I am not convinced that this is a good change, though. It is\n true that there is no other sensible interpretation of the user's\n intent when --reverse is given a single positive commit without\n any other revision, but at the same time, this feels a bit too\n much special casing that could hurt casual users when they are\n still forming their mental world model by learning from examples.\n\n Documentation/blame-options.txt | 5 +++--\n builtin/blame.c | 38 ++++++++++++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\nindex 02cb684..6c6c78f 100644\n--- a/Documentation/blame-options.txt\n+++ b/Documentation/blame-options.txt\n@@ -28,12 +28,13 @@ include::line-range-format.txt[]\n -S ::\n \tUse revisions from revs-file instead of calling linkgit:git-rev-list[1].\n \n---reverse::\n+--reverse ..::\n \tWalk history forward instead of backward. Instead of showing\n \tthe revision in which a line appeared, this shows the last\n \trevision in which a line has existed. This requires a range of\n \trevision like START..END where the path to blame exists in\n-\tSTART.\n+\tSTART. `git blame --reverse START` is taken as `git blame\n+\t--reverse START..HEAD` for convenience.\n \n -p::\n --porcelain::\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex a027b8a..574b47d 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2447,6 +2447,41 @@ static char *prepare_final(struct scoreboard *sb)\n \treturn xstrdup_or_null(name);\n }\n \n+static const char *dwim_reverse_initial(struct scoreboard *sb)\n+{\n+\t/*\n+\t * DWIM \"git blame --reverse ONE -- PATH\" as\n+\t * \"git blame --reverse ONE..HEAD -- PATH\" but only do so\n+\t * when it makes sense.\n+\t */\n+\tstruct object *obj;\n+\tstruct commit *head_commit;\n+\tunsigned char head_sha1[20];\n+\n+\tif (sb->revs->pending.nr != 1)\n+\t\treturn NULL;\n+\n+\t/* Is that sole rev a committish? */\n+\tobj = sb->revs->pending.objects[0].item;\n+\tobj = deref_tag(obj, NULL, 0);\n+\tif (obj->type != OBJ_COMMIT)\n+\t\treturn NULL;\n+\n+\t/* Do we have HEAD? */\n+\tif (!resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING, head_sha1, NULL))\n+\t\treturn NULL;\n+\thead_commit = lookup_commit_reference_gently(head_sha1, 1);\n+\tif (!head_commit)\n+\t\treturn NULL;\n+\n+\t/* Turn \"ONE\" into \"ONE..HEAD\" then */\n+\tobj->flags |= UNINTERESTING;\n+\tadd_pending_object(sb->revs, &head_commit->object, \"HEAD\");\n+\n+\tsb->final = (struct commit *)obj;\n+\treturn sb->revs->pending.objects[0].name;\n+}\n+\n static char *prepare_initial(struct scoreboard *sb)\n {\n \tint i;\n@@ -2472,6 +2507,9 @@ static char *prepare_initial(struct scoreboard *sb)\n \t\tsb->final = (struct commit *) obj;\n \t\tfinal_commit_name = revs->pending.objects[i].name;\n \t}\n+\n+\tif (!final_commit_name)\n+\t\tfinal_commit_name = dwim_reverse_initial(sb);\n \tif (!final_commit_name)\n \t\tdie(\"No commit to dig up from?\");\n \treturn xstrdup(final_commit_name);\n"}]}