bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42

bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42

[Ken Mankoff]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

Tags: patch

Hello,

Following up from https://lists.gnu.org/archive/html/emacs-devel/2024-08/msg00904.html I'm submitting a patch to fix Fortran indentation due to an overly aggressive match for do loops.

In GNU Emacs 29.3 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.33,
 cairo version 1.16.0) of 2024-08-20 built on t480
Repository revision: ae8f815613c2e072e92aa8fe7b4bcf2fdabc7408
Repository branch: HEAD
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Ubuntu 22.04.5 LTS

Configured using:
 'configure --prefix=/home/kdm/local/emacs'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-Fortran-indent-below-do_not_a_loop-42.patch --]
[-- Type: text/patch, Size: 953 bytes --]

From ee63122df9d4ad4904030568d56d0ab9f2d200ea Mon Sep 17 00:00:00 2001
From: "Kenneth D. Mankoff" <mankoff@gmail.com>
Date: Thu, 12 Sep 2024 11:34:38 -0700
Subject: [PATCH] Fix Fortran indent below do_not_a_loop=42

---
 lisp/progmodes/fortran.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/progmodes/fortran.el b/lisp/progmodes/fortran.el
index 8a726dfe66e..0643df64f65 100644
--- a/lisp/progmodes/fortran.el
+++ b/lisp/progmodes/fortran.el
@@ -1631,7 +1631,7 @@ fortran-calculate-indent
                (setq icol (+ icol fortran-if-indent)))
               ((looking-at "where[ \t]*(.*)[ \t]*\n")
                (setq icol (+ icol fortran-if-indent)))
-              ((looking-at "do\\b")
+              ((looking-at "do[\\ |0-9]+.*=[\\ a-z0-9_]*,[\\ a-z0-9_]*")
                (setq icol (+ icol fortran-do-indent)))
               ((looking-at
                 "\\(structure\\|union\\|map\\|interface\\)\
-- 
2.34.1

bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42

[Eli Zaretskii]

> From: Ken Mankoff <km@kenmankoff.com>
> Date: Thu, 12 Sep 2024 11:44:44 -0700
> 
> Following up from https://lists.gnu.org/archive/html/emacs-devel/2024-08/msg00904.html I'm submitting a patch to fix Fortran indentation due to an overly aggressive match for do loops.

Thanks.

> --- a/lisp/progmodes/fortran.el
> +++ b/lisp/progmodes/fortran.el
> @@ -1631,7 +1631,7 @@ fortran-calculate-indent
>                 (setq icol (+ icol fortran-if-indent)))
>                ((looking-at "where[ \t]*(.*)[ \t]*\n")
>                 (setq icol (+ icol fortran-if-indent)))
> -              ((looking-at "do\\b")
> +              ((looking-at "do[\\ |0-9]+.*=[\\ a-z0-9_]*,[\\ a-z0-9_]*")

What do you intend with the likes of "[\\ |0-9]+" ?  Is this a
character alternative, or is that an alternative of matches?  If the
former, there's no need to escape a backslash, but then why is '|'
there?

IOW, can you please describe in plain English what did you intend this
regexp to match?

Thanks.

bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42

[Ken Mankoff]

Hi Eli,

On 2024-09-12 at 23:16 -07, Eli Zaretskii <eliz@gnu.org> wrote...
>> --- a/lisp/progmodes/fortran.el
>> +++ b/lisp/progmodes/fortran.el
>> @@ -1631,7 +1631,7 @@ fortran-calculate-indent
>>                 (setq icol (+ icol fortran-if-indent)))
>>                ((looking-at "where[ \t]*(.*)[ \t]*\n")
>>                 (setq icol (+ icol fortran-if-indent)))
>> -              ((looking-at "do\\b")
>> +              ((looking-at "do[\\ |0-9]+.*=[\\ a-z0-9_]*,[\\ a-z0-9_]*")
>
> What do you intend with the likes of "[\\ |0-9]+" ?  Is this a
> character alternative, or is that an alternative of matches?  If the
> former, there's no need to escape a backslash, but then why is '|'
> there?

I agree the '|' is not needed. I'm not sure what 'character alternative' or 'alternative of matches' means. I meant the '|' as an "OR" (is that an alterntative?), but realize now it is not needed. I now suggest

"do[\\ 0-9]+.*=[\\ a-z0-9_]*,[\\ a-z0-9_]*"

The pattern attempts to match "do", then either (space, numbers, or nothing), then equal sign, then something that looks like two numbers or valid variable names separated by a comma. I used these as tests:

      do42I=1,42 ! match
      do_foo = bar()
      do i = 1,42 ! match
      do i = 1,n ! match
      do i_var = a_var,b_var ! match
      do i_var5 = a_var,b_var ! match
      do42i_var = a_var,b_var ! match
      DO42 = [1,2]
      DO6I=5 7
      DO6I=5,7  ! match
      do_not_loop = [a,b]
      donot_loop = (/4,5/)
      donotloop = 42
      do_notloop = 42

Should I submit an updated patch? Or is the patch applier able to make this small change?

Thanks,

  -k.

bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42

[Eli Zaretskii]

> From: Ken Mankoff <km@kenmankoff.com>
> Cc: 73218@debbugs.gnu.org
> Date: Fri, 13 Sep 2024 08:44:45 -0700
> 
> >> --- a/lisp/progmodes/fortran.el
> >> +++ b/lisp/progmodes/fortran.el
> >> @@ -1631,7 +1631,7 @@ fortran-calculate-indent
> >>                 (setq icol (+ icol fortran-if-indent)))
> >>                ((looking-at "where[ \t]*(.*)[ \t]*\n")
> >>                 (setq icol (+ icol fortran-if-indent)))
> >> -              ((looking-at "do\\b")
> >> +              ((looking-at "do[\\ |0-9]+.*=[\\ a-z0-9_]*,[\\ a-z0-9_]*")
> >
> > What do you intend with the likes of "[\\ |0-9]+" ?  Is this a
> > character alternative, or is that an alternative of matches?  If the
> > former, there's no need to escape a backslash, but then why is '|'
> > there?
> 
> I agree the '|' is not needed. I'm not sure what 'character alternative' or 'alternative of matches' means. I meant the '|' as an "OR" (is that an alterntative?), but realize now it is not needed. I now suggest
> 
> "do[\\ 0-9]+.*=[\\ a-z0-9_]*,[\\ a-z0-9_]*"

Why do we need a backslashes before SPC?

Since the SPC is optional in Fortran, how about the following instead?

  "do *[0-9]* *[a-z0-9_]+ *= *[a-z0-9_]+ *, *[a-z0-9_]+"

> The pattern attempts to match "do", then either (space, numbers, or nothing), then equal sign, then something that looks like two numbers or valid variable names separated by a comma. I used these as tests:
> 
>       do42I=1,42 ! match
>       do_foo = bar()
>       do i = 1,42 ! match
>       do i = 1,n ! match
>       do i_var = a_var,b_var ! match
>       do i_var5 = a_var,b_var ! match
>       do42i_var = a_var,b_var ! match
>       DO42 = [1,2]
>       DO6I=5 7
>       DO6I=5,7  ! match
>       do_not_loop = [a,b]
>       donot_loop = (/4,5/)
>       donotloop = 42
>       do_notloop = 42

Please use the above to test my suggestion.

> Should I submit an updated patch? Or is the patch applier able to make this small change?

An updated patch would be better, but let's first find the correct
regexp to use.

Thanks.

bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42

[Ken Mankoff]

Hi Eli,

On 2024-09-14 at 04:15 -07, Eli Zaretskii <eliz@gnu.org> wrote...
>> From: Ken Mankoff <km@kenmankoff.com>
>> Cc: 73218@debbugs.gnu.org
>> Date: Fri, 13 Sep 2024 08:44:45 -0700
>> 
>> >> --- a/lisp/progmodes/fortran.el
>> >> +++ b/lisp/progmodes/fortran.el
>> >> @@ -1631,7 +1631,7 @@ fortran-calculate-indent
>> >>                 (setq icol (+ icol fortran-if-indent)))
>> >>                ((looking-at "where[ \t]*(.*)[ \t]*\n")
>> >>                 (setq icol (+ icol fortran-if-indent)))
>> >> -              ((looking-at "do\\b")
>> >> +              ((looking-at "do[\\ |0-9]+.*=[\\ a-z0-9_]*,[\\ a-z0-9_]*")
>
> Since the SPC is optional in Fortran, how about the following instead?
>
>   "do *[0-9]* *[a-z0-9_]+ *= *[a-z0-9_]+ *, *[a-z0-9_]+"
>

Yes this pattern works.

  -k.

bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42

[Eli Zaretskii]

> From: Ken Mankoff <km@kenmankoff.com>
> Cc: 73218@debbugs.gnu.org
> Date: Sat, 14 Sep 2024 07:09:02 -0700
> 
> Hi Eli,
> 
> On 2024-09-14 at 04:15 -07, Eli Zaretskii <eliz@gnu.org> wrote...
> >> From: Ken Mankoff <km@kenmankoff.com>
> >> Cc: 73218@debbugs.gnu.org
> >> Date: Fri, 13 Sep 2024 08:44:45 -0700
> >> 
> >> >> --- a/lisp/progmodes/fortran.el
> >> >> +++ b/lisp/progmodes/fortran.el
> >> >> @@ -1631,7 +1631,7 @@ fortran-calculate-indent
> >> >>                 (setq icol (+ icol fortran-if-indent)))
> >> >>                ((looking-at "where[ \t]*(.*)[ \t]*\n")
> >> >>                 (setq icol (+ icol fortran-if-indent)))
> >> >> -              ((looking-at "do\\b")
> >> >> +              ((looking-at "do[\\ |0-9]+.*=[\\ a-z0-9_]*,[\\ a-z0-9_]*")
> >
> > Since the SPC is optional in Fortran, how about the following instead?
> >
> >   "do *[0-9]* *[a-z0-9_]+ *= *[a-z0-9_]+ *, *[a-z0-9_]+"
> >
> 
> Yes this pattern works.

Thanks, so I've now installed this on the master branch, and I'm
closing this bug.