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'); > >