GNU bug report logs - #76851
29.4; cperl-mode builtin fcn indent bug and fix

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 76851 in the body.
You can then email your comments to 76851 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: John Ciolfi <ciolfi <at> mathworks.com>
To: <bug-gnu-emacs <at> gnu.org>
Subject: 29.4; cperl-mode builtin fcn indent bug and fix
Date: Sat, 8 Mar 2025 01:07:24 -0500

[Message part 1 (text/plain, inline)]

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.

Thanks,
John

[cperl-mode-builtin-fix.patch (text/x-diff, inline)]

--- a/cperl-mode.el
+++ b/cperl-mode.el
@@ -5606,8 +5606,16 @@ Do not look before LIM."
 	 (progn
 	   (forward-sexp -1)
 	   (not
+            ;; Used to indent functions like:
+            ;;    exec 'prog',
+            ;;        @ARGS;
+            ;; [[:space:]] means we will not match exec_fcn and 'sub other' is indented correctly:
+            ;;    sub exec_fcn {
+            ;;    }
+            ;;    sub other {
+            ;;    }
 	    (looking-at
-	     "\\(map\\|grep\\|say\\|printf?\\|system\\|exec\\|tr\\|s\\)\\>")))))))
+	     "\\(map\\|grep\\|say\\|printf?\\|system\\|exec\\|tr\\|s\\)[[:space:]]")))))))
 
 
 (defun cperl-indent-exp ()

[ATT00001 (text/plain, attachment)]

Message #10 received at 76851 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: John Ciolfi <ciolfi <at> mathworks.com>, 76851 <at> debbugs.gnu.org
Cc: Harald Jörg <haj <at> posteo.de>
Subject: Re: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix
Date: Sat, 8 Mar 2025 08:01:36 +0000

John Ciolfi via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> 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?

>
> Thanks,
> John
>
> --- a/cperl-mode.el
> +++ b/cperl-mode.el
> @@ -5606,8 +5606,16 @@ Do not look before LIM."
>  	 (progn
>  	   (forward-sexp -1)
>  	   (not
> +            ;; Used to indent functions like:
> +            ;;    exec 'prog',
> +            ;;        @ARGS;
> +            ;; [[:space:]] means we will not match exec_fcn and 'sub other' is indented correctly:
> +            ;;    sub exec_fcn {
> +            ;;    }
> +            ;;    sub other {
> +            ;;    }
>  	    (looking-at
> -	     "\\(map\\|grep\\|say\\|printf?\\|system\\|exec\\|tr\\|s\\)\\>")))))))
> +	     "\\(map\\|grep\\|say\\|printf?\\|system\\|exec\\|tr\\|s\\)[[:space:]]")))))))
>
>  
>  (defun cperl-indent-exp ()
>
> In GNU Emacs 29.4 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.38,
>  cairo version 1.16.0) of 2024-07-22, modified by Debian built on
>  x86-ubc-01
> Windowing system distributor 'The X.Org Foundation', version 11.0.12101006
> System Description: Debian GNU/Linux 12 (bookworm)
>
> Configured using:
>  'configure --build x86_64-linux-gnu --prefix=/usr
>  --sharedstatedir=/var/lib --libexecdir=/usr/libexec
>  --localstatedir=/var/lib --infodir=/usr/share/info
>  --mandir=/usr/share/man --with-libsystemd --with-pop=yes
>  --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/29.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/29.4/site-lisp:/usr/share/emacs/site-lisp
>  --with-sound=alsa --without-gconf --with-mailutils
>  --with-native-compilation --build x86_64-linux-gnu --prefix=/usr
>  --sharedstatedir=/var/lib --libexecdir=/usr/libexec
>  --localstatedir=/var/lib --infodir=/usr/share/info
>  --mandir=/usr/share/man --with-libsystemd --with-pop=yes
>  --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/29.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/29.4/site-lisp:/usr/share/emacs/site-lisp
>  --with-sound=alsa --without-gconf --with-mailutils
>  --with-native-compilation --with-cairo --with-x=yes
>  --with-x-toolkit=gtk3 --with-toolkit-scroll-bars 'CFLAGS=-g -O2
>  -ffile-prefix-map=/build/reproducible-path/emacs-29.4+1=. -fstack-protector-strong
>  -Wformat -Werror=format-security -Wall' 'CPPFLAGS=-Wdate-time
>  -D_FORTIFY_SOURCE=2' LDFLAGS=-Wl,-z,relro'
>
> Configured features:
> ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
> JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
> NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3
> THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2
> XPM GTK3 ZLIB
>
> Important settings:
>   value of $LANG: en_US.UTF-8
>   locale-coding-system: utf-8-unix
>
> Major mode: Fundamental
>
> Minor modes in effect:
>   tooltip-mode: t
>   global-eldoc-mode: t
>   show-paren-mode: t
>   electric-indent-mode: t
>   mouse-wheel-mode: t
>   tool-bar-mode: t
>   menu-bar-mode: t
>   file-name-shadow-mode: t
>   global-font-lock-mode: t
>   blink-cursor-mode: t
>   buffer-read-only: t
>   line-number-mode: t
>   indent-tabs-mode: t
>   transient-mark-mode: t
>   auto-composition-mode: t
>   auto-encryption-mode: t
>   auto-compression-mode: t
>
> Load-path shadows:
> None found.
>
> Features:
> (shadow sort mail-extr emacsbug message mailcap yank-media puny dired
> dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
> epg-config gnus-util text-property-search time-date mm-decode mm-bodies
> mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
> rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils comp comp-cstr
> warnings subr-x rx cl-seq cl-macs gv cl-extra help-mode bytecomp
> byte-compile cus-edit pp cus-start cus-load icons wid-edit cl-loaddefs
> cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify
> ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
> term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
> tabulated-list replace newcomment text-mode lisp-mode prog-mode register
> page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
> scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
> frame minibuffer nadvice seq simple cl-generic indonesian philippine
> cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
> korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
> european ethiopic indian cyrillic chinese composite emoji-zwj charscript
> charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
> cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
> files window text-properties overlay sha1 md5 base64 format env
> code-pages mule custom widget keymap hashtable-print-readable backquote
> threads dbusbind inotify lcms2 dynamic-setting system-font-setting
> font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x multi-tty
> make-network-process native-compile emacs)
>
> Memory information:
> ((conses 16 105675 8530)
>  (symbols 48 8853 7)
>  (strings 32 23487 2243)
>  (string-bytes 1 691014)
>  (vectors 16 17555)
>  (vector-slots 8 353713 18074)
>  (floats 8 39 42)
>  (intervals 56 579 2)
>  (buffers 984 14))

Message #13 received at 76851 <at> debbugs.gnu.org (full text, mbox):

From: Harald Jörg <haj <at> posteo.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: John Ciolfi <ciolfi <at> mathworks.com>, 76851 <at> debbugs.gnu.org
Subject: Re: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix
Date: Sat, 08 Mar 2025 09:48:35 +0000

Stefan Kangas <stefankangas <at> gmail.com> writes:

> John Ciolfi via "Bug reports for GNU Emacs, the Swiss army knife of text
> editors" <bug-gnu-emacs <at> gnu.org> 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.

>
>>
>> Thanks,
>> John
>>
>> --- a/cperl-mode.el
>> +++ b/cperl-mode.el
>> @@ -5606,8 +5606,16 @@ Do not look before LIM."
>>  	 (progn
>>  	   (forward-sexp -1)
>>  	   (not
>> +            ;; Used to indent functions like:
>> +            ;;    exec 'prog',
>> +            ;;        @ARGS;
>> +            ;; [[:space:]] means we will not match exec_fcn and 'sub other' is indented correctly:
>> +            ;;    sub exec_fcn {
>> +            ;;    }
>> +            ;;    sub other {
>> +            ;;    }
>>  	    (looking-at
>> -	     "\\(map\\|grep\\|say\\|printf?\\|system\\|exec\\|tr\\|s\\)\\>")))))))
>> +	     "\\(map\\|grep\\|say\\|printf?\\|system\\|exec\\|tr\\|s\\)[[:space:]]")))))))
>>
>>  
>>  (defun cperl-indent-exp ()
>>
>> In GNU Emacs 29.4 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.38,
>>  cairo version 1.16.0) of 2024-07-22, modified by Debian built on
>>  x86-ubc-01
>> Windowing system distributor 'The X.Org Foundation', version 11.0.12101006
>> System Description: Debian GNU/Linux 12 (bookworm)
>>
>> Configured using:
>>  'configure --build x86_64-linux-gnu --prefix=/usr
>>  --sharedstatedir=/var/lib --libexecdir=/usr/libexec
>>  --localstatedir=/var/lib --infodir=/usr/share/info
>>  --mandir=/usr/share/man --with-libsystemd --with-pop=yes
>>  --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/29.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/29.4/site-lisp:/usr/share/emacs/site-lisp
>>  --with-sound=alsa --without-gconf --with-mailutils
>>  --with-native-compilation --build x86_64-linux-gnu --prefix=/usr
>>  --sharedstatedir=/var/lib --libexecdir=/usr/libexec
>>  --localstatedir=/var/lib --infodir=/usr/share/info
>>  --mandir=/usr/share/man --with-libsystemd --with-pop=yes
>>  --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/29.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/29.4/site-lisp:/usr/share/emacs/site-lisp
>>  --with-sound=alsa --without-gconf --with-mailutils
>>  --with-native-compilation --with-cairo --with-x=yes
>>  --with-x-toolkit=gtk3 --with-toolkit-scroll-bars 'CFLAGS=-g -O2
>>  -ffile-prefix-map=/build/reproducible-path/emacs-29.4+1=. -fstack-protector-strong
>>  -Wformat -Werror=format-security -Wall' 'CPPFLAGS=-Wdate-time
>>  -D_FORTIFY_SOURCE=2' LDFLAGS=-Wl,-z,relro'
>>
>> Configured features:
>> ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
>> JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
>> NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3
>> THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2
>> XPM GTK3 ZLIB
>>
>> Important settings:
>>   value of $LANG: en_US.UTF-8
>>   locale-coding-system: utf-8-unix
>>
>> Major mode: Fundamental
>>
>> Minor modes in effect:
>>   tooltip-mode: t
>>   global-eldoc-mode: t
>>   show-paren-mode: t
>>   electric-indent-mode: t
>>   mouse-wheel-mode: t
>>   tool-bar-mode: t
>>   menu-bar-mode: t
>>   file-name-shadow-mode: t
>>   global-font-lock-mode: t
>>   blink-cursor-mode: t
>>   buffer-read-only: t
>>   line-number-mode: t
>>   indent-tabs-mode: t
>>   transient-mark-mode: t
>>   auto-composition-mode: t
>>   auto-encryption-mode: t
>>   auto-compression-mode: t
>>
>> Load-path shadows:
>> None found.
>>
>> Features:
>> (shadow sort mail-extr emacsbug message mailcap yank-media puny dired
>> dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
>> epg-config gnus-util text-property-search time-date mm-decode mm-bodies
>> mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
>> rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils comp comp-cstr
>> warnings subr-x rx cl-seq cl-macs gv cl-extra help-mode bytecomp
>> byte-compile cus-edit pp cus-start cus-load icons wid-edit cl-loaddefs
>> cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify
>> ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
>> term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
>> tabulated-list replace newcomment text-mode lisp-mode prog-mode register
>> page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
>> scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
>> frame minibuffer nadvice seq simple cl-generic indonesian philippine
>> cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
>> korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
>> european ethiopic indian cyrillic chinese composite emoji-zwj charscript
>> charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
>> cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
>> files window text-properties overlay sha1 md5 base64 format env
>> code-pages mule custom widget keymap hashtable-print-readable backquote
>> threads dbusbind inotify lcms2 dynamic-setting system-font-setting
>> font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x multi-tty
>> make-network-process native-compile emacs)
>>
>> Memory information:
>> ((conses 16 105675 8530)
>>  (symbols 48 8853 7)
>>  (strings 32 23487 2243)
>>  (string-bytes 1 691014)
>>  (vectors 16 17555)
>>  (vector-slots 8 353713 18074)
>>  (floats 8 39 42)
>>  (intervals 56 579 2)
>>  (buffers 984 14))

-- 
Cheers,
haj

Message #16 received at 76851 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Harald Jörg <haj <at> posteo.de>
Cc: John Ciolfi <ciolfi <at> mathworks.com>, 76851 <at> debbugs.gnu.org
Subject: Re: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix
Date: Sat, 8 Mar 2025 09:51:37 +0000

Harald Jörg <haj <at> posteo.de> writes:

> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>> John Ciolfi via "Bug reports for GNU Emacs, the Swiss army knife of text
>> editors" <bug-gnu-emacs <at> gnu.org> 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.

Message #19 received at 76851 <at> debbugs.gnu.org (full text, mbox):

From: Harald Jörg <haj <at> posteo.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: John Ciolfi <ciolfi <at> mathworks.com>, 76851 <at> debbugs.gnu.org
Subject: Re: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix
Date: Sat, 08 Mar 2025 13:22:51 +0000

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Harald Jörg <haj <at> posteo.de> writes:
>
>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>
>>> John Ciolfi via "Bug reports for GNU Emacs, the Swiss army knife of text
>>> editors" <bug-gnu-emacs <at> gnu.org> 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?
-- 
Cheers,
haj

Message #22 received at 76851 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Harald Jörg <haj <at> posteo.de>
Cc: John Ciolfi <ciolfi <at> mathworks.com>, 76851 <at> debbugs.gnu.org
Subject: Re: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix
Date: Sat, 8 Mar 2025 14:16:59 +0000

Harald Jörg <haj <at> posteo.de> writes:

> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>> Harald Jörg <haj <at> posteo.de> writes:
>>
>>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>>
>>>> John Ciolfi via "Bug reports for GNU Emacs, the Swiss army knife of text
>>>> editors" <bug-gnu-emacs <at> gnu.org> 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?

Message #25 received at 76851 <at> debbugs.gnu.org (full text, mbox):

From: John Ciolfi <ciolfi <at> mathworks.com>
To: Stefan Kangas <stefankangas <at> gmail.com>, Harald Jörg
 <haj <at> posteo.de>
Cc: "76851 <at> debbugs.gnu.org" <76851 <at> debbugs.gnu.org>
Subject: Re: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix
Date: Sat, 8 Mar 2025 15:04:37 +0000

[Message part 1 (text/plain, inline)]

Thanks for the quick responses. The change is not a regression. The issue has been there for a while. I debugged though the code and it is a very safe change. The frequency of hitting it is somewhat low. You have to start a function name using one of the built-in functions, e.g. if you use map_fcn it would trigger the bug because map is one of the built-ins. Not sure how many people would do that.

Thanks

________________________________
From: Stefan Kangas <stefankangas <at> gmail.com>
Sent: Saturday, March 8, 2025 9:16 AM
To: Harald Jörg <haj <at> posteo.de>
Cc: John Ciolfi <ciolfi <at> mathworks.com>; 76851 <at> debbugs.gnu.org <76851 <at> debbugs.gnu.org>
Subject: Re: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix

Harald Jörg <haj <at> posteo.de> writes:

> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>> Harald Jörg <haj <at> posteo.de> writes:
>>
>>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>>
>>>> John Ciolfi via "Bug reports for GNU Emacs, the Swiss army knife of text
>>>> editors" <bug-gnu-emacs <at> gnu.org> 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?

[Message part 2 (text/html, inline)]

Message #28 received at 76851 <at> debbugs.gnu.org (full text, mbox):

From: Harald Jörg <haj <at> posteo.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: John Ciolfi <ciolfi <at> mathworks.com>, 76851 <at> debbugs.gnu.org
Subject: Re: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix
Date: Sat, 08 Mar 2025 18:21:50 +0000

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Harald Jörg <haj <at> posteo.de> writes:
>
>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>
>>> Harald Jörg <haj <at> posteo.de> writes:
>>>
>>>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>>>
>>>>> John Ciolfi via "Bug reports for GNU Emacs, the Swiss army knife of text
>>>>> editors" <bug-gnu-emacs <at> gnu.org> 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.

Replacing end-of-word by end-of-symbol here is very low risk.  The same
change has been applied to other regular expressions in the past,
e.g. to fix Bug#18985.  The function patched here is one of the last
"tie-breakers" to distinguish whether a } character ends a statement.

> 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.

I _guess_ this bug sits here since 1999, commit 8f2222484972.  Before
that, the underscore character was considered a word character by CPerl
mode, so the regexp was "correct" unless someone fiddled with the option
cperl-under-as-char (deprecated since Emacs 24.4, and its replacement
superword-mode affects only movement, not regexps).

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

The chance to hit this bug is low, as indicated by the fact that it
didn't surface for that long.  I find it nasty, because it affects the
following code ... until it finds an extra semicolon which it takes as
end of the statement.

So, that's also a workaround: Add a semicolon after the closing brace.
    sub exec_fcn {...};

So, to summarize:
 + Risk: Very low.
 + Probability to hit: Very low.
 + Bug impact: annoying.
 + Workaround: exists and is easy (albeit ugly).

Pushing it to master works for me just fine, too.
-- 
Cheers,
haj

Message #31 received at 76851 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Harald Jörg <haj <at> posteo.de>
Cc: John Ciolfi <ciolfi <at> mathworks.com>, 76851 <at> debbugs.gnu.org
Subject: Re: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix
Date: Sun, 9 Mar 2025 11:10:29 +0000

Harald Jörg <haj <at> posteo.de> writes:

> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>> Harald Jörg <haj <at> posteo.de> writes:
>>
>>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>>
>>>> Harald Jörg <haj <at> posteo.de> writes:
>>>>
>>>>> Stefan Kangas <stefankangas <at> gmail.com> writes:
>>>>>
>>>>>> John Ciolfi via "Bug reports for GNU Emacs, the Swiss army knife of text
>>>>>> editors" <bug-gnu-emacs <at> gnu.org> 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.
>
> Replacing end-of-word by end-of-symbol here is very low risk.  The same
> change has been applied to other regular expressions in the past,
> e.g. to fix Bug#18985.  The function patched here is one of the last
> "tie-breakers" to distinguish whether a } character ends a statement.
>
>> 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.
>
> I _guess_ this bug sits here since 1999, commit 8f2222484972.  Before
> that, the underscore character was considered a word character by CPerl
> mode, so the regexp was "correct" unless someone fiddled with the option
> cperl-under-as-char (deprecated since Emacs 24.4, and its replacement
> superword-mode affects only movement, not regexps).
>
>> What is your estimation of this issue?  Do you consider it important to
>> fix on the release branch?
>
> The chance to hit this bug is low, as indicated by the fact that it
> didn't surface for that long.  I find it nasty, because it affects the
> following code ... until it finds an extra semicolon which it takes as
> end of the statement.
>
> So, that's also a workaround: Add a semicolon after the closing brace.
>     sub exec_fcn {...};
>
> So, to summarize:
>  + Risk: Very low.
>  + Probability to hit: Very low.
>  + Bug impact: annoying.
>  + Workaround: exists and is easy (albeit ugly).
>
> Pushing it to master works for me just fine, too.

Based on the above, and John Ciolfi's explanation, installing the fix on
emacs-30 sounds okay to me.  Thanks.

Message #36 received at 76851-done <at> debbugs.gnu.org (full text, mbox):

From: Harald Jörg <haj <at> posteo.de>
To: 76851-done <at> debbugs.gnu.org
Subject: Fixed in the repository: branch emacs-30
Date: Sun, 09 Mar 2025 16:09:38 +0000

This is now fixed in the release branch for Emacs and will be
published with the next bugfix release.
-- 
Cheers,
haj

Message #39 received at 76851 <at> debbugs.gnu.org (full text, mbox):

From: Harald Jörg <haj <at> posteo.de>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: John Ciolfi <ciolfi <at> mathworks.com>, 76851 <at> debbugs.gnu.org
Subject: Re: bug#76851: 29.4; cperl-mode builtin fcn indent bug and fix
Date: Sun, 09 Mar 2025 16:12:32 +0000

Stefan Kangas writes:

> [...]
> Based on the above, and John Ciolfi's explanation, installing the fix on
> emacs-30 sounds okay to me.  Thanks.

Done!  Thanks for your review and approval.
-- 
Cheers,
haj

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.