Re: [PATCH 2/2] ylwrap: various fixes

[Stefano Lattarini]

On 12/21/2012 06:31 PM, Akim Demaille wrote:
> 
> Le 21 déc. 2012 à 17:44, Stefano Lattarini <address@hidden> a écrit :
> 
>> Hi Akim.
> 
> Hi!
> 
>> On 12/19/2012 02:55 PM, Akim Demaille wrote:
>>> * lib/ylwrap (guard): Properly honor $1.
>>>
>> I fear this is the ChangLog style that I dislike: just reporting the
>> "what" of the change, without the "why".
> 
> The "why" here is obvious: because it was wrong.  If you
> think "Fix a typo" is better, I sure can.  But I see no
> point in stating a better why (the title is there).
> IMHO, "properly" suffices.
> 
> But I extended below.
> 
>>  Since you have a very good
>> explanation of such a "why" (as seen in the NEWS file), would you
>> mind reporting it (at least in an abridged form) in the commit message
>> as well?
> 
> If you think it's useful, I sure can.
> 
I think it is.  In fact, I find your new commit message a vast
improvement. module few nits inlined below (please don't hate
me  for all this pickiness).

>> Sorry if I sound dense, but what are exactly these "implementation
>> files"?
> 
> *.c vs *.h.
>
Could that be reported in the commit message and NEWS entries as well?
Oh, but I see you've already done that in the relevant places.  Thanks.

> commit 2af6bc0c5f3d19b2a39fa22166094dc42d8dec2f
> Author: Akim Demaille <address@hidden>
> Date:   Wed Dec 19 14:51:58 2012 +0100
> 
>     ylwrap: various fixes
>     
>     rename properly header guards in generated header files, instead of
>
Missing capitalization at the beginning of the sentence.

>     leaving Y_TAB_H.
>
>     convert header guards in implementation files.  Because ylwrap failed
>
Likewise.

>     to rename properly #include in the implementation files, current
>     versions of Bison (e.g., 2.7) duplicate the generated header file in
>     the implementation file.  The header guard then protects the
>     implementation file from duplicate definitions from the header file.
>     
>     generate header guards with a single '_' for series of non alphabetic
>
Likewise.

>     characters, instead of several.  This is what Bison does.
>     
>     Fixes t/yacc-d-basic.sh.
>
Or "Makes the test 't/yacc-d-basic.sh' pass again".

>     * lib/ylwrap (guard): Properly honor $1 to rename properly the
>     header guards.
>     Keep a single _ instead of several.
>     (rename_sed): Rename as...
>     (sed_fix_filenames): this.
>     Suggested by Stefano Lattarini.
>     (sed_fix_header_guards): New.
>     Use it.
> 
> diff --git a/NEWS b/NEWS
> index 5bf4379..d827798 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -49,6 +49,18 @@ New in 1.13:
>      should take precedence over the same-named automake-provided macro
>      (defined in '/usr/local/share/aclocal-1.14/vala.m4').
>  
> +* Bugs fixed:
> +
> +  - ylwrap renames properly header guards in generated header files
> +    (*.h), instead of leaving Y_TAB_H.
> +
> +  - ylwrap now also converts header guards in implementation files
> +    (*.c).  Because ylwrap failed to rename properly #include in the
> +    implementation files, current versions of Bison (e.g., 2.7)
> +    duplicate the generated header file in the implementation file.
> +    The header guard then protects the implementation file from
> +    duplicate definitions from the header file.
> +
>  * Version requirements:
>  
>    - Autoconf 2.65 or greater is now required.
> @@ -206,6 +218,12 @@ New in 1.13:
>    - Support for tcc (the Tiny C Compiler) has been improved, and is now
>      handled through a dedicated 'tcc' mode.
>  
> +* The ylwrap script:
> +
> +  - ylwrap generates header guards with a single '_' for series of non
> +    alphabetic characters, instead of several.  This is what Bison
>
s/Bison/Bison >= 2.7/ maybe?

The rest of the patch seems OK.  Thanks for taking care of this.

Regards,
  Stefano