jrockway / cperl-mode

cperl-mode with 5.10 fixes, mx-declare support, perl6 support, etc.
102 stars 33 forks source link

sub print_foo {} #35

Closed rlauer6 closed 6 years ago

rlauer6 commented 7 years ago

open a new file and add subroutine:

sub print_foo {
}

  sub foo {
  }
choroba commented 7 years ago

And then what happened?

Oh, I see, the second sub is indented.

rlauer6 commented 7 years ago

indent incorrect...after sub print_anything { }

hakonhagland commented 7 years ago

Good catch! The same thing happens with sub grep_anything {}. So it gives indication that cperl-mode gets confused if the name of the sub starts with the same name as a Perl builtin function?

rlauer6 commented 7 years ago

I'm don't lisp, but it appears that somewhere near line 4145 there is some detection of built-ins for some reason. Perhaps not allowing for a preceding keyword of sub?

hakonhagland commented 7 years ago

As far as I can see, it seems cperl-after-block-and-statement-beg on line 4948 does not take into account that sub names can start with Perl builtin keywords. Please correct me if I am wrong. Here is a pull request Pull #36

hakonhagland commented 7 years ago

Hmm.. I think there might be some other issues with the regex also. The regex (at line 4962):

"\\(map\\|grep\\|say\\|printf?\\|system\\|exec\\|tr\\|s\\)\\>"

uses \\> to check that the builtin name is proper (not followed by word characters). Such that grep is recognized as a builtin but grepa is not. I am not sure if this is correct? I did some testing, and it seems like \\> will not recognize underscore or unicode word characters as "word" characters. Hence, it will accept print_any or printБ as print. Should it be replaced by [^[:alnum:]_] or something else?

hakonhagland commented 7 years ago

Here is a test with the new patch [https://github.com/jrockway/cperl-mode/pull/36](Pull #36). It works as expected (gives expected indentation according to my setup for cperl-mode) for this test:

use strict;
use warnings;
use feature qw(say);
use utf8;

sub f (&$$) {
    1
}

sub grepБ (&$$) {
    1
}

grep {
    $_ == 2
}
  2, 3;

grepБ {
    /\d/
}
  2, 3;

f {
    say "Testing"
}
  3,4;

So the possible issue with the regex \\>, that I mentioned does not seem to apply for this test case. I am not sure why :-/ We should probably devise some more test cases to check edge cases before applying this patch?

choroba commented 7 years ago

The problem only happens with no prototypes.

rlauer6 commented 7 years ago

Works for me. Tnx!

renormalist commented 6 years ago

merged #36