From: Erik Faye-Lund <kusmabite@googlemail.com>
To: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
Cc: git@vger.kernel.org
Subject: Re: [PATCH RFC] cherry: support --abbrev option
Date: Mon, 15 Mar 2010 23:30:45 +0100 [thread overview]
Message-ID: <40aa078e1003151530y19f10bf1l852a0e5230b11cb6@mail.gmail.com> (raw)
In-Reply-To: <4B9E69A4.2080304@lsrfire.ath.cx>
On Mon, Mar 15, 2010 at 6:08 PM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> Am 15.03.2010 17:03, schrieb Erik Faye-Lund:
>> Switch to parse-options API while we're at it.
>>
>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
>> ---
>>
>> I sometimes find it useful to look at the commit-subject together with
>> the SHA-1s. Using --abbrev increases the chance that the lines fits on
>> an 80 character wide terminal, making the output easier to read.
>>
>> builtin/log.c | 40 ++++++++++++++++++++++------------------
>> 1 files changed, 22 insertions(+), 18 deletions(-)
>>
>> diff --git a/builtin/log.c b/builtin/log.c
>> index b70d0f7..020d618 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -1286,8 +1286,11 @@ static int add_pending_commit(const char *arg, struct rev_info *revs, int flags)
>> return -1;
>> }
>>
>> -static const char cherry_usage[] =
>> -"git cherry [-v] [<upstream> [<head> [<limit>]]]";
>> +static const char * const cherry_usage[] = {
>> + "git cherry [-v] [<upstream> [<head> [<limit>]]]",
>> + NULL
>> +};
>> +
>> int cmd_cherry(int argc, const char **argv, const char *prefix)
>> {
>> struct rev_info revs;
>> @@ -1298,26 +1301,26 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
>> const char *upstream;
>> const char *head = "HEAD";
>> const char *limit = NULL;
>> - int verbose = 0;
>> + int verbose = 0, abbrev = 40;
>>
>> - if (argc > 1 && !strcmp(argv[1], "-v")) {
>> - verbose = 1;
>> - argc--;
>> - argv++;
>> - }
>> + struct option options[] = {
>> + OPT__ABBREV(&abbrev),
>> + OPT__VERBOSE(&verbose),
>> + OPT_END()
>> + };
>
> If I use --no-abbrev, do I get 0 or 40 hash chars? I didn't actually
> test it, but I suspect an "if (!abbrev) abbrev = 40;" is needed somewhere.
>
"abbrev" is initialized to 40 when declared, so you get the same
behavior as before by default.
>> - if (argc > 1 && !strcmp(argv[1], "-h"))
>> - usage(cherry_usage);
>> + argc = parse_options(argc, argv, prefix, options, cherry_usage,
>> + PARSE_OPT_KEEP_UNKNOWN);
>
> Why do you use PARSE_OPT_KEEP_UNKNOWN here? I think the old option
> parsing code lazily relied on invalid options being found out by
> add_pending_commit() later. We can be lazy again by leaving invalid
> option handling to parse_options(); sure, it changes the error message,
> but for the better.
Actually, It's a mistake, thanks for pointing it out :)
Yeah, I think your suggestion is better. Corrected.
--
Erik "kusma" Faye-Lund
next prev parent reply other threads:[~2010-03-15 22:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-15 16:03 [PATCH RFC] cherry: support --abbrev option Erik Faye-Lund
2010-03-15 17:08 ` René Scharfe
2010-03-15 22:30 ` Erik Faye-Lund [this message]
2010-03-15 23:16 ` René Scharfe
2010-03-15 23:50 ` Erik Faye-Lund
2010-03-16 0:46 ` René Scharfe
2010-03-16 0:59 ` Erik Faye-Lund
2010-03-16 1:44 ` Junio C Hamano
2010-03-15 17:13 ` Junio C Hamano
2010-03-15 22:39 ` Erik Faye-Lund
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=40aa078e1003151530y19f10bf1l852a0e5230b11cb6@mail.gmail.com \
--to=kusmabite@googlemail.com \
--cc=git@vger.kernel.org \
--cc=kusmabite@gmail.com \
--cc=rene.scharfe@lsrfire.ath.cx \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).