From: Dmitry Kakurin <dmitry.kakurin@gmail.com>
Date: Sun, 22 Jul 2007 07:52:12 GMT
Subject: Re: [PATCH] Added 'git-rev-list --igrep' option for case insensitive grep
Message-ID: <a1bbc6950707220052v3b9b5032sd1b95d7b9c229a84@mail.gmail.com>
In-Reply-To: <7vwswtrmhe.fsf@assigned-by-dhcp.cox.net>

Sounds good to me.
Let's just put short option names in front of long names in the doc to
be consistent with other sections.

On 7/21/07, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Are there notable side effects you would need to warn users
> > about?
> >
> > For example, what is the difference between "--igrep=foo
> > --author=bar" and "--grep=foo --author=bar"?
>
> Nah, let's not try to be too subtle.  It does not work.
>
> What I am wondering is if there is an unintended option name
> conflict if we simply accept "-i" and "-E" as synonyms to
> "--regexp-ignore-case" and "--extended-regexp".  The option
> parser for log family in revision.c is used from many places,
> so it is a bit hard to verify everobody to be absolutely sure,
> but I do not think of any offhand.  There indeed are commands
> that take "-i" to mean something other than ignore-case, but
> ignore-case would not be meaningful for their operation.
>
> So I would rather prefer this replacement patch.
>
> -- >8 --
> Synonyms: -i == --regexp-ignore-case, -E == --extended-regexp
>
> These options to log family were too long to type.  Give them
> shorter synonyms.
>
> Fix the parsing of the long options while at it.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-rev-list.txt |    7 ++++---
>  revision.c                     |    6 ++++--
>  2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
> index 08e7573..f9fadaf 100644
> --- a/Documentation/git-rev-list.txt
> +++ b/Documentation/git-rev-list.txt
> @@ -27,7 +27,8 @@ SYNOPSIS
>              [ \--cherry-pick ]
>              [ \--encoding[=<encoding>] ]
>              [ \--(author|committer|grep)=<pattern> ]
> -            [ \--regexp-ignore-case ] [ \--extended-regexp ]
> +            [ \--regexp-ignore-case | \-i ]
> +            [ \--extended-regexp | \-E ]
>              [ \--date={local|relative|default|iso|rfc|short} ]
>              [ [\--objects | \--objects-edge] [ \--unpacked ] ]
>              [ \--pretty | \--header ]
> @@ -227,11 +228,11 @@ limiting may be applied.
>         Limit the commits output to ones with log message that
>         matches the specified pattern (regular expression).
>
> ---regexp-ignore-case::
> +--regexp-ignore-case, -i::
>
>         Match the regexp limiting patterns without regard to letters case.
>
> ---extended-regexp::
> +--extended-regexp, -E::
>
>         Consider the limiting patterns to be extended regular expressions
>         instead of the default basic regular expressions.
> diff --git a/revision.c b/revision.c
> index 7036cf2..00b75bc 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1165,11 +1165,13 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
>                                 add_message_grep(revs, arg+7);
>                                 continue;
>                         }
> -                       if (!prefixcmp(arg, "--extended-regexp")) {
> +                       if (!strcmp(arg, "--extended-regexp") ||
> +                           !strcmp(arg, "-E")) {
>                                 regflags |= REG_EXTENDED;
>                                 continue;
>                         }
> -                       if (!prefixcmp(arg, "--regexp-ignore-case")) {
> +                       if (!strcmp(arg, "--regexp-ignore-case") ||
> +                           !strcmp(arg, "-i")) {
>                                 regflags |= REG_ICASE;
>                                 continue;
>                         }
>
>