From: Steve Hancock Date: Wed, 23 Dec 2020 22:36:45 +0000 (-0800) Subject: updated to an improved list marking method X-Git-Tag: 20210111~29 X-Git-Url: https://git.donarmstrong.com/?a=commitdiff_plain;h=d97c216dbcbff1d0670efabf30727cda2794d432;p=perltidy.git updated to an improved list marking method --- diff --git a/lib/Perl/Tidy.pm b/lib/Perl/Tidy.pm index f358a081..0ef55091 100644 --- a/lib/Perl/Tidy.pm +++ b/lib/Perl/Tidy.pm @@ -1,5 +1,5 @@ # -###########################################################- +########################################################### # # perltidy - a perl script indenter and formatter # diff --git a/lib/Perl/Tidy/VerticalAligner.pm b/lib/Perl/Tidy/VerticalAligner.pm index b8898442..ed0ae56e 100644 --- a/lib/Perl/Tidy/VerticalAligner.pm +++ b/lib/Perl/Tidy/VerticalAligner.pm @@ -340,7 +340,7 @@ use constant DEBUG_VALIGN => 0; # Flag for use during conversion to using new $list_seqno # to identify lists of items. The flag 'is_forced_break' will # be removed when conversion is complete. -use constant TEST_LIST_SEQNO => 0; +use constant TEST_OLD_LIST => 0; sub valign_input { @@ -717,7 +717,7 @@ EOM # Decide if this is a simple list of items. # We use this to be less restrictive in deciding what to align. # -------------------------------------------------------------------- - if (TEST_LIST_SEQNO) { + if ( !TEST_OLD_LIST ) { decide_if_list($new_line) if ($list_seqno); } else { @@ -1549,6 +1549,11 @@ sub _flush_group_lines { my $imax_pair = $line_1->get_imax_pair(); if ( $imax_pair > $imax_align ) { $imax_align = $imax_pair } + ## flag for possible future use: + ## my $is_isolated_pair = $imax_pair < 0 + ## && ( $jbeg == 0 + ## || $rall_lines->[ $jbeg - 1 ]->get_imax_pair() < 0 ); + my ( $is_marginal, $imax_align_fix ) = is_marginal_match( $line_0, $line_1, $grp_level, $imax_align ); if ($is_marginal) { @@ -1739,6 +1744,64 @@ EOM } } +sub two_line_pad { + + my ( $line_m, $line, $imax_min ) = @_; + + # Given: + # two isolated (list) lines + # imax_min = number of common alignment tokens + # Return: + # $pad_max = maximum suggested pad distnce + # = 0 if alignment not recommended + # Method: + # Note that this is only for two lines which do not have alignment tokens + # in common with any other lines. It is intended for lists, but it might + # also be used for two non-list lines with a common leading '='. + + # Use a minimum pad distance not less than the minimum of two mean + # field lengths. The idea is to avoid aligning lines with very + # different field lengths, like these two: + # [ + # 'VARCHAR', DBI::SQL_VARCHAR, undef, "'", "'", undef, 0, 1, + # 1, 0, 0, 0, undef, 0, 0 + # ]; + my $short_pad = 4; + my $rfield_lengths = $line->get_rfield_lengths(); + my $rfield_lengths_m = $line_m->get_rfield_lengths(); + + # Safety check + my $jmax_m = $line_m->get_jmax(); + my $jmax = $line->get_jmax(); + if ( $imax_min > $jmax_m - 2 ) { $imax_min = $jmax_m - 2 } + if ( $imax_min > $jmax - 2 ) { $imax_min = $jmax - 2 } + + my $avg = 0; + my $avg_m = 0; + for ( my $i = 0 ; $i <= $imax_min ; $i++ ) { + $avg_m += $rfield_lengths_m->[$i]; + $avg += $rfield_lengths->[$i]; + } + $avg /= ( $imax_min + 1 ); + $avg_m /= ( $imax_min + 1 ); + my $avg_min = $avg <= $avg_m ? $avg : $avg_m; + + my $pad_max = $short_pad; + if ( $pad_max < $avg_min ) { $pad_max = $avg_min } + my $pad_first = $rfield_lengths->[0] - $rfield_lengths_m->[0]; + + # Usually we allow pads to occur wherever they can go, but for two + # list lines we will not align if the first pad is too large. This + # can avoid some poor alignments of two lines. For example, this is + # not so good: + # [ + # 0, 'OPpLVREF_SV', 'SV', 1, 'OPpLVREF_AV', 'AV', 2, + # 'OPpLVREF_HV', 'HV', 3, 'OPpLVREF_CV', 'CV', + # ]; + $pad_max = 0 if ( $pad_first < -$pad_max || $pad_first > $pad_max ); + return $pad_max; +} + sub sweep_left_to_right { my ( $rlines, $rgroups, $group_level ) = @_; @@ -1835,7 +1898,7 @@ sub sweep_left_to_right { if ( $jend == $jbeg && $jend_m == $jbeg_m - && !( $list_type && !TEST_LIST_SEQNO ) + && !( $list_type && TEST_OLD_LIST ) && ( $ng == 1 || $istop_mm < 0 ) && ( $ng == $ng_max || $istop < 0 ) && !$line->get_j_terminal_match() @@ -1849,16 +1912,20 @@ sub sweep_left_to_right { { # We will just align assignments and simple lists + next unless ( $imax_min >= 0 ); next - unless ( $imax_min >= 0 && $rtokens->[0] =~ /^=\d/ - || ( TEST_LIST_SEQNO && $list_type ) ); + unless ( $rtokens->[0] =~ /^=\d/ + || ( !TEST_OLD_LIST && $list_type ) ); - # In this case we will limit padding to one indent distance. This + # In this case we will limit padding to a short distance. This # is a compromise to keep some vertical alignment but prevent large # gaps, which do not look good for just two lines. + my $pad_max = + two_line_pad( $rlines->[$jbeg], $rlines->[$jbeg_m], $imax_min ); + next unless ($pad_max); my $ng_m = $ng - 1; - $max_move{"$ng_m"} = $short_pad; - $max_move{"$ng"} = $short_pad; + $max_move{"$ng_m"} = $pad_max; + $max_move{"$ng"} = $pad_max; } # Loop to find all common leading tokens. @@ -3163,7 +3230,7 @@ sub match_line_pairs { elsif ( $list_type && $list_type eq $list_type_m ) { # do not align lists across a ci jump with new list method - if ( TEST_LIST_SEQNO && $ci_jump ) { $imax_min = -1 } + if ( !TEST_OLD_LIST && $ci_jump ) { $imax_min = -1 } my $i_nomatch = $imax_min + 1; for ( my $i = 0 ; $i <= $imax_min ; $i++ ) { diff --git a/local-docs/BugLog.pod b/local-docs/BugLog.pod index 831f31ed..e7ff6522 100644 --- a/local-docs/BugLog.pod +++ b/local-docs/BugLog.pod @@ -2,6 +2,49 @@ =over 4 +=item B + +In the process of making vertical alignments, lines which are simple lists of +items are treated different from other lines. The old method for finding and +marking these lines had a few problems which are corrected with this update. +The main problem was that the old method ran into trouble when there were side +comments. For example, the old method was not marking the following list and +as a result the two columns of values were not aligned: + + # OLD + return ( + $startpos, $ldelpos - $startpos, # PREFIX + $ldelpos, 1, # OPENING BRACKET + $ldelpos + 1, $endpos - $ldelpos - 2, # CONTENTS + $endpos - 1, 1, # CLOSING BRACKET + $endpos, length($$textref) - $endpos, # REMAINDER + ); + + # NEW + return ( + $startpos, $ldelpos - $startpos, # PREFIX + $ldelpos, 1, # OPENING BRACKET + $ldelpos + 1, $endpos - $ldelpos - 2, # CONTENTS + $endpos - 1, 1, # CLOSING BRACKET + $endpos, length($$textref) - $endpos, # REMAINDER + ); + +Another problem was that occasionally unwanted alignments were made between +lines which were not really lists because the lines were incorrectly marked. +For example (note padding after first comma) + + # OLD: (undesirable alignment) + my ( $isig2, $chisq ) = ( 1 / ( $sig * $sig ), 0 ); + my ( $ym, $al, $cov, $bet, $olda, $ochisq, $di, $pivt, $info ) = + map { null } ( 0 .. 8 ); + + # NEW: (no alignment) + my ( $isig2, $chisq ) = ( 1 / ( $sig * $sig ), 0 ); + my ( $ym, $al, $cov, $bet, $olda, $ochisq, $di, $pivt, $info ) = + map { null } ( 0 .. 8 ); + +This update was made 22 Dec 2020. + =item B Closing pattern delimiter tokens of qw quotes were not following the -cti flag diff --git a/t/snippets/expect/kba1.kba1 b/t/snippets/expect/kba1.kba1 index 453e439c..c18f5ea1 100644 --- a/t/snippets/expect/kba1.kba1 +++ b/t/snippets/expect/kba1.kba1 @@ -1,6 +1,6 @@ $this_env = join( "", $before, $closures - , $contents + , $contents , ( $defenv ? '' : &balance_tags() ) , $reopens ); diff --git a/t/snippets/expect/style.style1 b/t/snippets/expect/style.style1 index c25402fd..785d4865 100644 --- a/t/snippets/expect/style.style1 +++ b/t/snippets/expect/style.style1 @@ -1,7 +1,7 @@ # This test snippet is from package bbbike v3.214 by Slaven Rezic; GPL 2.0 licence sub arrange_topframe { my (@order) = ( - $hslabel_frame, $km_frame, $speed_frame[0], + $hslabel_frame, $km_frame, $speed_frame[0], $power_frame[0], $wind_frame, $percent_frame, $temp_frame, @speed_frame[1 .. $#speed_frame], @power_frame[1 .. $#power_frame], diff --git a/t/snippets10.t b/t/snippets10.t index c6f904ef..302e6148 100644 --- a/t/snippets10.t +++ b/t/snippets10.t @@ -760,7 +760,7 @@ sub arrange_topframe { # This test snippet is from package bbbike v3.214 by Slaven Rezic; GPL 2.0 licence sub arrange_topframe { my (@order) = ( - $hslabel_frame, $km_frame, $speed_frame[0], + $hslabel_frame, $km_frame, $speed_frame[0], $power_frame[0], $wind_frame, $percent_frame, $temp_frame, @speed_frame[1 .. $#speed_frame], @power_frame[1 .. $#power_frame], diff --git a/t/snippets22.t b/t/snippets22.t index 12567450..92563c25 100644 --- a/t/snippets22.t +++ b/t/snippets22.t @@ -628,7 +628,7 @@ method 'foo2' => [ Int, Int ] => sub { expect => <<'#16...........', $this_env = join( "", $before, $closures - , $contents + , $contents , ( $defenv ? '' : &balance_tags() ) , $reopens );