Re: [PATCH v3 13/25] sequencer: prepare for rebase -i's commit functionality

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:

>> Hmph, didn't we recently add parse_key_value_squoted() to build
>> read_author_script() in builtin/am.c on top of it, so that this
>> piece of code can also take advantage of and share the parser?
>
> I already pointed out that the author-script file may *not* be quoted.

I think my puzzlement comes from here.  What makes it OK for "am" to
expect the contents of author-script file to be quoted but it is not
OK to expect the same here?  What makes it not quoted for _this_
reader, in other words?

I am not sure what you meant by "nominally related", but the purpose
of the author-script in these two codepaths is the same, isn't it?
Somebody leaves the author information from the source (either from
an e-mailed patch or an existing commit), so that a later step can
use that pieces of information left in the file when (re)creating a
commit to record the tree made by using pieces of information from
the source.

Are our use in the author-script in these two codepaths _already_
inconsistent?  IOW, "am" never writes malformed unquoted values,
while the sequencer writes out in a way that is randomly quoted or
not quoted, iow, if you fed such an author-file to "am", it wouldn't
understand it?

I fully support your position to use different codepaths, if the
file that has the same name and that is used for the same purpose
uses different format in these two separate codepaths and the users
already expect them to be different.  We obviously need to have two
separate parsers.

But if that is not the case, IOW, if "am"'s author-script shares the
same issue (i.e. "'am' initially writes the file properly quoted,
but this or that can happen to change its quoting and we need to
read from such a file"), then perhaps sharing needs to happen the
other way around?  This patch may prepare "rebase -i" side for the
"this or that" (I still do not know what they are) to allow the
resulting file read correctly, but the same "this or that" can break
what "am" has used and is in use there if that is the case, no?

What makes it OK for "am" to expect the contents of author-script
file to be quoted but it is not OK to expect the same here?  What
makes it not quoted for _this_ reader, and doesn't "am" share the
same issue?

>> > +/*
>> 
>> Offtopic: this line and the beginning of the new comment block that
>> begins with "Read the author-script" above show a suboptimal marking
>> of what is added and what is left.  I wonder "diff-indent-heuristic"
>> topic by Michael can help to make it look better.
>
> Maybe. I'll try to look into that once the more serious questions about
> this patch series have been addressed.

You do not have to; the remark was meant for Michael (newly cc'ed in
the message you are responding to).



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]