From: Steve Hancock Date: Tue, 20 Jul 2021 02:19:57 +0000 (-0700) Subject: Fix some problems with -kgb in complex structures X-Git-Tag: 20210717.02~83 X-Git-Url: https://git.donarmstrong.com/?a=commitdiff_plain;h=071a3f6d8ce792e6c066b95babd7f5117b4f2d22;p=perltidy.git Fix some problems with -kgb in complex structures --- diff --git a/lib/Perl/Tidy/Formatter.pm b/lib/Perl/Tidy/Formatter.pm index b185082e..c55445f9 100644 --- a/lib/Perl/Tidy/Formatter.pm +++ b/lib/Perl/Tidy/Formatter.pm @@ -9753,6 +9753,8 @@ EOM my $rlines = $self->[_rlines_]; my $rLL = $self->[_rLL_]; my $K_closing_container = $self->[_K_closing_container_]; + my $K_opening_container = $self->[_K_opening_container_]; + my $rK_weld_right = $self->[_rK_weld_right_]; # variables for the current group and subgroups: my ( $ibeg, $iend, $count, $level_beg, $K_closing, @iblanks, @group, @@ -9949,25 +9951,42 @@ EOM my $find_container_end = sub { - # If the keyword lines ends with an open token, find the closing token - # '$K_closing' so that we can easily skip past the contents of the - # container. - return if ( $K_last <= $K_first ); - my $KK = $K_last; - my $type_last = $rLL->[$KK]->[_TYPE_]; - my $tok_last = $rLL->[$KK]->[_TOKEN_]; - if ( $type_last eq '#' ) { - $KK = $self->K_previous_nonblank($KK); - $tok_last = $rLL->[$KK]->[_TOKEN_]; - } - if ( $KK > $K_first && $tok_last =~ /^[\(\{\[]$/ ) { + # If the keyword line is continued onto subsequent lines, find the + # closing token '$K_closing' so that we can easily skip past the + # contents of the container. - my $type_sequence = $rLL->[$KK]->[_TYPE_SEQUENCE_]; - my $lev = $rLL->[$KK]->[_LEVEL_]; - if ( $lev == $level_beg ) { - $K_closing = $K_closing_container->{$type_sequence}; - } - } + # We only set this value if we find a simple list, meaning + # -contents only one level deep + # -not welded + + # First check: skip if next line is not one deeper + my $Knext_nonblank = $self->K_next_nonblank($K_last); + goto RETURN if ( !defined($Knext_nonblank) ); + my $level_next = $rLL->[$Knext_nonblank]->[_LEVEL_]; + goto RETURN if ( $level_next != $level_beg + 1 ); + + # Find the parent container of the first token on the next line + my $parent_seqno = $self->parent_seqno_by_K($Knext_nonblank); + goto RETURN unless defined($parent_seqno); + + # Must not be a weld (can be unstable) + goto RETURN + if ( $total_weld_count && $self->is_welded_at_seqno($parent_seqno) ); + + # Opening container must exist and be on this line + my $Ko = $K_opening_container->{$parent_seqno}; + goto RETURN unless ( defined($Ko) && $Ko > $K_first && $Ko <= $K_last ); + + # Verify that the closing container exists and is on a later line + my $Kc = $K_closing_container->{$parent_seqno}; + goto RETURN unless ( defined($Kc) && $Kc > $K_last ); + + # That's it + $K_closing = $Kc; + goto RETURN; + + RETURN: + return; }; my $add_to_group = sub { @@ -10060,15 +10079,36 @@ EOM return $rhash_of_desires; } - # This is not for keywords in lists ( keyword 'my' can occur in lists, - # see case b760) - next if ( $self->is_list_by_K($K_first) ); - my $level = $rLL->[$K_first]->[_LEVEL_]; my $type = $rLL->[$K_first]->[_TYPE_]; my $token = $rLL->[$K_first]->[_TOKEN_]; my $ci_level = $rLL->[$K_first]->[_CI_LEVEL_]; + # End a group 'badly' at an unexpected level. This will prevent + # blank lines being incorrectly placed after the end of the group. + # We are looking for any deviation from two acceptable patterns: + # PATTERN 1: a simple list; secondary lines are at level+1 + # PATTERN 2: a long statement; all secondary lines same level + # This was added as a fix for case b1177, in which a complex structure + # got incorrectly inserted blank lines. + if ( $ibeg >= 0 ) { + + # Check for deviation from PATTERN 1, simple list: + if ( defined($K_closing) && $K_first < $K_closing ) { + $end_group->(1) if ( $level != $level_beg + 1 ); + } + + # Check for deviation from PATTERN 2, single statement: + elsif ( $level != $level_beg ) { $end_group->(1) } + } + + # Do not look for keywords in lists ( keyword 'my' can occur in lists, + # see case b760); fixed for c048. + if ( $self->is_list_by_K($K_first) ) { + if ( $ibeg >= 0 ) { $iend = $i } + next; + } + # see if this is a code type we seek (i.e. comment) if ( $CODE_type && $Opt_comment_pattern @@ -10087,7 +10127,7 @@ EOM # first end old group if any; we might be starting new # keywords at different level - if ( $ibeg > 0 ) { $end_group->(); } + if ( $ibeg >= 0 ) { $end_group->(); } $add_to_group->( $i, $tok, $level ); } next; @@ -10110,7 +10150,7 @@ EOM # first end old group if any; we might be starting new # keywords at different level - if ( $ibeg > 0 ) { $end_group->(); } + if ( $ibeg >= 0 ) { $end_group->(); } $add_to_group->( $i, $token, $level ); } next; @@ -10123,7 +10163,7 @@ EOM # - bail out on a large level change; we may have walked into a # data structure or anoymous sub code. if ( $level > $level_beg + 1 || $level < $level_beg ) { - $end_group->(); + $end_group->(1); next; } diff --git a/local-docs/BugLog.pod b/local-docs/BugLog.pod index b37001c8..a4245604 100644 --- a/local-docs/BugLog.pod +++ b/local-docs/BugLog.pod @@ -2,6 +2,74 @@ =over 4 +=item B + +This update fixes two problems involving the -kgb option. + +The first problem is that testing with random parameters produced some examples +of formatting instabilities involving applying --keyword-group-blanks to +complex structures, particularly welded structures. The -kgb parameters work +well on simple statements or simple lists, so a check was added to prevent them +from working on lists which are more than one level deep. This fixes issues +b1177 and b1178 without significantly changing the -kgb functionality. + +The second problem is that a terminal blank line could be misplaced if +the last statement was a structure. This is illustrated with the following +snippet: + + sub new { + my $class = shift; + my $number = shift || croak "What?? No number??\n"; + my $classToGenerate = shift || croak "Need a class to generate, man!\n"; + my $hash = shift; #No carp here, some operators do not need specific stuff + my $self = { _number => $number, + _class => $classToGenerate, + _hash => $hash }; + bless $self, $class; # And bless it + return $self; + } + + # OLD: perltidy -kgb -sil=0 gave + sub new { + + my $class = shift; + my $number = shift || croak "What?? No number??\n"; + my $classToGenerate = shift || croak "Need a class to generate, man!\n"; + my $hash = shift; #No carp here, some operators do not need specific stuff + my $self = { + _number => $number, + + _class => $classToGenerate, + _hash => $hash + }; + bless $self, $class; # And bless it + return $self; + } + +The blank line which has appeared after the line '_number =>' was intended +to go after the closing brace but a line count was off. This has been fixed: + + # NEW: perltidy -kgb -sil=0 gives + sub new { + + my $class = shift; + my $number = shift || croak "What?? No number??\n"; + my $classToGenerate = shift || croak "Need a class to generate, man!\n"; + my $hash = shift; #No carp here, some operators do not need specific stuff + my $self = { + _number => $number, + _class => $classToGenerate, + _hash => $hash + }; + + bless $self, $class; # And bless it + return $self; + } + +This fixes issue c048. + +19 Jul 2021. + =item B A problem in the current version of perltidy is that a blank line @@ -43,7 +111,7 @@ A simple workaround until the next release is to include the parameter It can be removed after the next release. -18 Jul 2021. +18 Jul 2021, 9648e16. =item B @@ -73,7 +141,7 @@ when the following specific parameters were used This update fixes this issue, case b1174. -18 Jul 2021. +18 Jul 2021, 12ae46b. =item B @@ -93,7 +161,7 @@ the variable '$t' was being mis-tokenized as a possible indirect object. This update fixes cases b1175, b1176. -17 Jul 2021. +17 Jul 2021, 4aa1318. =item B