Re: [PATCH v2 09/34] sequencer (rebase -i): write an author-script file

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

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

>> We keep coming back to the same argument. You want this quoting/dequoting
>> to be turned into a full-fledged parser. And I keep pointing out that the
>> code here does not *need* to parse but only construct an environment
>> block.
>
> I am afraid you are mis-reading me.  I see a code that _READS_ some
> data format, for which we already have a parser, and then write out
> things based on what it read.  I do not want you to make anything
> into a full-fledged parser---I just do not want to see an ad-hoc
> reader and instead the code to USE existing parser.

To extend this a bit.

I care rather deeply about not spreading the re-invention of parsers
and formatters throughout the code, because that would introduce
unnecesary maintenance burden.

Quoting a string so that it is acceptable inside a pair of single
quotes, occasionally stepping outside of the sq context and using
backquote to excape individual byte, for example, might feel simple
enough that anybody can just write inline instead of learning how to
call and actually calling sq_quote().  You open ', show each byte
unless it is a ', in which case you close ' and give \' and
immediately open '.  That was what sq_quote() did originally and for
a long time.  If however we allowed everybody to reinvent the code
to quote all over the place, with a lame "This is different and does
not *need* to call shared code" excuse, it would have required a lot
more effort to do a change like the one in 77d604c309 ("Enhanced
sq_quote()", 2005-10-10) that changed the quoting rules slightly to
make the output safer to accomodate different variants of shells.

The same thing can be said for split_ident() code.  The "author"
line has name, '<', email address, '>', timestring, '+' or '-', and
timezone.  Splitting them into NAME and TIME may be very simple, and
"does not *need* to parse", right?  Not really.  If you look at how
split_ident() evolved to accomodate the real world malformed lines
like duplicated closing '>' and realize that what we have there may
probably *NOT* the perfect one (i.e. there may be other malformed
input we may have to accomodate by tweaking the implementation
further), we really do not want a hand-rolled ad-hoc "splitter" that
"does not *need* to parse".

The same thing can be said for the code that reads the
author-script, too.

So please do not waste any more time arguing.  I think you spent
arguing more time than you would otherwise have had to spend if you
just used existing helper functions, both in this thread and the
older one about reading the author-script.



[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]