From 5c4d945eece763582e5703f008a3f668c649e1f7 Mon Sep 17 00:00:00 2001 From: Steve Hancock Date: Sun, 2 Jun 2019 20:10:20 -0700 Subject: [PATCH] fixed glitch with comma alignment; added tests --- lib/Perl/Tidy/Formatter.pm | 33 +++++++++++++++----------- lib/Perl/Tidy/VerticalAligner.pm | 2 +- t/snippets/align29.in | 6 +++++ t/snippets/align30.in | 3 +++ t/snippets/expect/align29.def | 6 +++++ t/snippets/expect/align30.def | 3 +++ t/snippets/expect/align4.def | 2 +- t/snippets/expect/given1.def | 6 ++--- t/snippets/expect/kgb3.def | 2 +- t/snippets/expect/kgb3.kgb | 2 +- t/snippets/expect/listop1.def | 2 +- t/snippets/expect/smart.def | 40 ++++++++++++++++---------------- t/snippets/packing_list.txt | 4 +++- t/snippets1.t | 2 +- t/snippets10.t | 40 ++++++++++++++++---------------- t/snippets14.t | 4 ++-- t/snippets15.t | 40 ++++++++++++++++++++++++++++++++ t/snippets3.t | 6 ++--- t/snippets5.t | 2 +- 19 files changed, 135 insertions(+), 70 deletions(-) create mode 100644 t/snippets/align29.in create mode 100644 t/snippets/align30.in create mode 100644 t/snippets/expect/align29.def create mode 100644 t/snippets/expect/align30.def diff --git a/lib/Perl/Tidy/Formatter.pm b/lib/Perl/Tidy/Formatter.pm index c1314a33..13b68830 100644 --- a/lib/Perl/Tidy/Formatter.pm +++ b/lib/Perl/Tidy/Formatter.pm @@ -11535,20 +11535,25 @@ sub get_seqno { $alignment_type = $vert_last_nonblank_type; } -## #-------------------------------------------------------- -## # DEACTIVATED; DOES NOT SEEM NEEDED WITH NEW VERTICAL ALIGNER -## # patch for =~ operator. We only align this if it -## # is the first operator in a line, and the line is a simple -## # statement. Aligning them within a statement -## # could interfere with other good alignments. -## #-------------------------------------------------------- -## if ( $alignment_type eq '=~' ) { -## my $terminal_type = $types_to_go[$i_terminal]; -## if ( $count > 0 || $max_line > 0 || $terminal_type ne ';' ) -## { -## $alignment_type = ""; -## } -## } + #-------------------------------------------------------- + # patch for =~ operator. We only align this if it + # is the first operator in a line, and the line is a simple + # statement. Aligning them within a statement + # could interfere with other good alignments. + #-------------------------------------------------------- + if ( $alignment_type eq '=~' ) { + ##my $terminal_type = $types_to_go[$i_terminal]; + ##if ( $count > 0 || $max_line > 0 || $terminal_type ne ';' ) + ## TEMPORARY PATCH: allow =~ except in an 'elsif' statement + ## BUBBA FIXME: This prevents loss of padding in if/elsif lines, but the + ## Vertical aligner should be updated to handle this automatically. + ## Then this section can be removed + if ( $types_to_go[$ibeg] eq 'k' + && $tokens_to_go[$ibeg] eq 'elsif' ) + { + $alignment_type = ""; + } + } #-------------------------------------------------------- # then store the value diff --git a/lib/Perl/Tidy/VerticalAligner.pm b/lib/Perl/Tidy/VerticalAligner.pm index ee490d66..e386d538 100644 --- a/lib/Perl/Tidy/VerticalAligner.pm +++ b/lib/Perl/Tidy/VerticalAligner.pm @@ -2307,7 +2307,7 @@ EOM #print "tok=$tok, lev=$lev, gl=$group_level, i=$i, ieq=$i_eq\n"; return if ( defined($i_eq) && $i < $i_eq ); - return if ( $lev >= $group_level ); + return if ( $lev <= $group_level ); } # most operators with an equals sign should be retained if at diff --git a/t/snippets/align29.in b/t/snippets/align29.in new file mode 100644 index 00000000..78cd0e60 --- /dev/null +++ b/t/snippets/align29.in @@ -0,0 +1,6 @@ +# alignment with lots of commas +is( floor(1.23441242), 1, "Basic floor(1.23441242) test" ); +is( fmod( 3.5, 2.0 ), 1.5, "Basic fmod(3.5, 2.0) test" ); +is( join( " ", frexp(1) ), "0.5 1", "Basic frexp(1) test" ); +is( ldexp( 0, 1 ), 0, "Basic ldexp(0,1) test" ); +is( log10(1), 0, "Basic log10(1) test" ); diff --git a/t/snippets/align30.in b/t/snippets/align30.in new file mode 100644 index 00000000..74b64e3e --- /dev/null +++ b/t/snippets/align30.in @@ -0,0 +1,3 @@ +# commas on lhs align, commas on rhs do not (different subs) +($x,$y,$z)=spherical_to_cartesian($rho,$theta,$phi); +($rho_c,$theta,$z)=spherical_to_cylindrical($rho_s,$theta,$phi); diff --git a/t/snippets/expect/align29.def b/t/snippets/expect/align29.def new file mode 100644 index 00000000..b30f966d --- /dev/null +++ b/t/snippets/expect/align29.def @@ -0,0 +1,6 @@ +# alignment with lots of commas +is( floor(1.23441242), 1, "Basic floor(1.23441242) test" ); +is( fmod( 3.5, 2.0 ), 1.5, "Basic fmod(3.5, 2.0) test" ); +is( join( " ", frexp(1) ), "0.5 1", "Basic frexp(1) test" ); +is( ldexp( 0, 1 ), 0, "Basic ldexp(0,1) test" ); +is( log10(1), 0, "Basic log10(1) test" ); diff --git a/t/snippets/expect/align30.def b/t/snippets/expect/align30.def new file mode 100644 index 00000000..39cd8af0 --- /dev/null +++ b/t/snippets/expect/align30.def @@ -0,0 +1,3 @@ +# commas on lhs align, commas on rhs do not (different subs) +( $x, $y, $z ) = spherical_to_cartesian( $rho, $theta, $phi ); +( $rho_c, $theta, $z ) = spherical_to_cylindrical( $rho_s, $theta, $phi ); diff --git a/t/snippets/expect/align4.def b/t/snippets/expect/align4.def index ebdfa58b..255219d6 100644 --- a/t/snippets/expect/align4.def +++ b/t/snippets/expect/align4.def @@ -1,6 +1,6 @@ # removed 'eq' and '=~' from alignment tokens to get alignment of '?'s my $salute = - $name eq $EMPTY_STR ? 'Customer' + $name eq $EMPTY_STR ? 'Customer' : $name =~ m/\A((?:Sir|Dame) \s+ \S+) /xms ? $1 : $name =~ m/(.*), \s+ Ph[.]?D \z /xms ? "Dr $1" : $name; diff --git a/t/snippets/expect/given1.def b/t/snippets/expect/given1.def index 37ae011d..f3e71bde 100644 --- a/t/snippets/expect/given1.def +++ b/t/snippets/expect/given1.def @@ -1,10 +1,10 @@ given ( [ 9, "a", 11 ] ) { when (qr/\d/) { given ($count) { - when (1) { ok( $count == 1 ) } - else { ok( $count != 1 ) } + when (1) { ok( $count == 1 ) } + else { ok( $count != 1 ) } when ( [ 5, 6 ] ) { ok(0) } - else { ok(1) } + else { ok(1) } } } ok(1) when 11; diff --git a/t/snippets/expect/kgb3.def b/t/snippets/expect/kgb3.def index 0f805444..5cdbe88c 100644 --- a/t/snippets/expect/kgb3.def +++ b/t/snippets/expect/kgb3.def @@ -1,5 +1,5 @@ #!/usr/bin/perl -w -use strict; # with -kgb, no break after hash bang +use strict; # with -kgb, no break after hash bang our ( @Changed, $TAP ); # break after isolated 'our' use File::Compare; use Symbol; diff --git a/t/snippets/expect/kgb3.kgb b/t/snippets/expect/kgb3.kgb index 61e1c6d5..159b0c26 100644 --- a/t/snippets/expect/kgb3.kgb +++ b/t/snippets/expect/kgb3.kgb @@ -1,5 +1,5 @@ #!/usr/bin/perl -w -use strict; # with -kgb, no break after hash bang +use strict; # with -kgb, no break after hash bang our ( @Changed, $TAP ); # break after isolated 'our' use File::Compare; diff --git a/t/snippets/expect/listop1.def b/t/snippets/expect/listop1.def index d116a8f3..267fcb9d 100644 --- a/t/snippets/expect/listop1.def +++ b/t/snippets/expect/listop1.def @@ -1,3 +1,3 @@ my @sorted = map { $_->[0] } sort { $a->[1] <=> $b->[1] } - map { [ $_, rand ] } @list; + map { [ $_, rand ] } @list; diff --git a/t/snippets/expect/smart.def b/t/snippets/expect/smart.def index cf1600c5..d0278562 100644 --- a/t/snippets/expect/smart.def +++ b/t/snippets/expect/smart.def @@ -38,28 +38,28 @@ b_const ~~ a_const; map { $_ => 'x' } keys %main:: } ~~ \%main::; -\%hash ~~ \%tied_hash; -\%tied_hash ~~ \%hash; -\%tied_hash ~~ \%tied_hash; -\%tied_hash ~~ \%tied_hash; -\%:: ~~ [ keys %main:: ]; -[ keys %main:: ] ~~ \%::; -\%:: ~~ []; -[] ~~ \%::; -{ "" => 1 } ~~ [undef]; -[undef] ~~ { "" => 1 }; -{ foo => 1 } ~~ qr/^(fo[ox])$/; -qr/^(fo[ox])$/ ~~ { foo => 1 }; -+{ 0 .. 100 } ~~ qr/[13579]$/; -qr/[13579]$/ ~~ +{ 0 .. 100 }; +\%hash ~~ \%tied_hash; +\%tied_hash ~~ \%hash; +\%tied_hash ~~ \%tied_hash; +\%tied_hash ~~ \%tied_hash; +\%:: ~~ [ keys %main:: ]; +[ keys %main:: ] ~~ \%::; +\%:: ~~ []; +[] ~~ \%::; +{ "" => 1 } ~~ [undef]; +[undef] ~~ { "" => 1 }; +{ foo => 1 } ~~ qr/^(fo[ox])$/; +qr/^(fo[ox])$/ ~~ { foo => 1 }; ++{ 0 .. 100 } ~~ qr/[13579]$/; +qr/[13579]$/ ~~ +{ 0 .. 100 }; +{ foo => 1, bar => 2 } ~~ "foo"; -"foo" ~~ +{ foo => 1, bar => 2 }; +"foo" ~~ +{ foo => 1, bar => 2 }; +{ foo => 1, bar => 2 } ~~ "baz"; -"baz" ~~ +{ foo => 1, bar => 2 }; -[] ~~ []; -[] ~~ []; -[] ~~ [1]; -[1] ~~ []; +"baz" ~~ +{ foo => 1, bar => 2 }; +[] ~~ []; +[] ~~ []; +[] ~~ [1]; +[1] ~~ []; [ ["foo"], ["bar"] ] ~~ [ qr/o/, qr/a/ ]; [ qr/o/, qr/a/ ] ~~ [ ["foo"], ["bar"] ]; [ "foo", "bar" ] ~~ [ qr/o/, qr/a/ ]; diff --git a/t/snippets/packing_list.txt b/t/snippets/packing_list.txt index c30742a0..a4ded4f3 100644 --- a/t/snippets/packing_list.txt +++ b/t/snippets/packing_list.txt @@ -126,6 +126,7 @@ ../snippets15.t break_old_methods.def ../snippets15.t bom1.bom ../snippets15.t bom1.def +../snippets15.t align28.def ../snippets2.t angle.def ../snippets2.t arrows1.def ../snippets2.t arrows2.def @@ -286,4 +287,5 @@ ../snippets9.t rt98902.def ../snippets9.t rt98902.rt98902 ../snippets9.t rt99961.def -../snippets15.t align28.def +../snippets15.t align29.def +../snippets15.t align30.def diff --git a/t/snippets1.t b/t/snippets1.t index 68927e32..94e11c23 100644 --- a/t/snippets1.t +++ b/t/snippets1.t @@ -287,7 +287,7 @@ if ( expect => <<'#5...........', # removed 'eq' and '=~' from alignment tokens to get alignment of '?'s my $salute = - $name eq $EMPTY_STR ? 'Customer' + $name eq $EMPTY_STR ? 'Customer' : $name =~ m/\A((?:Sir|Dame) \s+ \S+) /xms ? $1 : $name =~ m/(.*), \s+ Ph[.]?D \z /xms ? "Dr $1" : $name; diff --git a/t/snippets10.t b/t/snippets10.t index f2b4b214..2bda303f 100644 --- a/t/snippets10.t +++ b/t/snippets10.t @@ -533,28 +533,28 @@ b_const ~~ a_const; map { $_ => 'x' } keys %main:: } ~~ \%main::; -\%hash ~~ \%tied_hash; -\%tied_hash ~~ \%hash; -\%tied_hash ~~ \%tied_hash; -\%tied_hash ~~ \%tied_hash; -\%:: ~~ [ keys %main:: ]; -[ keys %main:: ] ~~ \%::; -\%:: ~~ []; -[] ~~ \%::; -{ "" => 1 } ~~ [undef]; -[undef] ~~ { "" => 1 }; -{ foo => 1 } ~~ qr/^(fo[ox])$/; -qr/^(fo[ox])$/ ~~ { foo => 1 }; -+{ 0 .. 100 } ~~ qr/[13579]$/; -qr/[13579]$/ ~~ +{ 0 .. 100 }; +\%hash ~~ \%tied_hash; +\%tied_hash ~~ \%hash; +\%tied_hash ~~ \%tied_hash; +\%tied_hash ~~ \%tied_hash; +\%:: ~~ [ keys %main:: ]; +[ keys %main:: ] ~~ \%::; +\%:: ~~ []; +[] ~~ \%::; +{ "" => 1 } ~~ [undef]; +[undef] ~~ { "" => 1 }; +{ foo => 1 } ~~ qr/^(fo[ox])$/; +qr/^(fo[ox])$/ ~~ { foo => 1 }; ++{ 0 .. 100 } ~~ qr/[13579]$/; +qr/[13579]$/ ~~ +{ 0 .. 100 }; +{ foo => 1, bar => 2 } ~~ "foo"; -"foo" ~~ +{ foo => 1, bar => 2 }; +"foo" ~~ +{ foo => 1, bar => 2 }; +{ foo => 1, bar => 2 } ~~ "baz"; -"baz" ~~ +{ foo => 1, bar => 2 }; -[] ~~ []; -[] ~~ []; -[] ~~ [1]; -[1] ~~ []; +"baz" ~~ +{ foo => 1, bar => 2 }; +[] ~~ []; +[] ~~ []; +[] ~~ [1]; +[1] ~~ []; [ ["foo"], ["bar"] ] ~~ [ qr/o/, qr/a/ ]; [ qr/o/, qr/a/ ] ~~ [ ["foo"], ["bar"] ]; [ "foo", "bar" ] ~~ [ qr/o/, qr/a/ ]; diff --git a/t/snippets14.t b/t/snippets14.t index 72126a68..207683f9 100644 --- a/t/snippets14.t +++ b/t/snippets14.t @@ -750,7 +750,7 @@ sub next_sibling { params => "def", expect => <<'#10...........', #!/usr/bin/perl -w -use strict; # with -kgb, no break after hash bang +use strict; # with -kgb, no break after hash bang our ( @Changed, $TAP ); # break after isolated 'our' use File::Compare; use Symbol; @@ -773,7 +773,7 @@ print "break before this line\n"; params => "kgb", expect => <<'#11...........', #!/usr/bin/perl -w -use strict; # with -kgb, no break after hash bang +use strict; # with -kgb, no break after hash bang our ( @Changed, $TAP ); # break after isolated 'our' use File::Compare; diff --git a/t/snippets15.t b/t/snippets15.t index 32dd4622..1d474ed7 100644 --- a/t/snippets15.t +++ b/t/snippets15.t @@ -11,6 +11,8 @@ #8 bom1.bom #9 bom1.def #10 align28.def +#11 align29.def +#12 align30.def # To locate test #13 you can search for its name or the string '#13' @@ -53,6 +55,21 @@ eval { if ( $a->{'abc'} eq 'ABC' ) { no_op(23) } else { no_op(42) } }; +---------- + + 'align29' => <<'----------', +# alignment with lots of commas +is( floor(1.23441242), 1, "Basic floor(1.23441242) test" ); +is( fmod( 3.5, 2.0 ), 1.5, "Basic fmod(3.5, 2.0) test" ); +is( join( " ", frexp(1) ), "0.5 1", "Basic frexp(1) test" ); +is( ldexp( 0, 1 ), 0, "Basic ldexp(0,1) test" ); +is( log10(1), 0, "Basic log10(1) test" ); +---------- + + 'align30' => <<'----------', +# commas on lhs align, commas on rhs do not (different subs) +($x,$y,$z)=spherical_to_cartesian($rho,$theta,$phi); +($rho_c,$theta,$z)=spherical_to_cylindrical($rho_s,$theta,$phi); ---------- 'bom1' => <<'----------', @@ -282,6 +299,29 @@ eval { }; #10........... }, + + 'align29.def' => { + source => "align29", + params => "def", + expect => <<'#11...........', +# alignment with lots of commas +is( floor(1.23441242), 1, "Basic floor(1.23441242) test" ); +is( fmod( 3.5, 2.0 ), 1.5, "Basic fmod(3.5, 2.0) test" ); +is( join( " ", frexp(1) ), "0.5 1", "Basic frexp(1) test" ); +is( ldexp( 0, 1 ), 0, "Basic ldexp(0,1) test" ); +is( log10(1), 0, "Basic log10(1) test" ); +#11........... + }, + + 'align30.def' => { + source => "align30", + params => "def", + expect => <<'#12...........', +# commas on lhs align, commas on rhs do not (different subs) +( $x, $y, $z ) = spherical_to_cartesian( $rho, $theta, $phi ); +( $rho_c, $theta, $z ) = spherical_to_cylindrical( $rho_s, $theta, $phi ); +#12........... + }, }; my $ntests = 0 + keys %{$rtests}; diff --git a/t/snippets3.t b/t/snippets3.t index aa37aef9..9d8bf290 100644 --- a/t/snippets3.t +++ b/t/snippets3.t @@ -794,10 +794,10 @@ $_, $val given ( [ 9, "a", 11 ] ) { when (qr/\d/) { given ($count) { - when (1) { ok( $count == 1 ) } - else { ok( $count != 1 ) } + when (1) { ok( $count == 1 ) } + else { ok( $count != 1 ) } when ( [ 5, 6 ] ) { ok(0) } - else { ok(1) } + else { ok(1) } } } ok(1) when 11; diff --git a/t/snippets5.t b/t/snippets5.t index f65be6ec..1184fd9c 100644 --- a/t/snippets5.t +++ b/t/snippets5.t @@ -356,7 +356,7 @@ return $pdl->slice( expect => <<'#2...........', my @sorted = map { $_->[0] } sort { $a->[1] <=> $b->[1] } - map { [ $_, rand ] } @list; + map { [ $_, rand ] } @list; #2........... }, -- 2.39.5