Re: Request for feedback on texi2any customization file

[Gavin Smith]

On Tue, Aug 29, 2023 at 10:58:21AM +0200, Jonas Hahnfeld via Bug reports for 
the GNU Texinfo documentation system wrote:
> Dear Texinfo / texi2any developers,
> 
> if some of the experts have time, I would highly appreciate some
> feedback on a texi2any customization file for LilyPond:
> https://gitlab.com/lilypond/lilypond/-/merge_requests/2089
> As a bit of context, LilyPond uses texi2html for its HTML documentation
> since quite some time because it had superior customization
> possibilities when the switch was made from makeinfo to texi2html. With
> texi2html being legacy now, it would of course be nice to transition
> "back" to texi2any.
> 
> A transition to texi2any isn't entirely trivial because LilyPond's
> current customization file for texi2html is around 600 lines of code -
> already after significant cleanups and simplifications. For that reason
> I decided to rewrite the customization file basically from scratch
> (still 550 lines of code), and organize the different parts into
> logical commits which should both make it easier to review right now
> and serve as documentation of sorts in the future.
> 
> A couple of specific questions to start with:
>  * Is it possible to make texi2any fail if there is a syntax error or
> other severe problems in an initialization file? Right now, the 'eval'
> call only seems to emit a warning and just carry on without our
> customization code which was a bit infuriating during development...

This at least should be easy to fix.  It looks like it would a change
like

diff --git a/tp/Texinfo/Config.pm b/tp/Texinfo/Config.pm
index 47082541fa..64cb69c393 100644
--- a/tp/Texinfo/Config.pm
+++ b/tp/Texinfo/Config.pm
@@ -116,7 +116,7 @@ sub _GNUT_document_warn($) {
   return if (texinfo_get_conf('NO_WARN'));
   my $text = shift;
   chomp ($text);
-  warn(_GNUT_encode_message(
+  die(_GNUT_encode_message(
         sprintf(__p("program name: warning: warning_message",
                    "%s: warning: %s"), $real_command_name, $text)."\n"));
 }


Of course, we'd have to decide if this change should be made
unconditionally or if it should be configured somehow.

However, I don't fully understand what the problem is with carrying on.
Could you not test the code with "-c TEXINFO_OUTPUT_FORMAT=parse " just
to see if any messages are printed with the customization code, without
doing the conversion to HTML?


>  * For lilypond_label_target_name, we need to deal with three different
> function signatures if we want to support a range of Texinfo versions:
> four arguments in current master / post-7.0; three arguments in Texinfo
> 7.0; two arguments before Texinfo 7.0. Can we expect this signature
> (and others) to be stable after the release of Texinfo 7.1? (originally
> I thought Texinfo 7.0 already marked a stable API, but here we are...)

Hopefully Patrice will answer this.

I looked at the definition of lilypond_label_target_name in the patch
you linked.  One thing that confuses me is the function signature:

    sub lilypond_label_target_name($$$$) {

If the function only receives three arguments, then this is incorrect.
It's likely the signature isn't being checked.  No signature would be
better:

    sub lilypond_label_target_name {

Or possibly mark the third and fourth arguments as optional:

    sub lilypond_label_target_name($$;$$) {

I haven't had time to look at the following points yet.

>  * Is it possible to format the external reference href without dealing
> with the entire reference in lilypond_format_ref? It seems possible to
> control the manual's directory name using htmlxrefs, but there seems to
> be no hook to control the node file names, ie lower-case them
> (label_target_name seems to be called though). texi2html had a hook to
> customize the external_href and I also see left-overs in older versions
> of texi2any. Would you accept adding such hook again?
>  * What would be the best way to translate some of the extra strings in
> our customization file? Using libintl-perl from a process with LANG=C
> and forcing it to honor documentlanguage seems a pain, at least from my
> experiments and looking at gdt() and _switch_messages_locale() in
> tp/Texinfo/Translations.pm...
> 
> But please, if you see anything in the customization that should not be
> done that way and / or is likely to break in the future, please let me
> know or directly comment on the merge request!
> 
> Thanks in advance!
> Jonas