Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> And then later in the documentation:
>
> -l <pattern>, --list <pattern>
>
> I.e. for git-branch this type of invocation wouldn't make sense and
> would just happen to work, but for git-tag the --list option is
> explicitly documented to immediately take a <pattern> argument.
But that (i.e. "to immediately take") is not how it actually works,
as you already know after looking at how 'l' is defined as OPT_CMDMODE.
The command line is more like "-l" chooses the "list mode", and the
pattern is _NOT_ an argument to the option at all. The command line
is more like "-l <options to affect selection criteria>..." and the
<pattern> is one of these criteria. The command line convention being
dashed-options first then other arguments, it makes sense to do
-l --contains HEAD v\*
because "--contains HEAD" is a dashed-option (which takes an argument)
and "v\*" is a pattern (which is "other arguments").
> I'll change it.
>
>> git tag -l --no-contains v2.10.1-3-gcf5c7253e0 'v[0-9]*' |
>> sort | tail -n 10
>
> Although I'll add a \ to that so you can still paste it to a terminal.
Please don't. The shell knows, when you end a line with pipe, that
you haven't finished your sentence and keeps listening to you.
>> Reviewers would appreciate you refrain from doing that in the same
>> patch. Do a minimum patch so that the review can concentrate on
>> what got changed (i.e. contents), followed by a mechanical reflow as
>> a follow-up, or something like that, would be much nicer to handle.
>
> Okey, so two patches, one where I potentially produce very long lines
> & then re-flow them in a subsequent commit.
Or preferrably, the last "-" line in a hunk of your first patch may
be longer than the first "+" line that replaces it that may be
overly short (i.e. chopping the end of existing paragraph and
replacing the remainder). And then reflow comes, e.g.
-Okey, so two patches, one where I potentially produce very long lines
+Okey, so two patches, one where I
+cut an existing line short if it makes the patch churn smaller
& then re-flow them in a subsequent commit.
> If I'd like to base on top of that to make things easier for you do
> you publish jk/ref-filter-flags-cleanup sowhere? I.e. as a git ref
> rather than me also following that topic, applying it on top of
> master, and then applying my topic on top of that.
Do you mean a repository that holds broken-out topics? If so:
git://github.com/gitster/git/
is what you are looking for, perhaps?
>
>> and have both default to HEAD? I know that would not make sense as
>> a set operation, but I am curious what our command line parser
>> (which is oblivious to what the command is doing) does. I am guessing
>> that it would barf saying "--contains" needs a commit but "--no-contains"
>> is not a commit (which is very sensible)?
>
> It'll spew out "error: malformed object name --no-contains".
>
> You can do "--contains HEAD --no-contains HEAD" to get nothing.
>
> In an earlier thread I was discussing with Jeff whether it would be
> worthwhile to error out in that case, but his opinion was
> (paraphrasing) "Nah, GIGO", which I think makes sense in this case.
I agree with Peff (I said "that would not make sense as a set
operation", didn't I? ;-); I was only curious if the notation used
in the documentation, i.e. "--opt [<object>]" made sense. It looks
as if it would accept "--contains --no-contains" (omitting arguments
from both options), but it is not so, and I was wondering if we need
to improve the documentation, or the readers are OK with the notation.
>>> -#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, 0)
>>> +#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("contains", v, h, PARSE_OPT_NONEG)
>>> +#define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH("no-contains", v, h, PARSE_OPT_NONEG)
>>> #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN)
>>> +#define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN)
>>
>> Hmph, perhaps WITH/WITHOUT also do not take "--no-" form hence need OPT_NONEG?
>
> I may be missing some subtlety here, but I think you've misread this
> (admittedly dense) chunk. the /WITH/ options don't supply NONEG, just
> HIDDEN.
Maybe I was unclear. As --contains should not take --no-contains
and use "unset" (because new code wants to see "no-contains" and act
on it in the new code, I was wondering if we should forbid --no-with
and --no-without in a similar way by using OPT_NONEG in addition to
OPT_HIDDEN.