Re: syntax error reported in maintain/generate_code_convert_data.pl

[Gavin Smith]

On Sat, Oct 19, 2024 at 03:30:13PM +0200, Patrice Dumas wrote:
> On Fri, Oct 18, 2024 at 09:49:00PM +0100, Gavin Smith wrote:
> > /opt/csw/bin/perl ./../../maintain/generate_code_convert_data.pl \
> >         < ./main/command_data.c \
> >             ./../Data/default_css_element_class_styles.csv \
> >             ./../Data/default_direction_strings.csv \
> >             ./../Data/default_special_unit_info.csv \
> >             ./../Data/html_style_commands_element.csv \
> >             C ./main/conversion_data.c \
> >             ./main/conversion_data.h
> > 
> > I guess "continue;" should have been "next;"?  I have never looked at
> > this Perl program before and don't know what it is for.
> 
> Actually, if you have time it could be good if you had a look at this
> code.  I have started using CSV files for 'data' that is converted to
> code both for C and Perl.  Previously, I used the Perl structures to
> generate C.  All the code generation is done by
> tp/maintain/generate_code_convert_data.pl, with the target language (C
> or perl) given in argument.
> 
> The CSV files are in tp/Texinfo/Data/*.csv.  The generated Perl code is
> in tp/Texinfo/Data.pm.  The generated C code is in
> tp/Texinfo/XS/main/conversion_data.*.

It could be a good thing to avoid duplicating information between C
and Perl.  However, it also has the possible downside of adding more
indirection to the code.

In general terms, it seems like a good idea when there is a large
amount of regularly structured information that is shared between
C and Perl.  The CSS in default_css_element_class_styles.csv seems
to fall into the category.

For something like html_style_commands_element.csv, it could potentially
make the code more impenetrable.  Suppose for example someone wants to
find which part of the code handles the @sansserif command.  If they
search HTML.pm for 'sansserif' that string isn't there.  As this file
is shorter, there is less benefit to avoiding duplicating its contents.
However, the purpose, structure and meaning of this file is quite clear.
(Files such as HTML.pm are also not self-contained, accessing information
in files such as Commands.pm, so having another file to access does not
really change the situation.)

For shorter files like default_special_unit_info.csv and
default_direction_strings.csv it is less clear that it offers a net
benefit.  To be honest, the function of these files is not particularly
clear to me other than one of them is something to do with node direction
pointers.  I don't someone looking at these files for the first time
would have an easy time figuring out what they are for.

Obviously, I don't know everything you were considering when you
changed the code to use these files, so it is possible you have
already considered the trade-offs. 

One suggestion I have is that Data.pm is a rather non-descriptive name.
Could this be better named as something like HTMLData.pm, or generated
as Texinfo/Data/HTML.pm if it is only used for HTML.pm output.  Data
shared among output formats (if there is any) could go in
Texinfo/Data/Common.pm.

> Is the design ok?  If so, I could add more, converting Perl structures
> to CSV files and previous code generating C from Perl to code generating
> both C and Perl.

As I said above, it depends on what it is for.

You say that it is replacing generating C code from Perl although
I couldn't tell which scripts it was replacing.  (I looked at
the output of "git log maintain/generate_code_convert_data.pl" but
nothing jumped out at me.)

Is this replacing generating C code from Perl, or is it new generated
code that was previously duplicated between C and Perl?