Re: [PATCH v4] ref-filter: Add --no-contains option to tag/branch/for-each-ref

Ævar Arnfjörð Bjarmason [off-list ref] 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.

quoted

    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.

quoted

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?

quoted

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.

quoted

quoted

-#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.