Hi Akim. Thanks for tackling and fixing these issues. The patch is basically OK, although I have few nits and question below, that I'd like to be addressed before giving an ACK.
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". 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? > Keep a single _ instead of several. > (RENAME_sed): new. > Micro-nit: Missing capitalization of "new". Major nit: I really dislike having tow variable whose names only differ in capitalization. How about renaming them like follows? rename_sed -> sed_fix_filenames RENAME_sed -> sed_fix_header_guards You can squash the rename in this patch, or do it with a follow-up, as you prefer. > Use it. > --- > NEWS | 18 ++++++++++++++++++ > lib/ylwrap | 27 ++++++++++++++++++--------- > 2 files changed, 36 insertions(+), 9 deletions(-) > > diff --git a/NEWS b/NEWS > index 7a230ef..482216c 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,3 +1,21 @@ > +New in 1.12.7: > + > +Bugs fixed in 1.12.7: > + > I don't plan to actually make a further release in the 1.12.x series for such a bug. That is, unless it is causing some serious problems that I've been missing (in which case, they should be reported more prominently in the commit message); is this the case? If not, just rebase your series on master, and report the news as "New in 1.13". > + - ylwrap renames properly header guards in generated header files, > + instead of leaving Y_TAB_H. > + > + - ylwrap now also converts header guards in implementation files. > Sorry if I sound dense, but what are exactly these "implementation files"? > + 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. > + > + - ylwrap generates header guards with a single '_' for series of non > + alphabetic characters, instead of several. This is what Bison > + does. > This is more a (justified) change in behaviour than a bug; it should be reported as that, IMHO. > [SNIP] the rest of the patch. Thanks, Stefano