GNU bug report logs - #73218
[PATCH] Fix Fortran indent below do_not_a_loop=42

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 73218 in the body.
You can then email your comments to 73218 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: Ken Mankoff <km <at> kenmankoff.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Fix Fortran indent below do_not_a_loop=42
Date: Thu, 12 Sep 2024 11:44:44 -0700

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

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'

[0001-Fix-Fortran-indent-below-do_not_a_loop-42.patch (text/patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ken Mankoff <km <at> kenmankoff.com>
Cc: 73218 <at> debbugs.gnu.org
Subject: Re: bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42
Date: Fri, 13 Sep 2024 09:16:05 +0300

> From: Ken Mankoff <km <at> 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.

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

From: Ken Mankoff <km <at> kenmankoff.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 73218 <at> debbugs.gnu.org
Subject: Re: bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42
Date: Fri, 13 Sep 2024 08:44:45 -0700

Hi Eli,

On 2024-09-12 at 23:16 -07, Eli Zaretskii <eliz <at> 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.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ken Mankoff <km <at> kenmankoff.com>
Cc: 73218 <at> debbugs.gnu.org
Subject: Re: bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42
Date: Sat, 14 Sep 2024 14:15:32 +0300

> From: Ken Mankoff <km <at> kenmankoff.com>
> Cc: 73218 <at> 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.

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

From: Ken Mankoff <km <at> kenmankoff.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 73218 <at> debbugs.gnu.org
Subject: Re: bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42
Date: Sat, 14 Sep 2024 07:09:02 -0700

Hi Eli,

On 2024-09-14 at 04:15 -07, Eli Zaretskii <eliz <at> gnu.org> wrote...
>> From: Ken Mankoff <km <at> kenmankoff.com>
>> Cc: 73218 <at> 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.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ken Mankoff <km <at> kenmankoff.com>
Cc: 73218-done <at> debbugs.gnu.org
Subject: Re: bug#73218: [PATCH] Fix Fortran indent below do_not_a_loop=42
Date: Sat, 14 Sep 2024 17:20:01 +0300

> From: Ken Mankoff <km <at> kenmankoff.com>
> Cc: 73218 <at> 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 <at> gnu.org> wrote...
> >> From: Ken Mankoff <km <at> kenmankoff.com>
> >> Cc: 73218 <at> 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.

Previous Next

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