From 5be949b44affec602aca79b0586ef47229649388 Mon Sep 17 00:00:00 2001 From: Steve Hancock Date: Thu, 5 Aug 2021 07:25:41 -0700 Subject: [PATCH] Fix edge cases of formatting instability, b1187 b1188 --- dev-bin/run_convergence_tests.pl.data | 68 +++++++++++++++++++++++++++ lib/Perl/Tidy/Formatter.pm | 40 +++++++++++++++- lib/Perl/Tidy/VerticalAligner.pm | 19 ++++++-- local-docs/BugLog.pod | 10 ++++ 4 files changed, 132 insertions(+), 5 deletions(-) diff --git a/dev-bin/run_convergence_tests.pl.data b/dev-bin/run_convergence_tests.pl.data index f1461684..6009a0b7 100644 --- a/dev-bin/run_convergence_tests.pl.data +++ b/dev-bin/run_convergence_tests.pl.data @@ -6961,6 +6961,74 @@ sub entaB { --paren-vertical-tightness=2 --space-function-paren +==> b1187.in <== +# S1 +find( + sub { + while( + defined( my $line = <$fh> + ) + ) + { + ...; + } + }, + @argv +); + +# S2 +find( + sub { + while(defined(my $line = <$fh>)) + { + ...; + } + }, + @argv +); + +==> b1187.par <== +--indent-columns=10 +--line-up-parentheses +--maximum-line-length=48 +--nospace-after-keyword='while' +--paren-tightness=2 +--weld-nested-containers + +==> b1188.in <== +# S1 +find( + sub { + while(defined(my $line = <$fh>)) + { + ...; + } + }, + @argv); + +# S2 +find( + sub { + while( + defined( my $line = <$fh> + )) + { + ...; + } + }, + @argv); + +==> b1188.par <== +# added -vertical-tightness flags to b1187 +--indent-columns=10 +--line-up-parentheses +--maximum-line-length=48 +--nospace-after-keyword='while' +--paren-tightness=2 +--paren-vertical-tightness-closing=1 +--paren-vertical-tightness=1 +--weld-nested-containers + ==> b120.in <== # Same as bug96 # State 1 diff --git a/lib/Perl/Tidy/Formatter.pm b/lib/Perl/Tidy/Formatter.pm index ce67f53e..d236e448 100644 --- a/lib/Perl/Tidy/Formatter.pm +++ b/lib/Perl/Tidy/Formatter.pm @@ -21891,11 +21891,17 @@ sub set_vertical_tightness_flags { # [3] valid flag: do not append if this flag is false. Will be # true if appropriate -vt flag is set. Otherwise, Will be # made true only for 2 line container in parens with -lp + # [4] seqno_beg = sequence number of first token of line + # [5] seqno_end = sequence number of last token of line + # [6] min number of lines for joining opening cache, 0=no constraint + # [7] max number of lines for joining opening cache, 0=no constraint # # These flags are used by sub set_leading_whitespace in # the vertical aligner - my $rvertical_tightness_flags = [ 0, 0, 0, 0, 0, 0 ]; + # FIXME: It would be nice to switch to a hash instead of an array + # to clarify the coding. + my $rvertical_tightness_flags = [ 0, 0, 0, 0, 0, 0, 0, 0 ]; # The vertical tightness mechanism can add whitespace, so whitespace can # continually increase if we allowed it when the -fws flag is set. @@ -22025,10 +22031,36 @@ sub set_vertical_tightness_flags { if ($ok) { my $valid_flag = $cvt; + my $min_lines = 0; + my $max_lines = 0; + + # Fix for b1187 and b1188: Blinking can occur if we allow + # welded tokens to re-form into one-line blocks during + # vertical alignment when -lp used. So for this case we + # set the minimum number of lines to be 1 instead of 0. + # The maximum should be 1 if -vtc is not used. If -vtc is + # used, we turn the valid + # flag off and set the maximum to 0. This is equivalent to + # using a large number. + my $seqno_ibeg_next = $type_sequence_to_go[$ibeg_next]; + if ( $rOpts_line_up_parentheses + && $total_weld_count + && $self->is_welded_at_seqno($seqno_ibeg_next) ) + { + $min_lines = 1; + $max_lines = $cvt ? 0 : 1; + $valid_flag = 0; + } + @{$rvertical_tightness_flags} = ( 2, $tightness{$token_next} == 2 ? 0 : 1, - $type_sequence_to_go[$ibeg_next], $valid_flag, + $type_sequence_to_go[$ibeg_next], + $valid_flag, + 0, + 0, + $min_lines, + $max_lines ); } } @@ -22193,6 +22225,10 @@ sub set_vertical_tightness_flags { } $rvertical_tightness_flags->[4] = $seqno_beg; $rvertical_tightness_flags->[5] = $seqno_end; + if ( !defined( $rvertical_tightness_flags->[6] ) ) { + $rvertical_tightness_flags->[6] = 0; + $rvertical_tightness_flags->[7] = 0; + } return $rvertical_tightness_flags; } diff --git a/lib/Perl/Tidy/VerticalAligner.pm b/lib/Perl/Tidy/VerticalAligner.pm index 206fc835..ee804262 100644 --- a/lib/Perl/Tidy/VerticalAligner.pm +++ b/lib/Perl/Tidy/VerticalAligner.pm @@ -485,12 +485,25 @@ sub valign_input { if ($rvertical_tightness_flags) { my $cached_seqno = get_cached_seqno(); if ( $cached_seqno - && $self->group_line_count() <= 1 && $rvertical_tightness_flags->[2] && $rvertical_tightness_flags->[2] == $cached_seqno ) { - $rvertical_tightness_flags->[3] ||= 1; - set_cached_line_valid(1); + + # Fix for b1187 and b1188: Normally this step is only done + # if the number of existing lines is 0 or 1. But to prevent + # blinking, this range can be controlled by the caller. + # If zero values are given we fall back on the range 0 to 1. + my $line_count = $self->group_line_count(); + my $min_lines = $rvertical_tightness_flags->[6]; + my $max_lines = $rvertical_tightness_flags->[7]; + $min_lines = 0 unless ($min_lines); + $max_lines = 1 unless ($max_lines); + if ( ( $line_count >= $min_lines ) + && ( $line_count <= $max_lines ) ) + { + $rvertical_tightness_flags->[3] ||= 1; + set_cached_line_valid(1); + } } } diff --git a/local-docs/BugLog.pod b/local-docs/BugLog.pod index cf543f87..14066e48 100644 --- a/local-docs/BugLog.pod +++ b/local-docs/BugLog.pod @@ -2,6 +2,16 @@ =over 4 +=item B. + +Testing with random parameters produced some cases of instability +involving -wn -lp and several other parameters. The mechanism +involved an interaction between the formatter and vertical aligner. + +This fixes cases b1187 and b1188. + +3 Aug 2021. + =item B. Testing with random parameters produced a case of instability involving -- 2.39.5