Re: [PATCH 3/3] ylwrap: rename header inclusion in generated parsers

[Akim Demaille]

Le 13 juil. 2012 à 13:43, Stefano Lattarini a écrit :

> See?  Another thing I had got wrong, given my ignorance and the lack
> of a proper explanation ;-)

I also meant that the bug is obvious :)  If you rename files, you
have to rename files that use them, and that this is in the context
of Bison or Flex or whatever, is irrelevant with the nature of
the bug.  Maybe some day the report file (*.output) will refer to the name
of the other files, in which case of course it is wrong not to
rename them.

>>> Hmmm... can '$from' contain sed metacharacters?  Certainly it can contain
>>> dots; still, that wouldn't cause spurious failures, only possible (albeit
>>> very unlikely) extra substitutions in "#include" lines; which I agree we
>>> don't need worry about, due to their very high unlikeliness.  So the code
>>> above is correct IMO, but it deserves some more comments, so that a future
>>> reader won't have to repeat my chain of thoughts.
>> 
>> The code is exactly the same as what was there before, just moved
>> elsewhere.
>> 
> A good excuse to improve comments and explanations, no?
> 
>> But I can add protections, sure.
>> 
> Oops, I fear you misread my feedback; I agree that extra protections are
> overkill, I'd just like you to add a brief comments (along my reasoning
> above) to explain why this is the case.

I think it is faster to see the protection being installed
installed of having to read a comment that states why we think
the protection is not needed.

>> OK.  So since I was just using the historical conventions, that
>> used to be ',', that are used in ylwrap, I understand your
>> request as a need to change all the other patterns, not just
>> the one I moved.
>> 
> Oops, no; that should be done in a follow-up patch if you are interested.
> Sorry for not stating that explicitly.  Please revert this part of the
> change.

Including in the part I just moved that you commented in the
first place?

>> +# quote_for_sed [STRING]
>> +# ----------------------
>> +# Return STRING (or stdin) quoted to be used as a sed pattern.
>> quote_for_sed ()
>> {
>> -  # FIXME: really we should care about more than '.' and '\'.
>> -  sed -e 's,[\\.],\\&,g'
>> +  case $# in
>> +    0) cat;;
>> +    1) echo "$1";;
>> 
> We'd be safer using printf, in case "$1" contains '\' characters:
> 
>   printf '%s\n' "$1"

OK.  Should I also remove the other occurrences of echo in ylwrap?

> (the Autoconf manual describes several issues with 'echo' in more details).

I'm just not used with the idea that printf is portable :)