On Wed, Feb 10, 2021 at 09:40:21PM +0000, Gavin Smith wrote:
> On Wed, Feb 10, 2021 at 12:15:39PM -0800, Per Bothner wrote:
> > On 2/10/21 1:07 AM, Gavin Smith wrote:
> > > For the long term health of the Texinfo project, more people do need
> > > to understand this Perl code. Nobody has come forward to write new
> > > back-ends to generate other output formats (LaTeX, Org Mode,
> > > MediaWiki...). I don't think it's much to do with the fact it's
> > > written in Perl.
> >
> > It doesn't help - Perl was never all that popular, and is less so than
> > it used to be. I never really learned it, though I did figure out how
> > to make some changes to the code.
>
> Speaking from personal experience, my biggest issue with Perl is not,
> as people often say, the syntax: it is the lack of types. It makes
> it harder to understand what code it supposed to do when everything
> is a "scalar" type that could be everything from a number, a character
> string, or a complex data structure.
>
> I do not think that Perl the best choice for a large complex program
> like this one, both for speed and comprehensibility, but it's completely
> impossible at this stage to move away from it.
I agree for the speed, but, and I may be largely biased, I do not think
that the comprehensibility is linked with the choice of the language,
the presence or absence of types, more to the data representation and the
to the code flow.
> I think some of the functions in ParserNonXS.pm would benefit from
> being split up into smaller functions (as I did in the C rewrite),
> although this would hurt performance due to the high cost of a
> function call in Perl.
I do not think that it would make sense to spend much time on
ParserNonXS.pm. As long as it is maintained it can be used to prototype
changes in parsing, but otherwise the future is the C parser.
> > I was able to make some local changes, and add/change the generated html
> > in places. However, some more complicated things I couldn't figure out
> > in the time I spent on it. The control flow with the table-driven
> > processing was confusing, especially for anyone not fluent in Perl.
>
> That is useful to know. I guess this is the HTML part of it you are
> talking about here. There is quite a lot of indirection in this code.
> For example, to see what the code is for the @vtable command, you
> search for 'vtable' in HTML.pm and you see there is a line
>
> $default_commands_conversion{'vtable'} = \&_convert_xtable_command;
>
> along with the definition of _convert_xtable_command. However, it's
> harder to find out when and how this is ever used. The
> %default_commands_conversion hash is copied elsewhere in the middle of
> this block of code:
>
> foreach my $command (keys(%misc_commands), keys(%brace_commands),
> keys (%block_commands), keys(%no_brace_commands), 'value') {
> if (exists($Texinfo::Config::texinfo_commands_conversion{$command})) {
> $self->{'commands_conversion'}->{$command}
> = $Texinfo::Config::texinfo_commands_conversion{$command};
> } else {
> if ($self->get_conf('FORMAT_MENU') ne 'menu'
> and ($command eq 'menu' or $command eq 'detailmenu')) {
> $self->{'commands_conversion'}->{$command} = undef;
> } elsif ($format_raw_commands{$command}
> and !$self->{'expanded_formats_hash'}->{$command}) {
> } elsif (exists($default_commands_conversion{$command})) {
> $self->{'commands_conversion'}->{$command}
> = $default_commands_conversion{$command};
> if ($command eq 'menu' and $self->get_conf('SIMPLE_MENU')) {
> $self->{'commands_conversion'}->{$command}
> = $default_commands_conversion{'example'};
> }
> }
> }
> }
>
> and that is perhaps hard to follow. I would have to think if it
> could be simplified at all, but it might not be possible.
>
> To take another example, in a few places in the code
> there are calls to output the footnotes like
>
> my $foot_text = &{$self->{'format_footnotes_text'}}($self);
>
> However, searching the code for the string "format_footnotes_text"
> doesn't show it being set anywhere, only used. It's a bit of a
> mystery to try to work this out (the "format_footnotes_text" is
> set after concatenating the two strings "format_" and
> "footnotes_text").
I wrote that code, but I agree that it is very confusing that the
strings for the indirections definitions and for their use are
different. This is a problem I am facing too when I want to do
modifications. I could have a look and try to change that to clarify
the code. I haven't checked, but my feeling is that it should be
possible or even easy. I probably didn't even think about the design
and the consequences of that particular implementation choice when I did
that code.
The indirections, however, are done on purpose and my feeling is that
they are unavoidable to be able to overwrite the code for customization
purposes.
--
Pat
