{"thread":{"id":"19605","subject":"[PATCH v2] add --abbrev to 'git cherry'","startedAt":"2009-05-30T14:03:49Z","lastAt":"2009-06-01T11:54:34Z","messageCount":9,"participants":["Jeff Epler","Markus Heidelberg","Stephen Boyd","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"115085","messageId":"20090530140349.GA25265@unpythonic.net","threadId":"19605","inReplyTo":null,"subject":"[PATCH v2] add --abbrev to 'git cherry'","fromName":"Jeff Epler","fromEmail":"jepler@unpythonic.net","sentAt":"2009-05-30T14:03:49Z","receivedAt":"2009-05-30T14:03:49Z","isPatch":true,"sender":{"key":"jepler@unpythonic.net","avatar":"https://avatars.githubusercontent.com/u/1517291?v=4"},"body":"Abbreviating ids makes 'git cherry -v' more useful, since you can see more\nof the commit message summary:\n    git cherry -v --abbrev | less -S\n\nSigned-off-by: Jeff Epler <jepler@unpythonic.net>\n---\n\nAn earlier version of this patch added multiple different flags to 'git\ncherry', but --abbrev (was -a) is really the important one.  Thanks to\nJakub Narebski and Michael J Gruber for comments on the first patch.\n\n Documentation/git-cherry.txt |    5 ++++-\n builtin-log.c                |   24 +++++++++++++++++++-----\n 2 files changed, 23 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt\nindex 7deefda..5c03da0 100644\n--- a/Documentation/git-cherry.txt\n+++ b/Documentation/git-cherry.txt\n@@ -7,7 +7,7 @@ git-cherry - Find commits not merged upstream\n \n SYNOPSIS\n --------\n-'git cherry' [-v] [<upstream> [<head> [<limit>]]]\n+'git cherry' [-v] [--abbrev[=<n>]] [<upstream> [<head> [<limit>]]]\n \n DESCRIPTION\n -----------\n@@ -49,6 +49,9 @@ OPTIONS\n -v::\n \tVerbose.\n \n+--abbrev[=<n>]::\n+\tAbbreviate commit ids to the given number of characters\n+\n <upstream>::\n \tUpstream branch to compare against.\n \tDefaults to the first tracked remote branch, if available.\ndiff --git a/builtin-log.c b/builtin-log.c\nindex f10cfeb..1f3093e 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -1130,7 +1130,7 @@ static int add_pending_commit(const char *arg, struct rev_info *revs, int flags)\n }\n \n static const char cherry_usage[] =\n-\"git cherry [-v] [<upstream> [<head> [<limit>]]]\";\n+\"git cherry [-v] [--abbrev[=<n>]] [<upstream> [<head> [<limit>]]]\";\n int cmd_cherry(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n@@ -1142,9 +1142,23 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n \tconst char *head = \"HEAD\";\n \tconst char *limit = NULL;\n \tint verbose = 0;\n+\tint abbrev = 40;\n+\n+\twhile(argc > 1 && argv[1][0] == '-') {\n+\t\tif (!strcmp(argv[1], \"-v\")) {\n+\t\t\tverbose = 1;\n+\t\t} else if(!strcmp(argv[1], \"--abbrev\")) {\n+\t\t\tabbrev = DEFAULT_ABBREV;\n+\t\t} else if(!prefixcmp(argv[1], \"--abbrev=\")) {\n+\t\t\tabbrev = strtol(argv[1] + 9, NULL, 10);\n+\t\t\tif(abbrev < MINIMUM_ABBREV)\n+\t\t\t\tabbrev = MINIMUM_ABBREV;\n+\t\t\telse if(abbrev > 40)\n+\t\t\t\tabbrev = 40;\n+\t\t} else {\n+\t\t\tdie(\"unrecognized argument: %s\", argv[1]);\n+\t\t}\n \n-\tif (argc > 1 && !strcmp(argv[1], \"-v\")) {\n-\t\tverbose = 1;\n \t\targc--;\n \t\targv++;\n \t}\n@@ -1218,12 +1232,12 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n \t\t\tstruct strbuf buf = STRBUF_INIT;\n \t\t\tpretty_print_commit(CMIT_FMT_ONELINE, commit,\n \t\t\t                    &buf, 0, NULL, NULL, 0, 0);\n-\t\t\tprintf(\"%c %s %s\\n\", sign,\n+\t\t\tprintf(\"%c %.*s %s\\n\", sign, abbrev,\n \t\t\t       sha1_to_hex(commit->object.sha1), buf.buf);\n \t\t\tstrbuf_release(&buf);\n \t\t}\n \t\telse {\n-\t\t\tprintf(\"%c %s\\n\", sign,\n+\t\t\tprintf(\"%c %.*s\\n\", sign, abbrev,\n \t\t\t       sha1_to_hex(commit->object.sha1));\n \t\t}\n \n-- \n1.5.4.3\n"},{"id":"115090","messageId":"200905301826.11924.markus.heidelberg@web.de","threadId":"19605","inReplyTo":"20090530140349.GA25265@unpythonic.net","subject":"Re: [PATCH v2] add --abbrev to 'git cherry'","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-05-30T16:26:11Z","receivedAt":"2009-05-30T16:26:11Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Jeff Epler, 30.05.2009:\n>  Documentation/git-cherry.txt |    5 ++++-\n>  builtin-log.c                |   24 +++++++++++++++++++-----\n>  2 files changed, 23 insertions(+), 6 deletions(-)\n\nYou could also add --abbrev= to the bash completion.\n\n> diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt\n> index 7deefda..5c03da0 100644\n> --- a/Documentation/git-cherry.txt\n> +++ b/Documentation/git-cherry.txt\n> @@ -49,6 +49,9 @@ OPTIONS\n>  -v::\n>  \tVerbose.\n>  \n> +--abbrev[=<n>]::\n> +\tAbbreviate commit ids to the given number of characters\n\nThe full stop is missing :)\nAnd you could add \"The default value is 7.\" as in the git-branch docs.\nOr even copy the whole description from there for consistency, it also\nmentions that this sets the minimum length, the displayed SHA1 may be\nlonger, but more about this below.\n\n> diff --git a/builtin-log.c b/builtin-log.c\n> index f10cfeb..1f3093e 100644\n> --- a/builtin-log.c\n> +++ b/builtin-log.c\n> @@ -1218,12 +1232,12 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n>  \t\t\tstruct strbuf buf = STRBUF_INIT;\n>  \t\t\tpretty_print_commit(CMIT_FMT_ONELINE, commit,\n>  \t\t\t                    &buf, 0, NULL, NULL, 0, 0);\n> -\t\t\tprintf(\"%c %s %s\\n\", sign,\n> +\t\t\tprintf(\"%c %.*s %s\\n\", sign, abbrev,\n>  \t\t\t       sha1_to_hex(commit->object.sha1), buf.buf);\n>  \t\t\tstrbuf_release(&buf);\n>  \t\t}\n>  \t\telse {\n> -\t\t\tprintf(\"%c %s\\n\", sign,\n> +\t\t\tprintf(\"%c %.*s\\n\", sign, abbrev,\n>  \t\t\t       sha1_to_hex(commit->object.sha1));\n>  \t\t}\n\nThere is no test for unique ids. \"git cherry --abbrev=4\" always prints 4\nchars per SHA1, so \"git show\" on these SHA1s mostly gives \"error: short\nSHA1 xxxx is ambiguous.\" in git.git.\n\nfind_unique_abbrev() will help.\n\nMarkus\n"},{"id":"115092","messageId":"20090530165306.GA1142@unpythonic.net","threadId":"19605","inReplyTo":"200905301826.11924.markus.heidelberg@web.de","subject":"[PATCH v3] add --abbrev to 'git cherry'","fromName":"Jeff Epler","fromEmail":"jepler@unpythonic.net","sentAt":"2009-05-30T16:53:06Z","receivedAt":"2009-05-30T16:53:06Z","isPatch":true,"sender":{"key":"jepler@unpythonic.net","avatar":"https://avatars.githubusercontent.com/u/1517291?v=4"},"body":"Abbreviating ids makes 'git cherry -v' more useful, since you can see more\nof the commit message summary:\n    git cherry -v --abbrev | less -S\n\nSigned-off-by: Jeff Epler <jepler@unpythonic.net>\n---\n\nCompared to the last patch, this adds to the bash completion, improves\ndoc consistency, and uses find_unique_abbrev to shorten commit ids.\nThanks to Markus Heidelberg for feedback.\n\n Documentation/git-cherry.txt           |    6 +++++-\n builtin-log.c                          |   24 +++++++++++++++++++-----\n contrib/completion/git-completion.bash |   10 +++++++++-\n 3 files changed, 33 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt\nindex 7deefda..c8cbbcc 100644\n--- a/Documentation/git-cherry.txt\n+++ b/Documentation/git-cherry.txt\n@@ -7,7 +7,7 @@ git-cherry - Find commits not merged upstream\n \n SYNOPSIS\n --------\n-'git cherry' [-v] [<upstream> [<head> [<limit>]]]\n+'git cherry' [-v] [--abbrev[=<n>]] [<upstream> [<head> [<limit>]]]\n \n DESCRIPTION\n -----------\n@@ -49,6 +49,10 @@ OPTIONS\n -v::\n \tVerbose.\n \n+--abbrev[=<n>]::\n+\tAlter the sha1's minimum display length in the output listing.\n+\tThe default value is 7.\n+\n <upstream>::\n \tUpstream branch to compare against.\n \tDefaults to the first tracked remote branch, if available.\ndiff --git a/builtin-log.c b/builtin-log.c\nindex f10cfeb..c115a8e 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -1130,7 +1130,7 @@ static int add_pending_commit(const char *arg, struct rev_info *revs, int flags)\n }\n \n static const char cherry_usage[] =\n-\"git cherry [-v] [<upstream> [<head> [<limit>]]]\";\n+\"git cherry [-v] [--abbrev[=<n>]] [<upstream> [<head> [<limit>]]]\";\n int cmd_cherry(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n@@ -1142,9 +1142,23 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n \tconst char *head = \"HEAD\";\n \tconst char *limit = NULL;\n \tint verbose = 0;\n+\tint abbrev = 40;\n+\n+\twhile(argc > 1 && argv[1][0] == '-') {\n+\t\tif (!strcmp(argv[1], \"-v\")) {\n+\t\t\tverbose = 1;\n+\t\t} else if(!strcmp(argv[1], \"--abbrev\")) {\n+\t\t\tabbrev = DEFAULT_ABBREV;\n+\t\t} else if(!prefixcmp(argv[1], \"--abbrev=\")) {\n+\t\t\tabbrev = strtol(argv[1] + 9, NULL, 10);\n+\t\t\tif(abbrev < MINIMUM_ABBREV)\n+\t\t\t\tabbrev = MINIMUM_ABBREV;\n+\t\t\telse if(abbrev > 40)\n+\t\t\t\tabbrev = 40;\n+\t\t} else {\n+\t\t\tdie(\"unrecognized argument: %s\", argv[1]);\n+\t\t}\n \n-\tif (argc > 1 && !strcmp(argv[1], \"-v\")) {\n-\t\tverbose = 1;\n \t\targc--;\n \t\targv++;\n \t}\n@@ -1219,12 +1233,12 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n \t\t\tpretty_print_commit(CMIT_FMT_ONELINE, commit,\n \t\t\t                    &buf, 0, NULL, NULL, 0, 0);\n \t\t\tprintf(\"%c %s %s\\n\", sign,\n-\t\t\t       sha1_to_hex(commit->object.sha1), buf.buf);\n+\t\t\t       find_unique_abbrev(commit->object.sha1, abbrev), buf.buf);\n \t\t\tstrbuf_release(&buf);\n \t\t}\n \t\telse {\n \t\t\tprintf(\"%c %s\\n\", sign,\n-\t\t\t       sha1_to_hex(commit->object.sha1));\n+\t\t\t       find_unique_abbrev(commit->object.sha1, abbrev));\n \t\t}\n \n \t\tlist = list->next;\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c84d765..536a769 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -804,7 +804,15 @@ _git_checkout ()\n \n _git_cherry ()\n {\n-\t__gitcomp \"$(__git_refs)\"\n+\tlocal cur=\"${COMP_WORDS[COMP_CWORD]}\"\n+\tcase \"$cur\" in\n+\t-*)\n+\t\t__gitcomp \"-v --abbrev --abbrev=\"\n+\t\t;;\n+\t*)\n+\t\t__gitcomp \"$(__git_refs)\"\n+\t\t;;\n+\tesac\n }\n \n _git_cherry_pick ()\n-- \n1.5.4.3\n"},{"id":"115097","messageId":"780e0a6b0905301413o2686fe34qaa076209c26c0b55@mail.gmail.com","threadId":"19605","inReplyTo":"20090530165306.GA1142@unpythonic.net","subject":"Re: [PATCH v3] add --abbrev to 'git cherry'","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-05-30T21:13:51Z","receivedAt":"2009-05-30T21:13:51Z","isPatch":true,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"On Sat, May 30, 2009 at 9:53 AM, Jeff Epler <jepler@unpythonic.net> wrote:\n> @@ -1142,9 +1142,23 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n>        const char *head = \"HEAD\";\n>        const char *limit = NULL;\n>        int verbose = 0;\n> +       int abbrev = 40;\n> +\n> +       while(argc > 1 && argv[1][0] == '-') {\n> +               if (!strcmp(argv[1], \"-v\")) {\n> +                       verbose = 1;\n> +               } else if(!strcmp(argv[1], \"--abbrev\")) {\n> +                       abbrev = DEFAULT_ABBREV;\n> +               } else if(!prefixcmp(argv[1], \"--abbrev=\")) {\n> +                       abbrev = strtol(argv[1] + 9, NULL, 10);\n> +                       if(abbrev < MINIMUM_ABBREV)\n> +                               abbrev = MINIMUM_ABBREV;\n> +                       else if(abbrev > 40)\n> +                               abbrev = 40;\n> +               } else {\n> +                       die(\"unrecognized argument: %s\", argv[1]);\n> +               }\n>\n\nYou might want to look at using the parse options API. It has options\nfor verbose and abbrev builtin, so you don't have to do any extra\nwork. Plus you get a nice usage message for free. See\nDocumentation/technical/api-parse-options.txt for more info.\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index c84d765..536a769 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -804,7 +804,15 @@ _git_checkout ()\n>\n>  _git_cherry ()\n>  {\n> -       __gitcomp \"$(__git_refs)\"\n> +       local cur=\"${COMP_WORDS[COMP_CWORD]}\"\n> +       case \"$cur\" in\n> +       -*)\n> +               __gitcomp \"-v --abbrev --abbrev=\"\n> +               ;;\n> +       *)\n> +               __gitcomp \"$(__git_refs)\"\n> +               ;;\n> +       esac\n\nCompletion doesn't include short options (-v). This also means that\n--* is used instead of -*\n\nFinally, you'll want to Cc Shawn (Shawn O. Pearce\n<spearce@spearce.org>) on bash completion.\n"},{"id":"115101","messageId":"7v63fiyyrz.fsf@alter.siamese.dyndns.org","threadId":"19605","inReplyTo":"780e0a6b0905301413o2686fe34qaa076209c26c0b55@mail.gmail.com","subject":"Re: [PATCH v3] add --abbrev to 'git cherry'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-30T23:08:32Z","receivedAt":"2009-05-30T23:08:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephen Boyd <bebarino@gmail.com> writes:\n\n> You might want to look at using the parse options API. It has options\n> for verbose and abbrev builtin, so you don't have to do any extra\n> work....\n\nWhy do people even think a change like this to a _plumbing_ command is\ndesirable?\n\nAdmittedly, there already is \"verbose\" option that adds redundant\ninformation to the output of this particular plumbing, which might\narguably be equally wrong as what this patch does, but I think it is\nexcusable.  At least it lets the Porcelain script that uses the command\navoid calling 'git cat-file commit' to find out the title of the commit.\n\nBut --abbrev does not even add any information.  If implemented correctly\n(which earlier iteration did not even do), it may not lose information by\nchoping the output too short to make it ambiguous, but as others pointed\nout about using grep in the calling Porcelain to filter (or more likely,\nsift the lines into \"+\" and \"-\" bins) to shoot down -d/-D options, I do\nnot see the point of adding --abbrev to this plumbing command very much.\n"},{"id":"115103","messageId":"200905310144.56380.markus.heidelberg@web.de","threadId":"19605","inReplyTo":"7v63fiyyrz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] add --abbrev to 'git cherry'","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-05-30T23:44:56Z","receivedAt":"2009-05-30T23:44:56Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Junio C Hamano, 31.05.2009:\n> Why do people even think a change like this to a _plumbing_ command is\n> desirable?\n\ngit-cherry is plumbing? In git(1) it is listed as porcelain.\nAnd the plumbings show-ref, ls-files and ls-tree support --abbrev.\n\nMarkus\n"},{"id":"115104","messageId":"7vzlcuw2ib.fsf@alter.siamese.dyndns.org","threadId":"19605","inReplyTo":"200905310144.56380.markus.heidelberg@web.de","subject":"Re: [PATCH v3] add --abbrev to 'git cherry'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-31T00:16:12Z","receivedAt":"2009-05-31T00:16:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Markus Heidelberg <markus.heidelberg@web.de> writes:\n\n> Junio C Hamano, 31.05.2009:\n>> Why do people even think a change like this to a _plumbing_ command is\n>> desirable?\n>\n> git-cherry is plumbing? In git(1) it is listed as porcelain.\n\nI'd say it is a miscategorization, but I do not care too deeply, as I\nnever use it myself (even though my Porcelain scripts would).\n"},{"id":"115113","messageId":"4A220D65.4040708@gmail.com","threadId":"19605","inReplyTo":"7v63fiyyrz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] add --abbrev to 'git cherry'","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-05-31T04:53:57Z","receivedAt":"2009-05-31T04:53:57Z","isPatch":true,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"Junio C Hamano wrote:\n> Stephen Boyd <bebarino@gmail.com> writes:\n>> You might want to look at using the parse options API. It has options\n>> for verbose and abbrev builtin, so you don't have to do any extra\n>> work....\n>\n> Why do people even think a change like this to a _plumbing_ command is\n> desirable?\n>\n> Admittedly, there already is \"verbose\" option that adds redundant\n> information to the output of this particular plumbing, which might\n> arguably be equally wrong as what this patch does, but I think it is\n> excusable.  At least it lets the Porcelain script that uses the command\n> avoid calling 'git cat-file commit' to find out the title of the commit.\n>\n> But --abbrev does not even add any information.  If implemented correctly\n> (which earlier iteration did not even do), it may not lose information by\n> choping the output too short to make it ambiguous, but as others pointed\n> out about using grep in the calling Porcelain to filter (or more likely,\n> sift the lines into \"+\" and \"-\" bins) to shoot down -d/-D options, I do\n> not see the point of adding --abbrev to this plumbing command very much.\n\nI was tempted to say the same thing, but I decided to leave it up to the\nmaintainer ;-) Maybe if there was a compelling use case it would make\nmore sense?\n\nOr, would it make more sense to just use git-log? Right now you can do\ngit log --oneline --cherry-pick <head>..<upstream> and get close. Maybe\nwe can add a \"--cherry\" option to git-log which will act like git-cherry\nby finding unmerged commits?\n"},{"id":"115195","messageId":"20090601115434.GA25837@unpythonic.net","threadId":"19605","inReplyTo":"7v8wkdrqys.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] add --abbrev to 'git cherry'","fromName":"Jeff Epler","fromEmail":"jepler@unpythonic.net","sentAt":"2009-06-01T11:54:34Z","receivedAt":"2009-06-01T11:54:34Z","isPatch":true,"sender":{"key":"jepler@unpythonic.net","avatar":"https://avatars.githubusercontent.com/u/1517291?v=4"},"body":"On Sun, May 31, 2009 at 12:51:23PM -0700, Junio C Hamano wrote:\n> Stopping here would be a good idea if \"log --left-right --cherry-pick A...B\"\n> (perhaps with a custom --pretty option) covers what you originally wanted\n> to do.\n\nYes, I now see that 'git log' can pretty much do what I want.  Having\nlearned of 'git cherry', it didn't cross my mind that 'git log' was set\nup to do all 'git cherry' did and more.\n\n> But if the reason why you wanted --abbrev was because you wanted to use it\n> in your scripted Porcelain, and if the reason why you have your scripted\n> Porcelain is because you wanted to add some _other_ information that \"log\"\n> does not give you easily, perhaps it would be a good idea to share _that_.\n\nNo, this is all about displaying directly to the user (me) in a pager,\nspecifically about getting as much of the change summary in the first 80\ncolumns as possible.\n\nJeff\n"}]}