Re: Request for feedback on texi2any customization file

[Gavin Smith]

On Wed, Aug 30, 2023 at 10:48:19AM +0200, Jonas Hahnfeld wrote:
> On Wed, 2023-08-30 at 00:08 +0100, Gavin Smith wrote:
> > On Tue, Aug 29, 2023 at 10:58:21AM +0200, Jonas Hahnfeld wrote:
> > > 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.
> 
> Not quite, I'm fine with warnings not resulting in errors (even though
> a -Werror like flag would be a nice addition!)

Here's the change I'm looking at currently:

diff --git a/tp/Texinfo/Config.pm b/tp/Texinfo/Config.pm
index 47082541fa..e4847018b2 100644
--- a/tp/Texinfo/Config.pm
+++ b/tp/Texinfo/Config.pm
@@ -128,15 +128,28 @@ my @init_file_loading_messages;
 sub GNUT_load_init_file($) {
   my $file = shift;
   push @init_file_loading_messages, [];
-  eval { require($file) ;};
-  my $e = $@;
-  if ($e ne '') {
-    # $e may not be correctly encoded, but it is not clear what is to
+
+  my $result = do($file);
+  my $message = $@;
+  my $read_error = $!;
+
+  if (!defined($result)) {
+    if (defined($message)) {
+      die sprintf(__("texi2any: error parsing %s: %s"),
+                  _GNUT_decode_input($file), $message);
+    } elsif (defined($read_error)) {
+      die sprintf(__("texi2any: error reading %s: %s"),
+                  _GNUT_decode_input($file), $read_error);
+    }
+  }
+
+  if ($message ne '') {
+    # $message may not be correctly encoded, but it is not clear what is to
     # be expected.  In the eval documentation there is only information
     # on the string eval case, not the block eval used here, and the
     # information is not particularly clear.
     _GNUT_document_warn(sprintf(__("error loading %s: %s"),
-                _GNUT_decode_input($file), $e));
+                _GNUT_decode_input($file), $message));
   }
   my $file_loading_messages = pop @init_file_loading_messages;
   my $error_nr = 0;

We can get more control by using "do" rather than "require".

https://perldoc.perl.org/functions/do

One way to test a warning is with the following input:

$ cat warn.init
$b = "a{x" ; $b =~ s/a{/b/ ; print $b

This gives the warning

Unescaped left brace in regex is passed through in regex; marked by <-- HERE in 
m/a{ <-- HERE / at ./warn.init line 1.

However, this warning does not appear to be captured by the existing code
and passed to _GNUT_document_warn.  It appears only to be bad syntax errors
that get caught.

I'll have to spend more time considering the impact of this change but am
out of time tonight.  It's possible that the call to _GNUT_document_warn
will never happen with this change in which case the code should be
removed.  (I'll also take the program name out of the string.)