Re: footer navigation headers

[Gavin Smith]

On Sun, Feb 14, 2021 at 11:31:21PM +0000, Gavin Smith wrote:
> On Sun, Feb 14, 2021 at 12:32:58PM -0800, Per Bothner wrote:
> > A related problem is that the end-of-page header is relative to the
> > final @subsection (and duplicates that internal header at the start
> > of the @subsection).  In my opinion, the header should point to
> > the next/previous/up of the @section (or @chapter for a chapter
> > without sections).  This is more logical, I think, whether or not
> > the @subsecton navigation headers are displayed.
> 
> Yes, I think this makes sense for --split=section.  Where there is more
> than one node per output file it makes no sense to have a navigation
> bar at the very end of the file that is relative to the last node only.
> I think there are two sensible possibilities:  A navigation bar at
> the top and possibly the bottom of the page relative to the @section,
> or a navigation bar at the beginning of each node in the page.

I'm typing what I did to try to fix this as I do it to try to help
other people learn how to fix similar problems like this.  (You shouldn't
assume it's quick or easy for me to fix.)

I looked through the code and couldn't see what the problem could
be.

I created a minimal failing example:

\input texinfo  @c -*-texinfo-*-

@node Top
@top top

@node chap1
@chapter chap1

@node sec1
@section sec1

@node chap2
@chapter chap2

@node  sec2
@section sec2

@bye

Procesing with ./texi2any.pl --html --split=chapter test.texi
gave spurious warnings:

test.texi:9: warning: node `sec1' unreferenced
test.texi:15: warning: node `sec2' unreferenced

The output chap1.html had the problem you mentioned, with a link to chap1
at the end of the file.

The next step was to find where this text was output.  I inserted print
statements into parts of the code and ran the program several times
to confirm this.  It is useful to use the print_tree function to check
some structures.  This showed that the problem appeared before
_default_format_element_footer was called, as the $element argument
was the section, and not the chapter.  I had to confirm where it was
being called, which I did by inserting more print statements.  I found
that _default_format_element_footer was called several times and the
footer was only output for some of them (when $end_page was set in
that function).  It showed that I didn't understand what
_convert_element_type was doing, as it looked like it was running once
for each node, not for each output file (the comment at the top of
the function, which I am sure that I wrote to help me to understand the
code, says otherwise).

I then had to think about the code to understand it more.
_convert_element_type is not called directly anywhere.  The comment on
_convert_element_type said to look at _prepare_elements.  That doesn't
seem to deal with chapter-level splitting.  Evidently "element" didn't
mean what I thought it did.

I looked at where _prepare_elements is called.  It is called from output
where there is other handling of split options.  I looked down in
that function to see where the "elements" would be converted.  After
looking at the wrong code for a while, I saw where the case of split
output was handled.  It appears that each "element" (i.e. node) is
converted independently, and then sent to the output file (line 7596).

I saw there was a "format_end_file" hook being called.  This took me
to _default_format_end_file (the recent renaming of these functions
really helps me get about in the file using the "*" command from the
Vim editor (although I use Kakoune now)).  This would be the natural
place to output the final navigation panel.  I had a look again at
the emails in the thread on this topic feeling that somebody had said
something about this.

I was ready now to attempt changing the code.  I would have to
be careful about the different configuration options (split v. non-split
etc.).  I added an argument to the function, $supreme_element.  Now
I went looking for code I could copy and paste to use this argument.
I thought of calling _default_format_element_footer directly but had
the thought that this function might be called twice for the top-level
element and it wouldn't be able to distinguish between them.  At this
point I took a break.

I thought that perhaps we could just call _default_format_element_footer
once from the new location, as that function had a lot of logic in it.
To minimize disruption, I thought that I could at first pass the wrong
value of $supreme_element to it (the last element in a file, not the
first).

I commented out the existing call to &{$self->{'format_element_footer'}}
and copied the code to _default_format_end_file.  I saw there were
$type and $content variables which weren't available in the new location.
Checking, I saw the $type parameter wasn't used at all.  The $content
was used for the WORDS_IN_PAGE feature.  This feature has confused many
people in the past, and I went back to my old emails to see people talking
about this.  I thought for a first implementation, I would just enable
the footer unconditionally (everything that's been output to that
output file not being easily available - the existing code counting the
words in the last node only, I assume).

Although trying to make minimally intrusive changes to the code, the
changes were piling up.  I did a "git diff" to review.  After I removed
old debugging print statements (with "git checkout --patch" (in fact, I
have a bash alias for this, "GCO -p")), the changes were as follows:

ff --git a/tp/Texinfo/Convert/HTML.pm b/tp/Texinfo/Convert/HTML.pm
index 2b6929b3a..b0bca385c 100644
--- a/tp/Texinfo/Convert/HTML.pm
+++ b/tp/Texinfo/Convert/HTML.pm
@@ -4744,8 +4744,8 @@ sub _convert_element_type($$$$)
     }
   }
   $result .= $content unless ($special_element);
-  $result .= &{$self->{'format_element_footer'}}($self, $type, 
-                                                 $element, $content);
+  # $result .= &{$self->{'format_element_footer'}}($self, $type, 
+  #                                                $element, $content);
   return $result;
 }
 
@@ -4803,14 +4803,14 @@ sub _default_format_element_footer($$$$)
   } elsif ($self->get_conf('SPLIT') eq 'node') {
     if ($self->get_conf('HEADERS')) {
       my $no_footer_word_count;
-      if ($self->get_conf('WORDS_IN_PAGE')) {
+      if (0 or $self->get_conf('WORDS_IN_PAGE')) {
         my @cnt = split(/\W*\s+\W*/, $content);
         if (scalar(@cnt) < $self->get_conf('WORDS_IN_PAGE')) {
           $no_footer_word_count = 1;
         }
       }
-      $buttons = $self->get_conf('NODE_FOOTER_BUTTONS')
-         unless ($no_footer_word_count);
+      $buttons = $self->get_conf('NODE_FOOTER_BUTTONS');
+         #unless ($no_footer_word_count);
     }
   } else {
     $maybe_in_page = 1;
@@ -6534,9 +6534,16 @@ sub _default_format_program_string($)
   }
 }
 
-sub _default_format_end_file($)
+sub _default_format_end_file($;$)
 {
   my $self = shift;
+  my $supreme_element = shift;
+
+  if ($supreme_element) {
+    $result .= &{$self->{'format_element_footer'}}($self, undef,
+                                                   $supreme_element, undef);
+  }
+
   my $closing_sections_text = join('', 
$self->close_registered_sections_level(0));
   my $program_text = '';
   if ($self->get_conf('PROGRAM_NAME_IN_FOOTER')) {

I still hadn't dealt with passing $supreme_element to
_default_format_end_file.  I added this to the call to
&{$self->{'format_end_file'}} (using the most recent node processed),
and then figured it was time to try running the texi2any test suite,
hoping that there weren't too many changes.

The quickest way, I've found, is to run "maintain/all_tests.sh generate"
followed by "git diff".

This led to syntax errors which I fixed (there was no such variable as
$result in _default_format_end_file).

Unfortunately, this led to a massive amount of changes in the test results.
At this point, I was out of steam for the evening.

If/when I come back to this, I'll be investigating why <hr> disappeared
from the output in many places, before looking at the other changes in
the test results.