Re: [PATCH v6] [GSOC] trailer: add new trailer.<token>.cmd config option

[Junio C Hamano <gitster@pobox.com>]

ZheNing Hu <adlternative@gmail.com> writes:

>> >  If some '<token>=<value>' arguments are also passed on the command
>> >  line, when a 'trailer.<token>.command' is configured, the command will
>> >  also be executed for each of these arguments. And the <value> part of
>>
>> This talks about 'trailer.<token>.command'.  Should this be changed
>> to '.cmd'?
>>
>> Or does everything after "When this option is specified, if there is
>> no trailer with ..." apply to both the old .command and new .cmd?
>> If so, that was not clear at all---we'd need to clarify this part.
>>
>
> Because ".command" will be eliminated, may be only leave those
> description Information to ".cmd" is better.

Not really.

Until .command goes away, people who want to migrate a config file
written in the .command days to .cmd would need to know what rules
govern .command variant.  Otherwise they would not know what the
original they inherited, using .command, wanted to do, and have no
way to emulate it with the new .cmd approach.  If something is
shared between the two, at least you need to mention that it applies
to both.

Until .command actually gets removed, that is.

>> Unusual indentation here.  But more importantly, I am not sure if
>> having both 'see' and 'help' examples is worth it---they are similar
>> enough that the second one does not teach anything new to those who
>> studied the first one already, aren't they?
>>
>
> Ok, I will think about other examples.

Or just use one, and let somebody else in the future encounter real
world example that is different enough to come up with a follow up
patch to describe that example.

>> > diff --git a/trailer.c b/trailer.c
>> > index be4e9726421c..6aeff6a1bd33 100644
>> > --- a/trailer.c
>> > +++ b/trailer.c
>> > ...
>> > -     strvec_push(&cp.args, cmd.buf);
>> > +     if (conf->cmd) {
>> > +             // cp.shell_no_implicit_args = 1;
>>
>> Do not add new code that is commented out.  Besides we do not use // comment.
>>
>> > +             strbuf_addstr(&cmd, conf->cmd);
>> > +             strvec_push(&cp.args, cmd.buf);
>> > +             if (arg)
>> > +                     strvec_push(&cp.args, arg);
>>
>> Thanks.
>
> Thanks.