From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Fri, 10 Mar 2017 12:09:30 GMT
Subject: Re: [PATCH v2] ref-filter: Add --no-contains option to tag/branch/for-each-ref
Message-ID: <CACBZZX7EW013ZhDtUWome5nOqLjha34B9gJCRQgc0J0qkvWBuA@mail.gmail.com>
In-Reply-To: <CACBZZX74fpEzic3Qs0kG5i5pb-up+Ct0cp71beDXaj+BoDcKRQ@mail.gmail.com>

On Fri, Mar 10, 2017 at 12:46 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Thu, Mar 9, 2017 at 9:31 PM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> On Thu, Mar 9, 2017 at 9:02 PM, Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>>>
>>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
>>> index 525737a5d8..4938496194 100644
>>> --- a/Documentation/git-tag.txt
>>> +++ b/Documentation/git-tag.txt
>>> @@ -12,7 +12,7 @@ SYNOPSIS
>>>  'git tag' [-a | -s | -u <keyid>] [-f] [-m <msg> | -F <file>]
>>>         <tagname> [<commit> | <object>]
>>>  'git tag' -d <tagname>...
>>> -'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
>>> +'git tag' [-n[<num>]] -l [--[no-]contains <commit>] [--points-at <object>]
>>>         [--column[=<options>] | --no-column] [--create-reflog] [--sort=<key>]
>>>         [--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]
>>>  'git tag' -v [--format=<format>] <tagname>...
>>> @@ -124,6 +124,10 @@ This option is only applicable when listing tags without annotation lines.
>>>         Only list tags which contain the specified commit (HEAD if not
>>>         specified).
>>>
>>> +--no-contains [<commit>]::
>>> +       Only list tags which don't contain the specified commit (HEAD if
>>> +       not secified).
>>
>> s/secified/specified/
>>
>>> +
>>>  --points-at <object>::
>>>         Only list tags of the given object.
>>>
>>> diff --git a/builtin/branch.c b/builtin/branch.c
>>> index 94f7de7fa5..e8d534604c 100644
>>> --- a/builtin/branch.c
>>> +++ b/builtin/branch.c
>>> @@ -548,7 +548,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>>>                 OPT_SET_INT('r', "remotes",     &filter.kind, N_("act on remote-tracking branches"),
>>>                         FILTER_REFS_REMOTES),
>>>                 OPT_CONTAINS(&filter.with_commit, N_("print only branches that contain the commit")),
>>> +               OPT_NO_CONTAINS(&filter.no_commit, N_("print only branches that don't contain the commit")),
>>>                 OPT_WITH(&filter.with_commit, N_("print only branches that contain the commit")),
>>> +               OPT_WITHOUT(&filter.with_commit, N_("print only branches that don't contain the commit")),
>>
>> s/with_commit/no_commit/
>
> Thanks!
>
> FWIW this is the current status of my WIP v3. I noticed a couple of
> other issues where --contains was mentioned without --no-contains.
>
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 4938496194..d9243bf5e4 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -129 +129 @@ This option is only applicable when listing tags
> without annotation lines.
> -       not secified).
> +       not specified).
> diff --git a/builtin/branch.c b/builtin/branch.c
> index e8d534604c..dd96319726 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -553 +553 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> -               OPT_WITHOUT(&filter.with_commit, N_("print only
> branches that don't contain the commit")),
> +               OPT_WITHOUT(&filter.no_commit, N_("print only branches
> that don't contain the commit")),
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index b1ae2388e6..a11542c4fd 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -12 +12 @@ static char const * const for_each_ref_usage[] = {
> -       N_("git for-each-ref [--contains [<object>]]"),
> +       N_("git for-each-ref [(--contains | --no-contains) [<object>]]"),
> diff --git a/builtin/tag.c b/builtin/tag.c
> index d83674e3e6..57289132a9 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -25 +25 @@ static const char * const git_tag_usage[] = {
> -       N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
> +       N_("git tag -l [-n[<num>]] [--[no-]contains <commit>]
> [--points-at <object>]"

Add to that:

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8ad5719962..bec3d0fb42 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1804 +1804 @@ EOF"
-       test_line_count ">" 70 actual
+       test_line_count ">" 10 actual

The tests currently fail with v2 if gpg isn't installed, which'll
cause the number to dip below 70, just setting it to a much more
conservative 10, but maybe this should just be "test -s actual" ...

>
> These last two hunks are going to bust the i18n files for most
> languages. Junio/Jiang, in cases like these where I could fix those up
> with a search/replace on po/* without knowing the languages in
> question (since it's purely changing e.g. --contains to
> --[no-]contains), what do you prefer to do, have that as part of this
> patch, or do it after the fact through the normal i18n maintenance
> process?