On Tue, Aug 28, 2007 at 01:21:46PM -0700, Eric Blake-1 wrote: > > +++ coreutils-6.9/src/ls.c 2007-08-26 19:58:20.000000000 +0200 > > @@ -76,7 +76,6 @@ > > # define SA_RESTART 0 > > #endif > > > > -#include "system.h" > > #include <fnmatch.h> > > Why are you deleting this include? Without it, how do you ensure > that <config.h> is pulled in before anything else? If you intend for > ls-clp.h to fill this role, then it must be included before any > system files. Also, are you sure you are not falling foul of > any 'make distcheck' rules in Makefile.maint?
I need the following definitions in ls-clp.c: 1. the i18n macro _() 2. the definition of PACKAGE_BUGREPORT 3. the definition of true and false I got everything by including system.h in ls-clp.c. Unfortunately I had to exclude it from ls.c then because there were duplicate definitons. I entered this when I wrote the patch for the wc command and at that time I was happy to get it compiled, that's all. I'm sure there is a better solution. Maybe I have to include other files. I agree that this has to be fixed. > > + Cmdline(&cmdline, argc, argv); > > GNU coding standards want a space between the function > name and open (. Right, thanks. > > +/* Extract the following section an process it with genparse > > + (see http://genparse.sourceforge.net) in order to generate a parser > > + for the command line arguments and a usage function for printing a > > help > > + screen. */ > > + > > +/* genparse file starts here > > +#include <config.h> > > +#include "system.h" > > +#include "ls.h" > > + > > +#exit_value LS_FAILURE > > I know the C standard requires this, but in practice, are all > C preprocessors tolerant of comments that contain lines > that look like preprocessing directives but which are not? That's potentially another drawback of embedding the genparse files in the C sources. > > +NONE / help flag "display this help and exit" > > +NONE / version flag "output version information and > > exit" > > It looks like one drawback of using genparse is that you lose > the system.h magic that ensures consistency between all > the apps with --help and --version, since you can't really > use the preprocessor macros *_HELP_OPTION_* here. I could imagine that this can be solved by adding the capability to include parameter definitions in a genparse file, i.e. include genparse files in other genparse files. There could be a shared genparse file with the parameter definitions for help and version which could be included by all other genparse files. > > +Report bugs to <__STRING__(PACKAGE_BUGREPORT)>. > > What happened to the TRANSLATOR comment that reminds > them to add a second line, including the address to report > translation bugs to? Also, it isn't very obvious how this > will affect xgettext extraction of strings that need > translation. Are you sure you haven't broken things > for other locales? Would the generated ls-clp.c need > to be added to POTFILES.in, or is your intent still to > have all translatable strings reside in ls.c? If I understood the i18n mechanism right then the C preprocesor is needed for the _() macros to take effect. So the genparse files can't be translated directly, even if they are embedded in C files because they are still inside of a comment. So I think ls-clp.c would have to be added to POTFILES.in. I haven't investigated how a genparse based solution affects i18n and I generally have very view experiance with i18n. I would expect that problems are caused by different partitioning of text. In the present version of ls the usage() function calls fputs() several times. The genparse version prints everything in 1 single call to printf(). So the usage() text in the present ls.c is split into multiple _() macros, whereas ls-clp.c uses 1 single _() macro for the whole help screen. Do you agree that this is the main source of trouble? Do you see other problems? I haven't fully thought this through but I think I could change genparse such that the user can control when a new print command should start thus giving control of partitioning translatable text to the user. Michael _______________________________________________ Bug-coreutils mailing list [email protected] http://lists.gnu.org/mailman/listinfo/bug-coreutils
