Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
In fact, it _would_ be a bug to make it a CMDMODE if --batch were
correctly marked as one (but it is not sufficient to reason the other
way; --batch without --batch-all-objects is still mutually exclusive
with -t, etc).
> What really makes things confusing, IMHO, is the --textconv and --filter
quoted
options. They are marked as CMDMODEs, and they are indeed mutually
exclusive with -t, etc. But they also work with --batch, which is itself
a different mode.
So I don't think OPT_CMDMODE could ever present this complete set of
rules, because they are not all mutually exclusive with each other. But
I think calling "--batch-all-objects" a mode is just muddying the waters
even further.
I think we've got some different understanding of what a CMDMODE
means. --batch-all-objects should be a cmdmode, but --batch, --buffer
etc. can't be. Similarly it's not a bug that --filters and --textconv
are cmdmodes, but you think that's bad.
Among options[] elements, "batch" and "batch-check" take &batch, and
they are obviously mutually exclusive. "batch-all-objects" can flip
the batch.all_objects flag to affect operations that use &batch
(namely, these two), so it is more like a modifier and can never be
a cmdmode.
Is "git cat-file -t tag --batch" a valid way to invoke the command?
Are there options (like "-t" in the above example) that are marked
with OPT_CMDMODE that can be used with "--batch" or "--batch-check"?
If the answer is "no", then "--batch" and "--batch-check" could also
be command modes, but I suspect OPT_CMDMODE() does not have enough
flexibility to say "use the &opt to record which command mode is
requested, and by the way, there is this extra pointer &batch to
stuff necessary information in and use this callback to fill it", so
even if "--batch" and "--batch-check" are incompatible with existing
command modes, it needs a bit or preparatory work to make them.
On Fri, Oct 08, 2021 at 01:34:26PM -0700, Junio C Hamano wrote:
quoted
quoted
So I don't think OPT_CMDMODE could ever present this complete set of
rules, because they are not all mutually exclusive with each other. But
I think calling "--batch-all-objects" a mode is just muddying the waters
even further.
I think we've got some different understanding of what a CMDMODE
means. --batch-all-objects should be a cmdmode, but --batch, --buffer
etc. can't be. Similarly it's not a bug that --filters and --textconv
are cmdmodes, but you think that's bad.
Among options[] elements, "batch" and "batch-check" take &batch, and
they are obviously mutually exclusive. "batch-all-objects" can flip
the batch.all_objects flag to affect operations that use &batch
(namely, these two), so it is more like a modifier and can never be
a cmdmode.
Is "git cat-file -t tag --batch" a valid way to invoke the command?
Are there options (like "-t" in the above example) that are marked
with OPT_CMDMODE that can be used with "--batch" or "--batch-check"?
If the answer is "no", then "--batch" and "--batch-check" could also
be command modes, but I suspect OPT_CMDMODE() does not have enough
flexibility to say "use the &opt to record which command mode is
requested, and by the way, there is this extra pointer &batch to
stuff necessary information in and use this callback to fill it", so
even if "--batch" and "--batch-check" are incompatible with existing
command modes, it needs a bit or preparatory work to make them.
Yes, it would definitely need that extension. But it's also weirder than
that. --textconv is an OPT_CMDMODE(), because it is mutually exclusive
with "-t", etc. But it is not exclusive with "--batch". So there is not
a single set of mutually exclusive cmdmodes. There are three sets:
1. single-object options like "-t", "-p", etc
2. --textconv and --filters, which can take a single object or batch
input
3. batch options like --batch-check --batch
(1) is mutually exclusive with (2) and (3), but the latter two are not
exclusive with each other.
The current code uses OPT_CMDMODE() for (1) and (2), and then manually
enforces the exclusion between (1) and (3). But IMHO it is (2) that is
the odd-man out, in that it can be its own mode or a modifier, and it
probably should not be OPT_CMDMODE() (but from the end-user perspective,
that is OK, though it may influence how we document or group things).
I do not think talking about --batch-all-objects makes sense here. As
you said above (and I said already several times), it is a modifier for
--batch/--batch-check. Making it into a CMDMODE does not break anything
(currently), but:
- it does not really help; it only makes sense with batch options, and
we already must manually catch those interacting with single-object
options (because making them OPT_CMDMODE does not work for the
reasons above).
- if we _were_ to make the batch options CMDMODEs, then it would be
doing the wrong thing (though I don't foresee that happening because
of those same reasons)
- if we allowed "--textconv --batch --batch-all-objects", then it
would likewise do the wrong thing (by conflicting with --textconv's
CMDMODE). We disallow that now, but it is a potentially well-formed
request (show all objects, textconv-ing blobs). It's not useful
enough for anybody to have cared, but I think it shows why the
semantic distinction of treating --batch-all-objects as a
batch-option and not itself a mode is important.
-Peff