On Sat, Jul 05, 2025 at 01:06:29AM +0100, Gavin Smith wrote:
> On Fri, Jul 04, 2025 at 10:18:05PM +0100, Gavin Smith wrote:
> > All these nodes (in the Sphinx output) have explicit pointers on the
> > node lines, just like the affected part of the libc manual.
> > 
> > So I expect we will have to take some account of node pointers
> > when generating these warnings.  (We could still give 1 or 2 warnings if
> > there is a simple way of doing so, but not 197.)
> 
> Here's my current version (done in both Perl and C).  Only the first block
> of code is new in each file; the second ("check consistency..."") is
> just moved from later in the function.
> 
> As with the existing warnings, node pointers can be used to get the next
> expected node to appear in a menu.
> 
> I just need to check through all the changes in the test suite to check
> if they are satisfactory and tidy up the patch a bit.  I probably need
> one or two more test cases for explicit node pointers.
> 
> 
> diff --git a/tta/perl/Texinfo/Structuring.pm b/tta/perl/Texinfo/Structuring.pm
> index 5f79484786..5345b28026 100644
> --- a/tta/perl/Texinfo/Structuring.pm
> +++ b/tta/perl/Texinfo/Structuring.pm
> @@ -844,12 +844,133 @@ sub check_node_tree_menu_structure($)
>      }
>    }
>  
> +  # Go through all the menus and check if they match subordinate
> +  # nodes.
> +  if ($customization_information->get_conf('CHECK_NORMAL_MENU_STRUCTURE')) {
> +    foreach my $node_relations (@{$nodes_list}) {
> +      if ($node_relations->{'menus'}) {
> +        next if !$node_relations->{'associated_section'};
> +        my $section_childs = $node_relations->{'associated_section'}
> +                                            ->{'section_childs'};
> +        next if !defined($section_childs) or !@{$section_childs};
> +

In the following code, and also maybe below in the main loop, I think
that the next section should be used instead of going out of the loop.
That way, a structure like the following will have checks:

@node chap
@chapter Chap

@menu
* with node::
* ...
@end menu

@section no node

@node with node
@section a section with node

....

> +        my $first_child_node_relations = 
> $section_childs->[0]->{'associated_node'};
> +        next if !defined($first_child_node_relations);
> +
> +        # Set to the first subordinate section, which should appear first in 
> the
> +        # menu.
> +        my ($section_node, $last_menu_node_relations);
> +        $section_node = $first_child_node_relations->{'element'};
> +        next if !defined($section_node);
> +
> +        foreach my $menu (@{$node_relations->{'menus'}}) {
> +          # Loop through each each entry in the menu and
> +          # check if it is the menu entry we were expecting
> +          # to see based on what came before.
> +          MENU_CONTENT:
> +          foreach my $menu_content (@{$menu->{'contents'}}) {
> +            next if !defined($menu_content->{'type'})
> +              or $menu_content->{'type'} ne 'menu_entry';
> +
> +            my $menu_node;
> +            foreach my $content (@{$menu_content->{'contents'}}) {
> +              next if $content->{'type'} ne 'menu_entry_node';
> +              if ($content->{'extra'}) {
> +                if (!$content->{'extra'}->{'manual_content'}) {
> +                  if (defined($content->{'extra'}->{'normalized'})) {
> +                    my $menu_node_name = $content->{'extra'}->{'normalized'};
> +                    $menu_node = $identifier_target->{$menu_node_name};
> +                  }
> +                }
> +              }
> +              last; # menu_entry_node found
> +            }
> +            next MENU_CONTENT if !defined($menu_node)
> +              or !defined($menu_node->{'extra'})
> +              or !defined($menu_node->{'extra'}->{'node_number'});
> +
> +            my $menu_node_element_number = 
> $menu_node->{'extra'}->{'node_number'};
> +
> +            if (!defined($section_node)) {
> +              $registrar->line_warn(
> +                sprintf(__("unexpected node `%s' in menu"),
> +                  target_element_to_texi_label($menu_node)),
> +                $menu_content->{'source_info'}, 0,
> +                $customization_information->get_conf('DEBUG'));
> +              $node_errors{$menu_node_element_number} = 1;
> +            } elsif ($menu_node ne $section_node) {
> +              my $next_pointer_node;
> +              if ($last_menu_node_relations) {
> +                # If there are explicit node pointers, also allow
> +                # the "next" node.
> +                my $last_menu_node = $last_menu_node_relations->{'element'};
> +                my $arguments_line = $last_menu_node->{'contents'}->[0];
> +                my $automatic_directions
> +                  = (not (scalar(@{$arguments_line->{'contents'}}) > 1));
> +                if (!$automatic_directions) {
> +                  my $last_menu_node_directions = 
> $last_menu_node_relations->{'node_directions'};
> +                  $next_pointer_node = $last_menu_node_directions->{'next'};
> +                }
> +              }
> +
> +              if (!defined($next_pointer_node)
> +                  or $next_pointer_node ne $menu_node) {
> +                $registrar->line_warn(
> +                  sprintf(__("node `%s' in menu where `%s' expected"),
> +                    target_element_to_texi_label($menu_node),
> +                    target_element_to_texi_label($section_node)),
> +                  $menu_content->{'source_info'}, 0,
> +                  $customization_information->get_conf('DEBUG'));
> +                $node_errors{$menu_node_element_number} = 1;
> +              }
> +            }
> +
> +            # Now set section_node for the section that is
> +            # expected to follow the current menu node.
> +
> +            $last_menu_node_relations
> +              = $nodes_list->[$menu_node_element_number - 1];
> +            next MENU_CONTENT if
> +              !$last_menu_node_relations
> +                or 
> !defined($last_menu_node_relations->{'associated_section'});
> +
> +            my $menu_section_dirs
> +              = $last_menu_node_relations->{'associated_section'}
> +                  ->{'section_directions'};
> +
> +            next MENU_CONTENT
> +              if !defined($menu_section_dirs)
> +                 or !defined($menu_section_dirs->{'up'})
> +                 or 
> !defined($menu_section_dirs->{'up'}->{'associated_node'});
> +
> +            my $menu_node_up = 
> $menu_section_dirs->{'up'}->{'associated_node'};
> +            if ($menu_node_up ne $node_relations) {
> +              # Keep the same expected section as the current menu node
> +              # is misplaced.

Here, it would be better to skip over section not associated to nodes, I
believe.

> +            } elsif (defined($menu_section_dirs->{'next'})
> +                and 
> defined($menu_section_dirs->{'next'}->{'associated_node'})
> +                and defined($menu_section_dirs->{'next'}->{'associated_node'}
> +                              ->{'element'})) {
> +              $section_node = 
> $menu_section_dirs->{'next'}->{'associated_node'}
> +                                ->{'element'};
> +            } else {
> +              # We reached the last subordinate section so no more menu
> +              # entries are expected.
> +              undef $section_node;
> +            }
> +          }
> +        }
> +      }
> +    }
> +  }
> +
>    my %cached_menu_nodes;
>  
>    # check for node up / menu up mismatch
>    if ($customization_information->get_conf('CHECK_MISSING_MENU_ENTRY')) {
>      foreach my $node_relations (@{$nodes_list}) {
>        my $node = $node_relations->{'element'};
> +      next if $node_errors{$node->{'extra'}->{'node_number'}};
>  
>        my $section_relations = $node_relations->{'associated_section'};
>        next if !defined($section_relations);
> @@ -906,6 +1027,54 @@ sub check_node_tree_menu_structure($)
>      }
>    }
>  
> +  if ($customization_information->get_conf('CHECK_NORMAL_MENU_STRUCTURE')) {
> +    foreach my $node_relations (@{$nodes_list}) {
> +      my $node = $node_relations->{'element'};
> +      next if $node_errors{$node->{'extra'}->{'node_number'}};
> +      next if $node->{'extra'}->{'normalized'} eq 'Top';
> +
> +      my $node_directions = $node_relations->{'node_directions'};
> +
> +      my $arguments_line = $node->{'contents'}->[0];
> +      my $automatic_directions
> +        = (not (scalar(@{$arguments_line->{'contents'}}) > 1));
> +
> +      my $menu_directions = $node_relations->{'menu_directions'};
> +      # check consistency between explicit node pointer and
> +      # node entries menu order
> +      if (!$automatic_directions) {
> +        if ($node_directions and $menu_directions) {
> +          foreach my $direction (@node_directions_names) {
> +            if ($node_directions->{$direction}
> +                and not $node_directions->{$direction}
> +                      ->{'extra'}->{'manual_content'}
> +                and $menu_directions->{$direction}
> +                and $menu_directions->{$direction}
> +                  ne $node_directions->{$direction}
> +                and not $menu_directions->{$direction}
> +                              ->{'extra'}->{'manual_content'}) {
> +              $registrar->line_warn(
> +             sprintf(__("node %s pointer for `%s' is `%s' but %s is `%s' in 
> menu"),
> +                    $direction,
> +                    target_element_to_texi_label($node),
> +                    target_element_to_texi_label(
> +                        $node_directions->{$direction}),
> +                    $direction,
> +                    target_element_to_texi_label(
> +                        $menu_directions->{$direction})),
> +                                    $node->{'source_info'}, 0,
> +                             $customization_information->get_conf('DEBUG'));
> +            }
> +          }
> +        }
> +      }
> +    }
> +  }
> +
> +  return;
> +
> +  ###################################################################
> +
>    # Any problems with 'up' direction should have been found by previous code.
>    #my @checked_node_directions_names = ('next', 'prev', 'up');
>    my @checked_node_directions_names = ('next', 'prev');
> 
> 

Reply via email to