* [PATCH v2] add --abbrev to 'git cherry'
@ 2009-05-30 14:03 Jeff Epler
2009-05-30 16:26 ` Markus Heidelberg
0 siblings, 1 reply; 9+ messages in thread
From: Jeff Epler @ 2009-05-30 14:03 UTC (permalink / raw)
To: git
Abbreviating ids makes 'git cherry -v' more useful, since you can see more
of the commit message summary:
git cherry -v --abbrev | less -S
Signed-off-by: Jeff Epler <jepler@unpythonic.net>
---
An earlier version of this patch added multiple different flags to 'git
cherry', but --abbrev (was -a) is really the important one. Thanks to
Jakub Narebski and Michael J Gruber for comments on the first patch.
Documentation/git-cherry.txt | 5 ++++-
builtin-log.c | 24 +++++++++++++++++++-----
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt
index 7deefda..5c03da0 100644
--- a/Documentation/git-cherry.txt
+++ b/Documentation/git-cherry.txt
@@ -7,7 +7,7 @@ git-cherry - Find commits not merged upstream
SYNOPSIS
--------
-'git cherry' [-v] [<upstream> [<head> [<limit>]]]
+'git cherry' [-v] [--abbrev[=<n>]] [<upstream> [<head> [<limit>]]]
DESCRIPTION
-----------
@@ -49,6 +49,9 @@ OPTIONS
-v::
Verbose.
+--abbrev[=<n>]::
+ Abbreviate commit ids to the given number of characters
+
<upstream>::
Upstream branch to compare against.
Defaults to the first tracked remote branch, if available.
diff --git a/builtin-log.c b/builtin-log.c
index f10cfeb..1f3093e 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1130,7 +1130,7 @@ static int add_pending_commit(const char *arg, struct rev_info *revs, int flags)
}
static const char cherry_usage[] =
-"git cherry [-v] [<upstream> [<head> [<limit>]]]";
+"git cherry [-v] [--abbrev[=<n>]] [<upstream> [<head> [<limit>]]]";
int cmd_cherry(int argc, const char **argv, const char *prefix)
{
struct rev_info revs;
@@ -1142,9 +1142,23 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
const char *head = "HEAD";
const char *limit = NULL;
int verbose = 0;
+ int abbrev = 40;
+
+ while(argc > 1 && argv[1][0] == '-') {
+ if (!strcmp(argv[1], "-v")) {
+ verbose = 1;
+ } else if(!strcmp(argv[1], "--abbrev")) {
+ abbrev = DEFAULT_ABBREV;
+ } else if(!prefixcmp(argv[1], "--abbrev=")) {
+ abbrev = strtol(argv[1] + 9, NULL, 10);
+ if(abbrev < MINIMUM_ABBREV)
+ abbrev = MINIMUM_ABBREV;
+ else if(abbrev > 40)
+ abbrev = 40;
+ } else {
+ die("unrecognized argument: %s", argv[1]);
+ }
- if (argc > 1 && !strcmp(argv[1], "-v")) {
- verbose = 1;
argc--;
argv++;
}
@@ -1218,12 +1232,12 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
struct strbuf buf = STRBUF_INIT;
pretty_print_commit(CMIT_FMT_ONELINE, commit,
&buf, 0, NULL, NULL, 0, 0);
- printf("%c %s %s\n", sign,
+ printf("%c %.*s %s\n", sign, abbrev,
sha1_to_hex(commit->object.sha1), buf.buf);
strbuf_release(&buf);
}
else {
- printf("%c %s\n", sign,
+ printf("%c %.*s\n", sign, abbrev,
sha1_to_hex(commit->object.sha1));
}
--
1.5.4.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2] add --abbrev to 'git cherry'
2009-05-30 14:03 [PATCH v2] add --abbrev to 'git cherry' Jeff Epler
@ 2009-05-30 16:26 ` Markus Heidelberg
2009-05-30 16:53 ` [PATCH v3] " Jeff Epler
0 siblings, 1 reply; 9+ messages in thread
From: Markus Heidelberg @ 2009-05-30 16:26 UTC (permalink / raw)
To: Jeff Epler; +Cc: git
Jeff Epler, 30.05.2009:
> Documentation/git-cherry.txt | 5 ++++-
> builtin-log.c | 24 +++++++++++++++++++-----
> 2 files changed, 23 insertions(+), 6 deletions(-)
You could also add --abbrev= to the bash completion.
> diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt
> index 7deefda..5c03da0 100644
> --- a/Documentation/git-cherry.txt
> +++ b/Documentation/git-cherry.txt
> @@ -49,6 +49,9 @@ OPTIONS
> -v::
> Verbose.
>
> +--abbrev[=<n>]::
> + Abbreviate commit ids to the given number of characters
The full stop is missing :)
And you could add "The default value is 7." as in the git-branch docs.
Or even copy the whole description from there for consistency, it also
mentions that this sets the minimum length, the displayed SHA1 may be
longer, but more about this below.
> diff --git a/builtin-log.c b/builtin-log.c
> index f10cfeb..1f3093e 100644
> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -1218,12 +1232,12 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
> struct strbuf buf = STRBUF_INIT;
> pretty_print_commit(CMIT_FMT_ONELINE, commit,
> &buf, 0, NULL, NULL, 0, 0);
> - printf("%c %s %s\n", sign,
> + printf("%c %.*s %s\n", sign, abbrev,
> sha1_to_hex(commit->object.sha1), buf.buf);
> strbuf_release(&buf);
> }
> else {
> - printf("%c %s\n", sign,
> + printf("%c %.*s\n", sign, abbrev,
> sha1_to_hex(commit->object.sha1));
> }
There is no test for unique ids. "git cherry --abbrev=4" always prints 4
chars per SHA1, so "git show" on these SHA1s mostly gives "error: short
SHA1 xxxx is ambiguous." in git.git.
find_unique_abbrev() will help.
Markus
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v3] add --abbrev to 'git cherry'
2009-05-30 16:26 ` Markus Heidelberg
@ 2009-05-30 16:53 ` Jeff Epler
2009-05-30 21:13 ` Stephen Boyd
0 siblings, 1 reply; 9+ messages in thread
From: Jeff Epler @ 2009-05-30 16:53 UTC (permalink / raw)
To: git
Abbreviating ids makes 'git cherry -v' more useful, since you can see more
of the commit message summary:
git cherry -v --abbrev | less -S
Signed-off-by: Jeff Epler <jepler@unpythonic.net>
---
Compared to the last patch, this adds to the bash completion, improves
doc consistency, and uses find_unique_abbrev to shorten commit ids.
Thanks to Markus Heidelberg for feedback.
Documentation/git-cherry.txt | 6 +++++-
builtin-log.c | 24 +++++++++++++++++++-----
contrib/completion/git-completion.bash | 10 +++++++++-
3 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt
index 7deefda..c8cbbcc 100644
--- a/Documentation/git-cherry.txt
+++ b/Documentation/git-cherry.txt
@@ -7,7 +7,7 @@ git-cherry - Find commits not merged upstream
SYNOPSIS
--------
-'git cherry' [-v] [<upstream> [<head> [<limit>]]]
+'git cherry' [-v] [--abbrev[=<n>]] [<upstream> [<head> [<limit>]]]
DESCRIPTION
-----------
@@ -49,6 +49,10 @@ OPTIONS
-v::
Verbose.
+--abbrev[=<n>]::
+ Alter the sha1's minimum display length in the output listing.
+ The default value is 7.
+
<upstream>::
Upstream branch to compare against.
Defaults to the first tracked remote branch, if available.
diff --git a/builtin-log.c b/builtin-log.c
index f10cfeb..c115a8e 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1130,7 +1130,7 @@ static int add_pending_commit(const char *arg, struct rev_info *revs, int flags)
}
static const char cherry_usage[] =
-"git cherry [-v] [<upstream> [<head> [<limit>]]]";
+"git cherry [-v] [--abbrev[=<n>]] [<upstream> [<head> [<limit>]]]";
int cmd_cherry(int argc, const char **argv, const char *prefix)
{
struct rev_info revs;
@@ -1142,9 +1142,23 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
const char *head = "HEAD";
const char *limit = NULL;
int verbose = 0;
+ int abbrev = 40;
+
+ while(argc > 1 && argv[1][0] == '-') {
+ if (!strcmp(argv[1], "-v")) {
+ verbose = 1;
+ } else if(!strcmp(argv[1], "--abbrev")) {
+ abbrev = DEFAULT_ABBREV;
+ } else if(!prefixcmp(argv[1], "--abbrev=")) {
+ abbrev = strtol(argv[1] + 9, NULL, 10);
+ if(abbrev < MINIMUM_ABBREV)
+ abbrev = MINIMUM_ABBREV;
+ else if(abbrev > 40)
+ abbrev = 40;
+ } else {
+ die("unrecognized argument: %s", argv[1]);
+ }
- if (argc > 1 && !strcmp(argv[1], "-v")) {
- verbose = 1;
argc--;
argv++;
}
@@ -1219,12 +1233,12 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
pretty_print_commit(CMIT_FMT_ONELINE, commit,
&buf, 0, NULL, NULL, 0, 0);
printf("%c %s %s\n", sign,
- sha1_to_hex(commit->object.sha1), buf.buf);
+ find_unique_abbrev(commit->object.sha1, abbrev), buf.buf);
strbuf_release(&buf);
}
else {
printf("%c %s\n", sign,
- sha1_to_hex(commit->object.sha1));
+ find_unique_abbrev(commit->object.sha1, abbrev));
}
list = list->next;
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c84d765..536a769 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -804,7 +804,15 @@ _git_checkout ()
_git_cherry ()
{
- __gitcomp "$(__git_refs)"
+ local cur="${COMP_WORDS[COMP_CWORD]}"
+ case "$cur" in
+ -*)
+ __gitcomp "-v --abbrev --abbrev="
+ ;;
+ *)
+ __gitcomp "$(__git_refs)"
+ ;;
+ esac
}
_git_cherry_pick ()
--
1.5.4.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3] add --abbrev to 'git cherry'
2009-05-30 16:53 ` [PATCH v3] " Jeff Epler
@ 2009-05-30 21:13 ` Stephen Boyd
2009-05-30 23:08 ` Junio C Hamano
0 siblings, 1 reply; 9+ messages in thread
From: Stephen Boyd @ 2009-05-30 21:13 UTC (permalink / raw)
To: Jeff Epler; +Cc: git, markus.heidelberg
On Sat, May 30, 2009 at 9:53 AM, Jeff Epler <jepler@unpythonic.net> wrote:
> @@ -1142,9 +1142,23 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
> const char *head = "HEAD";
> const char *limit = NULL;
> int verbose = 0;
> + int abbrev = 40;
> +
> + while(argc > 1 && argv[1][0] == '-') {
> + if (!strcmp(argv[1], "-v")) {
> + verbose = 1;
> + } else if(!strcmp(argv[1], "--abbrev")) {
> + abbrev = DEFAULT_ABBREV;
> + } else if(!prefixcmp(argv[1], "--abbrev=")) {
> + abbrev = strtol(argv[1] + 9, NULL, 10);
> + if(abbrev < MINIMUM_ABBREV)
> + abbrev = MINIMUM_ABBREV;
> + else if(abbrev > 40)
> + abbrev = 40;
> + } else {
> + die("unrecognized argument: %s", argv[1]);
> + }
>
You might want to look at using the parse options API. It has options
for verbose and abbrev builtin, so you don't have to do any extra
work. Plus you get a nice usage message for free. See
Documentation/technical/api-parse-options.txt for more info.
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index c84d765..536a769 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -804,7 +804,15 @@ _git_checkout ()
>
> _git_cherry ()
> {
> - __gitcomp "$(__git_refs)"
> + local cur="${COMP_WORDS[COMP_CWORD]}"
> + case "$cur" in
> + -*)
> + __gitcomp "-v --abbrev --abbrev="
> + ;;
> + *)
> + __gitcomp "$(__git_refs)"
> + ;;
> + esac
Completion doesn't include short options (-v). This also means that
--* is used instead of -*
Finally, you'll want to Cc Shawn (Shawn O. Pearce
<spearce@spearce.org>) on bash completion.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3] add --abbrev to 'git cherry'
2009-05-30 21:13 ` Stephen Boyd
@ 2009-05-30 23:08 ` Junio C Hamano
2009-05-30 23:44 ` Markus Heidelberg
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Junio C Hamano @ 2009-05-30 23:08 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Jeff Epler, git, markus.heidelberg
Stephen Boyd <bebarino@gmail.com> writes:
> You might want to look at using the parse options API. It has options
> for verbose and abbrev builtin, so you don't have to do any extra
> work....
Why do people even think a change like this to a _plumbing_ command is
desirable?
Admittedly, there already is "verbose" option that adds redundant
information to the output of this particular plumbing, which might
arguably be equally wrong as what this patch does, but I think it is
excusable. At least it lets the Porcelain script that uses the command
avoid calling 'git cat-file commit' to find out the title of the commit.
But --abbrev does not even add any information. If implemented correctly
(which earlier iteration did not even do), it may not lose information by
choping the output too short to make it ambiguous, but as others pointed
out about using grep in the calling Porcelain to filter (or more likely,
sift the lines into "+" and "-" bins) to shoot down -d/-D options, I do
not see the point of adding --abbrev to this plumbing command very much.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3] add --abbrev to 'git cherry'
2009-05-30 23:08 ` Junio C Hamano
@ 2009-05-30 23:44 ` Markus Heidelberg
2009-05-31 0:16 ` Junio C Hamano
2009-05-31 4:53 ` Stephen Boyd
[not found] ` <20090531133804.GB16770@unpythonic.net>
2 siblings, 1 reply; 9+ messages in thread
From: Markus Heidelberg @ 2009-05-30 23:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephen Boyd, Jeff Epler, git
Junio C Hamano, 31.05.2009:
> Why do people even think a change like this to a _plumbing_ command is
> desirable?
git-cherry is plumbing? In git(1) it is listed as porcelain.
And the plumbings show-ref, ls-files and ls-tree support --abbrev.
Markus
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3] add --abbrev to 'git cherry'
2009-05-30 23:44 ` Markus Heidelberg
@ 2009-05-31 0:16 ` Junio C Hamano
0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2009-05-31 0:16 UTC (permalink / raw)
To: markus.heidelberg; +Cc: Junio C Hamano, Stephen Boyd, Jeff Epler, git
Markus Heidelberg <markus.heidelberg@web.de> writes:
> Junio C Hamano, 31.05.2009:
>> Why do people even think a change like this to a _plumbing_ command is
>> desirable?
>
> git-cherry is plumbing? In git(1) it is listed as porcelain.
I'd say it is a miscategorization, but I do not care too deeply, as I
never use it myself (even though my Porcelain scripts would).
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] add --abbrev to 'git cherry'
2009-05-30 23:08 ` Junio C Hamano
2009-05-30 23:44 ` Markus Heidelberg
@ 2009-05-31 4:53 ` Stephen Boyd
[not found] ` <20090531133804.GB16770@unpythonic.net>
2 siblings, 0 replies; 9+ messages in thread
From: Stephen Boyd @ 2009-05-31 4:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff Epler, git, markus.heidelberg
Junio C Hamano wrote:
> Stephen Boyd <bebarino@gmail.com> writes:
>> You might want to look at using the parse options API. It has options
>> for verbose and abbrev builtin, so you don't have to do any extra
>> work....
>
> Why do people even think a change like this to a _plumbing_ command is
> desirable?
>
> Admittedly, there already is "verbose" option that adds redundant
> information to the output of this particular plumbing, which might
> arguably be equally wrong as what this patch does, but I think it is
> excusable. At least it lets the Porcelain script that uses the command
> avoid calling 'git cat-file commit' to find out the title of the commit.
>
> But --abbrev does not even add any information. If implemented correctly
> (which earlier iteration did not even do), it may not lose information by
> choping the output too short to make it ambiguous, but as others pointed
> out about using grep in the calling Porcelain to filter (or more likely,
> sift the lines into "+" and "-" bins) to shoot down -d/-D options, I do
> not see the point of adding --abbrev to this plumbing command very much.
I was tempted to say the same thing, but I decided to leave it up to the
maintainer ;-) Maybe if there was a compelling use case it would make
more sense?
Or, would it make more sense to just use git-log? Right now you can do
git log --oneline --cherry-pick <head>..<upstream> and get close. Maybe
we can add a "--cherry" option to git-log which will act like git-cherry
by finding unmerged commits?
^ permalink raw reply [flat|nested] 9+ messages in thread[parent not found: <20090531133804.GB16770@unpythonic.net>]
end of thread, other threads:[~2009-06-01 11:54 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-30 14:03 [PATCH v2] add --abbrev to 'git cherry' Jeff Epler
2009-05-30 16:26 ` Markus Heidelberg
2009-05-30 16:53 ` [PATCH v3] " Jeff Epler
2009-05-30 21:13 ` Stephen Boyd
2009-05-30 23:08 ` Junio C Hamano
2009-05-30 23:44 ` Markus Heidelberg
2009-05-31 0:16 ` Junio C Hamano
2009-05-31 4:53 ` Stephen Boyd
[not found] ` <20090531133804.GB16770@unpythonic.net>
[not found] ` <7v8wkdrqys.fsf@alter.siamese.dyndns.org>
2009-06-01 11:54 ` Jeff Epler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).