branch master updated: * tp/Texinfo/Convert/HTML.pm (output): move js files copying code out of output units formatting loop.

[Patrice Dumas]

This is an automated email from the git hooks/post-receive script.

pertusus pushed a commit to branch master
in repository texinfo.

The following commit(s) were added to refs/heads/master by this push:
     new cab86076a2 * tp/Texinfo/Convert/HTML.pm (output): move js files 
copying code out of output units formatting loop.
cab86076a2 is described below

commit cab86076a2f7d5013d420a7c2fd6e21c86c2dce5
Author: Patrice Dumas <pertusus@free.fr>
AuthorDate: Mon Oct 30 21:29:03 2023 +0100

    * tp/Texinfo/Convert/HTML.pm (output): move js files copying code out
    of output units formatting loop.
    
    * tp/Texinfo/Convert/Converter.pm (output)
    (determine_files_and_directory), tp/Texinfo/Convert/HTML.pm (output):
    make sure that $destination_directory is defined.
    
    * tp/Texinfo/Convert/HTML.pm (_do_jslicenses_file, output): handle the
    case of empty directory string, avoid calling File::Spec->catdir in
    that case.
    
    * tp/Texinfo/Convert/HTML.pm (output): update code used in case there
    are no pages, even though the situation cannot arise. Also add
    comments.
    
    * tp/Texinfo/Convert/HTML.pm (output): simplify units conversion code
    in output().
---
 ChangeLog                       |  28 ++++++--
 tp/Texinfo/Convert/Converter.pm |  29 ++++----
 tp/Texinfo/Convert/HTML.pm      | 146 ++++++++++++++++++++--------------------
 3 files changed, 113 insertions(+), 90 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6315e93ceb..f627e4dad1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,23 @@
+2023-10-30  Patrice Dumas  <pertusus@free.fr>
+
+       * tp/Texinfo/Convert/HTML.pm (output): move js files copying code out
+       of output units formatting loop.
+
+       * tp/Texinfo/Convert/Converter.pm (output)
+       (determine_files_and_directory), tp/Texinfo/Convert/HTML.pm (output):
+       make sure that $destination_directory is defined.
+
+       * tp/Texinfo/Convert/HTML.pm (_do_jslicenses_file, output): handle the
+       case of empty directory string, avoid calling File::Spec->catdir in
+       that case.
+
+       * tp/Texinfo/Convert/HTML.pm (output): update code used in case there
+       are no pages, even though the situation cannot arise. Also add
+       comments.
+
+       * tp/Texinfo/Convert/HTML.pm (output): simplify units conversion code
+       in output().
+
 2023-10-30  Patrice Dumas  <pertusus@free.fr>
 
        * tp/Texinfo/XS/convert/convert_html.c (html_convert_tree): rename
@@ -166,7 +186,7 @@
        (expand_cmd_args_to_texi), tp/Texinfo/XS/main/translations.c
        (substitute_element_array), tp/Texinfo/XS/parsetexi/parser.c
        (new_value_element): use brace_command_arg to hold the flag text in
-       the first contents for @value and @txiinternalvalue. 
+       the first contents for @value and @txiinternalvalue.
 
 2023-10-29  Patrice Dumas  <pertusus@free.fr>
 
@@ -408,7 +428,7 @@
        * tp/Texinfo/XS/Makefile.am (libtexinfoxs_la_SOURCES),
        tp/Texinfo/XS/main/call_perl_function.h: add call_perl_function.h file
        for a more common code, also given that these functions should
-       probably only called from the HTML converter. 
+       probably only called from the HTML converter.
 
 2023-10-28  Patrice Dumas  <pertusus@free.fr>
 
@@ -827,7 +847,7 @@
        (html_css_string_formatting_reference_names),
        tp/Texinfo/XS/main/utils.h (HTML_FORMATTING_REFERENCES_LIST)
        (FORMATTING_REFERENCE, CONVERTER):
-       pass formatting_function and default formatting references to 
+       pass formatting_function and default formatting references to
        converter initialization.
 
        * tp/Texinfo/Convert/HTML.pm (convert): setup simpletitle_tree
@@ -862,7 +882,7 @@
        (html_translate_names), tp/Texinfo/XS/convert/convert_html.c
        (html_prepare_conversion_units_targets): rename translate_names as
        html_translate_names, add a specific XS interface and remove from
-       html_prepare_conversion_units_targets. 
+       html_prepare_conversion_units_targets.
 
        * tp/Texinfo/Convert/HTML.pm (_sort_index_entries): reindent, comment
        out unused code.
diff --git a/tp/Texinfo/Convert/Converter.pm b/tp/Texinfo/Convert/Converter.pm
index e634282d20..27049c3e2d 100644
--- a/tp/Texinfo/Convert/Converter.pm
+++ b/tp/Texinfo/Convert/Converter.pm
@@ -1,20 +1,20 @@
 # Converter.pm: Common code for Converters.
 #
 # Copyright 2011-2023 Free Software Foundation, Inc.
-# 
+#
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
 # the Free Software Foundation; either version 3 of the License,
 # or (at your option) any later version.
-# 
+#
 # This program is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
-# 
+#
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
-# 
+#
 # Original author: Patrice Dumas <pertusus@free.fr>
 
 package Texinfo::Convert::Converter;
@@ -329,7 +329,7 @@ sub output($$)
     if ($output_file ne '') {
       if ($self->get_conf('SPLIT')) {
         my $top_node_file_name = $self->top_node_filename($document_name);
-        if (defined($destination_directory) and $destination_directory ne '') {
+        if ($destination_directory ne '') {
           $outfile_name = File::Spec->catfile($destination_directory,
                                               $top_node_file_name);
         } else {
@@ -387,7 +387,7 @@ sub output($$)
     print STDERR "DO Elements with filenames\n"
       if ($self->get_conf('DEBUG'));
     my %files_filehandle;
-    
+
     foreach my $output_unit (@$output_units) {
       my $output_unit_filename = $output_unit->{'unit_filename'};
       my $out_filepath = $self->{'out_filepaths'}->{$output_unit_filename};
@@ -754,13 +754,14 @@ sub determine_files_and_directory($;$)
     # $output_file_filename is not used, but $output_filename should be
     # the same as long as $output_file is the same as $output_filepath
     # which is the case except if $output_file is ''.
+    # Note that fileparse may return a string for the directory part even
+    # for a relative file without directory, ie
+    # myfile.html -> $output_dir = './'
+    # In that case the $destination_directory will never be ''.
     my ($output_file_filename, $output_dir, $suffix) = fileparse($output_file);
-    if ($output_dir ne '') {
-      $destination_directory = $output_dir;
-    }
+    $destination_directory = $output_dir;
   }
-  if (defined($destination_directory)
-      and $destination_directory ne '') {
+  if ($destination_directory ne '') {
     $destination_directory = File::Spec->canonpath($destination_directory);
   }
   return ($output_file, $destination_directory, $output_filename,
@@ -1783,7 +1784,7 @@ sub xml_accent($$$;$$$)
   my $in_upper_case = shift;
   my $use_numeric_entities = shift;
   my $accent = $command->{'cmdname'};
-  
+
   if ($in_upper_case and $text =~ /^\w$/) {
     $text = uc ($text);
   }
@@ -1801,7 +1802,7 @@ sub xml_accent($$$;$$$)
       return $text;
     }
   }
- 
+
   if ($use_numeric_entities) {
     my $formatted_accent = xml_numeric_entity_accent($accent, $text);
     if (defined($formatted_accent)) {
@@ -1850,7 +1851,7 @@ sub xml_accents($$;$)
   } else {
     $format_accents = \&xml_accent;
   }
-  
+
   return $self->convert_accents($accent, $format_accents,
                                 $self->get_conf('OUTPUT_CHARACTERS'),
                                 $in_upper_case);
diff --git a/tp/Texinfo/Convert/HTML.pm b/tp/Texinfo/Convert/HTML.pm
index 688fd65e58..84b153ab99 100644
--- a/tp/Texinfo/Convert/HTML.pm
+++ b/tp/Texinfo/Convert/HTML.pm
@@ -10945,8 +10945,12 @@ sub _do_jslicenses_file {
 __("cannot use absolute path or URL `%s' for JS_WEBLABELS_FILE when generating 
web labels file"), $path));
     return;
   }
-  my $license_file = File::Spec->catdir($destination_directory,
-                                        $path);
+  my $license_file;
+  if ($destination_directory ne '') {
+    $license_file = File::Spec->catdir($destination_directory, $path);
+  } else {
+    $license_file = $path;
+  }
   # sequence of bytes
   my ($licence_file_path, $path_encoding)
      = $self->encoded_output_file_name($license_file);
@@ -11622,22 +11626,17 @@ sub output($$)
   if (!$output_units
       or !defined($output_units->[0]->{'unit_filename'})) {
     # no page
+    # NOTE there are always output units, and there is always a file
+    # associated, so this situation cannot happen.
     if ($output_file ne '') {
       my $no_page_output_filename;
-      my $no_page_out_filepath;
       if ($self->get_conf('SPLIT')) {
         $no_page_output_filename = $self->top_node_filename($document_name);
-        if (defined($destination_directory) and $destination_directory ne '') {
-          $no_page_out_filepath = File::Spec->catfile($destination_directory,
-                                                    $no_page_output_filename);
-        } else {
-          $no_page_out_filepath = $no_page_output_filename;
-        }
+        $self->set_file_path($no_page_output_filename, $destination_directory);
       } else {
-        $no_page_out_filepath = $output_file;
         $no_page_output_filename = $output_filename;
+        $self->set_file_path($no_page_output_filename, undef, $output_file);
       }
-      $self->{'out_filepaths'}->{$no_page_output_filename} = 
$no_page_out_filepath;
 
       $self->{'current_filename'} = $no_page_output_filename;
     } else {
@@ -11657,6 +11656,9 @@ sub output($$)
 
   if (!$output_units
       or !defined($output_units->[0]->{'unit_filename'})) {
+    # NOTE there are always output units, and there is always a file
+    # associated, so this situation cannot happen.
+    # A lot of unreachable code here...
     my $output = '';
     my $fh;
     my $encoded_no_page_out_filepath;
@@ -11724,7 +11726,7 @@ sub output($$)
     return $output if ($output_file eq '');
   } else {
     # output with pages
-    print STDERR "DO Elements with filenames\n"
+    print STDERR "DO Units with filenames\n"
       if ($self->get_conf('DEBUG'));
     my %files;
 
@@ -11733,31 +11735,24 @@ sub output($$)
     # in turn
     $special_units = [] if (!defined($special_units));
     foreach my $output_unit (@$output_units, @$special_units) {
+      $unit_nr++;
+
       my $output_unit_filename = $output_unit->{'unit_filename'};
-      my $out_filepath = $self->{'out_filepaths'}->{$output_unit_filename};
       $self->{'current_filename'} = $output_unit_filename;
 
-      $unit_nr++;
-      # First do the special pages, to avoid outputting these if they are
-      # empty.
-      my $special_unit_content;
-      if (defined($output_unit->{'unit_type'})
-          and $output_unit->{'unit_type'} eq 'special_unit') {
+
+      # convert body before header in case this affects the header
+      # and, for special output unit, to avoid outputting anything if empty.
+      my $body;
+      if ($output_unit->{'unit_type'} eq 'special_unit') {
         print STDERR "\nUNIT SPECIAL $output_unit->{'special_unit_variety'}\n"
            if ($self->get_conf('DEBUG'));
-        $special_unit_content
-                  .= $self->convert_output_unit($output_unit,
-                                                "output s-unit $unit_nr");
-        if ($special_unit_content eq '') {
+        $body = $self->convert_output_unit($output_unit,
+                                           "output s-unit $unit_nr");
+        if ($body eq '') {
           $self->{'file_counters'}->{$output_unit_filename}--;
-          next ;
+          next;
         }
-      }
-
-      # convert body before header in case this affects the header
-      my $body = '';
-      if (defined($special_unit_content)) {
-        $body = $special_unit_content;
       } else {
         print STDERR "\nUNIT $unit_nr\n" if ($self->get_conf('DEBUG'));
         $body = $self->convert_output_unit($output_unit,
@@ -11769,12 +11764,13 @@ sub output($$)
       # elements have been converted.
       if (!exists($files{$output_unit_filename})) {
         $files{$output_unit_filename} = {'first_unit' => $output_unit,
-                                     'body' => ''};
+                                         'body' => ''};
       }
       $files{$output_unit_filename}->{'body'} .= $body;
       $self->{'file_counters'}->{$output_unit_filename}--;
 
       if ($self->{'file_counters'}->{$output_unit_filename} == 0) {
+        my $out_filepath = $self->{'out_filepaths'}->{$output_unit_filename};
         my $file_output_unit = $files{$output_unit_filename}->{'first_unit'};
         my ($encoded_out_filepath, $path_encoding)
           = $self->encoded_output_file_name($out_filepath);
@@ -11812,50 +11808,56 @@ sub output($$)
       }
     }
     delete $self->{'current_filename'};
-    if ($self->get_conf('INFO_JS_DIR')) {
-      my $jsdir = File::Spec->catdir($destination_directory,
-                                     $self->get_conf('INFO_JS_DIR'));
-      if (!-d $jsdir) {
-        if (-f $jsdir) {
-          $self->document_error($self,
-            sprintf(__("%s already exists but is not a directory"), $jsdir));
+  }
+
+  if ($self->get_conf('INFO_JS_DIR')) {
+    my $info_js_dir = $self->get_conf('INFO_JS_DIR');
+    my $jsdir;
+    if ($destination_directory ne '') {
+      $jsdir = File::Spec->catdir($destination_directory, $info_js_dir);
+    } else {
+      $jsdir = $info_js_dir;
+    }
+    if (!-d $jsdir) {
+      if (-f $jsdir) {
+        $self->document_error($self,
+          sprintf(__("%s already exists but is not a directory"), $jsdir));
+      } else {
+        mkdir $jsdir;
+      }
+    }
+    # Copy JS files.
+    if (-d $jsdir) {
+     if (!$self->get_conf('TEST')) {
+        my $jssrcdir;
+        if (!$Texinfo::ModulePath::texinfo_uninstalled) {
+          $jssrcdir = File::Spec->catdir(
+            $Texinfo::ModulePath::pkgdatadir, 'js');
         } else {
-          mkdir $jsdir;
+          $jssrcdir = File::Spec->catdir(
+            $Texinfo::ModulePath::top_srcdir, 'js');
         }
-      }
-      # Copy JS files.
-      if (-d $jsdir) {
-        if (!$self->get_conf('TEST')) {
-          my $jssrcdir;
-          if (!$Texinfo::ModulePath::texinfo_uninstalled) {
-            $jssrcdir = File::Spec->catdir(
-              $Texinfo::ModulePath::pkgdatadir, 'js');
-          } else {
-            $jssrcdir = File::Spec->catdir(
-              $Texinfo::ModulePath::top_srcdir, 'js');
-          }
-          for my $f ('info.js', 'modernizr.js', 'info.css') {
-            my $from = File::Spec->catfile($jssrcdir, $f);
+        for my $f ('info.js', 'modernizr.js', 'info.css') {
+          my $from = File::Spec->catfile($jssrcdir, $f);
 
-            if (!copy($from, $jsdir)) {
-              $self->document_error($self,
-                sprintf(__("error on copying %s into %s"), $from, $jsdir));
-            }
+          if (!copy($from, $jsdir)) {
+            $self->document_error($self,
+              sprintf(__("error on copying %s into %s"), $from, $jsdir));
           }
-        } else {
-        # create empty files for tests to keep results stable.
-          for my $f ('info.js', 'modernizr.js', 'info.css') {
-            my $filename = File::Spec->catfile($jsdir, $f);
-            if (!open (FH, '>', $filename)) {
-              $self->document_error($self,
-                sprintf(__("error on creating empty %s: %s"),
-                        $filename, $!));
-            }
-            if (!close(FH)) {
-              $self->document_error($self,
-                sprintf(__("error on closing empty %s: %s"),
-                        $filename, $!));
-            }
+        }
+      } else {
+      # create empty files for tests to keep results stable.
+        for my $f ('info.js', 'modernizr.js', 'info.css') {
+          my $filename = File::Spec->catfile($jsdir, $f);
+          if (!open (FH, '>', $filename)) {
+            $self->document_error($self,
+              sprintf(__("error on creating empty %s: %s"),
+                      $filename, $!));
+          }
+          if (!close(FH)) {
+            $self->document_error($self,
+              sprintf(__("error on closing empty %s: %s"),
+                      $filename, $!));
           }
         }
       }
@@ -11992,7 +11994,7 @@ sub output($$)
           = 
&{$self->formatting_function('format_node_redirection_page')}($self,
                                                                
$target_element);
         my $out_filename;
-        if (defined($destination_directory) and $destination_directory ne '') {
+        if ($destination_directory ne '') {
           $out_filename = File::Spec->catfile($destination_directory,
                                               $redirection_filename);
         } else {