Jeff King <peff@xxxxxxxx> writes:
> But I think that makes the --stdin check redundant. I.e., here:
>
>> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
>> index 08b340552f..c46b6e4061 100644
>> --- a/builtin/index-pack.c
>> +++ b/builtin/index-pack.c
>> @@ -1970,6 +1970,10 @@ int cmd_index_pack(int argc,
>> usage(index_pack_usage);
>> if (fix_thin_pack && !from_stdin)
>> die(_("the option '%s' requires '%s'"), "--fix-thin", "--stdin");
>> + if (promisor_msg && !from_stdin)
>> + die(_("the option '%s' requires '%s'"), "--promisor", "--stdin");
>> + if (promisor_msg && pack_name)
>> + die(_("--promisor cannot be used with a pack name"));
>
> ...just the second one would be sufficient, because the context just
> above this has:
>
> if (!pack_name && !from_stdin)
> usage(index_pack_usage);
>
> So if there isn't a pack name then from_stdin must be set anyway.
Nice findings that leads to ...
> What you've written won't behave incorrectly, but I wonder if this means
> we can explain the rule in a more simple way:
>
> - the --promisor option requires that we be indexing a pack in the
> object database
>
> - when not given a pack name on the command line, we know this is true
> (because we generate the name ourselves internally)
>
> - when given a pack name on the command line, we _could_ check that it
> is inside the object directory, but we don't currently do so and
> just bail. That could be changed in the future.
>
> And then there is no mention of --stdin at all (though of course it is
> an implication of the second point, since we have to get input somehow).
... a good simplification. Not of the implementation---as it is
already simple enough---but of the concept, and simplification of
the latter counts a lot more ;-)
Thanks, both, for working on this.
