From b364288ea908b784ed4c8cffb6c4afdb08d953ed Mon Sep 17 00:00:00 2001 From: Steve Hancock Date: Fri, 11 Oct 2024 07:20:49 -0700 Subject: [PATCH] fix two-line shear, issue c406 --- bin/perltidy | 28 ++++----------------- dev-bin/run_convergence_tests.pl.expect | 33 +++++++++++++------------ lib/Perl/Tidy/Formatter.pm | 30 ++++++++++++++++++++++ 3 files changed, 52 insertions(+), 39 deletions(-) diff --git a/bin/perltidy b/bin/perltidy index 42b9f195..e3258025 100755 --- a/bin/perltidy +++ b/bin/perltidy @@ -4096,7 +4096,7 @@ and attempt to delete non-bare commas, we delete a comma which has become bare, which is not what is wanted. This happened because the change was based on the starting rather than the final -line breaks. Rerunning with B<--converge> added fixes things +line breaks. Running with B<--converge> gives the desired result: # perltidy -wtc=b -dtc --converge f( @@ -4104,7 +4104,7 @@ line breaks. Rerunning with B<--converge> added fixes things b => 2, ); -because changes are based on the line breaks after the first iteration. +because comma changes are based on the line breaks after the first iteration. The additional computer time needed by the B<--converge> option to do another iteration or two will not be noticable except for files with many thousands of @@ -4118,27 +4118,9 @@ normally be necessary. =item * -Perltidy does not add a trailing comma to a list which appears to be near a -B. So if a comma is unexpectedly not added, this is probably -the reason. - -This issue can be illustrated with the following example. The closing brace is -at column 80, the default line length: - - # perltidy -atc -dtc -wtc=b - $menuitem_paste->signal_connect( 'activate' => sub { create_paste_window() } - ); - -Adding a trailing comma would cause the maximum line length to be exceeded. -This in turn would cause a different break point to occur for which the comma -is no longer bare. So it would get deleted on the next formatting pass, -returning things to the starting state. This is is an unstable situation. - -The rules used to detect and avoid instability work well but are not precise, -so in some cases where perltidy will not add a comma, it may be possible to add -a stable trailing comma with editing. For example, if the above example were -run with B<-l=81 -atc -wtc=b>, perltidy would still not add a trailing comma, -even though it would be stable. +Perltidy does not add a trailing comma in some B which appear to +be near a stability limit. So if a comma is unexpectedly not added, this is +probably the reason. =item * diff --git a/dev-bin/run_convergence_tests.pl.expect b/dev-bin/run_convergence_tests.pl.expect index 5f1d02b7..6ed49f26 100644 --- a/dev-bin/run_convergence_tests.pl.expect +++ b/dev-bin/run_convergence_tests.pl.expect @@ -2791,7 +2791,8 @@ $term = { # Don't allow interruptions $SIG{$_} = 'IGNORE'; } - ( open( CHECKPOINT, "> $aub_tmp" + ( open( + CHECKPOINT, "> $aub_tmp" ) ) || # This is just temporary... &abort( @@ -2802,7 +2803,8 @@ $term = { # Don't allow interruptions $SIG{$_} = 'IGNORE'; } - ( open( CHECKPOINT, "> $aub_tmp" + ( open( + CHECKPOINT, "> $aub_tmp" ) ) || # This is just temporary... &abort( @@ -3099,8 +3101,9 @@ foreach $port ( ==> b1105 <== foreach $port ( sort - ( @ARGV[ 1 .. $#ARGV ] - ) ) + ( + @ARGV[ 1 .. $#ARGV ] ) + ) __END__ foreach $port ( sort @@ -8326,16 +8329,16 @@ use vars qw($VERSION @ISA @EXPORT_OK %EXPORT_TAGS); ==> b1489 <== - $menuitem_dict->signal_connect ( 'activate' => sub { create_dict_window () } - ); - $menuitem_dict->signal_connect ( 'activate' => sub { create_dict_window () } - ); + $menuitem_dict->signal_connect ( + 'activate' => sub { create_dict_window () } ); + $menuitem_dict->signal_connect ( + 'activate' => sub { create_dict_window () } ); ==> b1490 <== -$e_str->bind ( '' => sub {$e_hnr->tabfocus if $e_hnr->can ( 'tabfocus' );} -); -$e_str->bind ( '' => sub {$e_hnr->tabfocus if $e_hnr->can ( 'tabfocus' );} -); +$e_str->bind ( +'' => sub {$e_hnr->tabfocus if $e_hnr->can ( 'tabfocus' );} ); +$e_str->bind ( +'' => sub {$e_hnr->tabfocus if $e_hnr->can ( 'tabfocus' );} ); ==> b156 <== # State 1 @@ -11392,14 +11395,12 @@ foreach my $x ( )->join() ; threads->create( - sub { lock($COUNT) ; $COUNT++ ; } - )->join() ; + sub { lock($COUNT) ; $COUNT++ ; } )->join() ; ==> b978 <== $ans = threads->create( sub { threads->create( - sub { $p->greeting } - )->join ; + sub { $p->greeting } )->join ; } )->join ; $ans = threads->create( sub { diff --git a/lib/Perl/Tidy/Formatter.pm b/lib/Perl/Tidy/Formatter.pm index 0a9946b6..3145d3dc 100644 --- a/lib/Perl/Tidy/Formatter.pm +++ b/lib/Perl/Tidy/Formatter.pm @@ -29343,6 +29343,36 @@ sub break_long_lines { # separated by this line } + # Fix two-line shear (c406) + my $i_next_nonblank = $inext_to_go[$i_lowest]; + if ( $tokens_to_go[$i_next_nonblank] eq ')' ) { + + # Example of a '2 line shear': + + # $wrapped->add_around_modifier( + # sub { push @tracelog => 'around 1'; $_[0]->(); } ); + + # If we try formatting this with increasing line lengths, the + # break based on bond strengths is after the '(' until the closing + # paren is just beyond the line length limit. In that case, it can + # switch to being just before the ')'. This is rare, and may be + # undesirable because it can cause unexpected formatting + # variations between similar code, and worse, instability with + # trailing commas. So we check for that here and put the break + # back after the opening '(' if the ')' is not preceded by a ','. + # Issue c406. + my $i_prev = iprev_to_go($i_next_nonblank); + my $i_opening = $mate_index_to_go[$i_next_nonblank]; + if ( $types_to_go[$i_prev] ne ',' + && defined($i_opening) + && $i_opening > $i_last_break ) + { + # set a forced breakpoint to block recombination + $i_lowest = $i_opening; + $forced_breakpoint_to_go[$i_lowest] = 1; + } + } + # guard against infinite loop (should never happen) if ( $i_lowest <= $i_last_break ) { DEVEL_MODE -- 2.39.5