Subject: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix

From: Stefan Kangas <[email-masked]>
Date: Sat, 8 Mar 2025 14:16:59 +0000

Harald Jörg <[email-masked]> writes:

> Stefan Kangas <[email-masked]> writes:
>
>> Harald Jörg <[email-masked]> writes:
>>
>>> Stefan Kangas <[email-masked]> writes:
>>>
>>>> John Ciolfi via "Bug reports for GNU Emacs, the Swiss army knife of text
>>>> editors" <[email-masked]> writes:
>>>>
>>>>> Hi
>>>>>
>>>>> Given this perl file:
>>>>>
>>>>>     sub test {
>>>>>         exec '/bin/echo',
>>>>>             'Your arguments are: ', @ARGV;
>>>>>     }
>>>>>
>>>>>     sub exec_fcn {
>>>>>     }
>>>>>
>>>>>         sub other {
>>>>>         }
>>>>>
>>>>> The 'sub other' is indented incorrectly (and all code following it).
>>>>>
>>>>> The fix is in cperl-after-block-and-statement-beg to not match exec_, i.e. we should
>>>>> not treat exec_fcn as a builtin. Attached is the fix.
>>>>
>>>> Harald, any comments on this patch?
>>>
>>> Yes :)
>>>
>>> I confirm this is a bug, and one of those nasty ones which affect all
>>> following code.
>>>
>>> I suggest a slightly improved patch.  Instead of replacing \> (end of
>>> word) by [[:space:]] it should be replaced by \_> (end of symbol).
>>> A space is not required after the keywords in the list.  In the case of
>>> exec, this is handled elsewhere, but there are keywords in the list
>>> where it matters.
>>>
>>> With [[:space:]], the following indentation happens:
>>>
>>>     my %h = map{$_=>1}
>>>     @ARGV;
>>>
>>> With \_>, @ARGV is correctly indented as a continuation line:
>>>
>>>     my %h = map{$_=>1}
>>>         @ARGV;
>>>
>>> This is less severe than the bug reported because it does not affect
>>> following code, but still should not happen.
>>
>> I recommend adding tests for the above.  I think we already have good
>> erts tests in cperl-indents.erts, that could easily be expanded.
>
> Sure, I plan to do that.  CPerl indentation with its multitude of
> options is one of its messier corners, without extensive tests it is
> dangerous to change anything.
>
> How do we proceed? Shall I commit my patch to emacs-30 together with
> tests?

I don't think I can fully assess the risks of installing or not
installing this change, as cperl-mode is quite complex and I'm not
too familiar with it.

If this is a regression, we would tend to favor installing the fix on
the release branch.  Otherwise, especially if this is a messy corner and
we feel that it's risky to change, perhaps we should install it on
master.

What is your estimation of this issue?  Do you consider it important to
fix on the release branch?