GNU bug report logs - #36317
[PATCH] Correct the name part of defun-prompt-regex in sh-script-mode

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 36317 in the body.
You can then email your comments to 36317 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: Ola Nilsson <ola.nilsson <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Cc: Ola Nilsson <ola.nilsson <at> gmail.com>
Subject: [PATCH] Correct the name part of defun-prompt-regex in sh-script-mode
Date: Fri, 21 Jun 2019 01:01:22 +0200

POSIX.1-2017 defines that functions should have a name that
'consisting solely of underscores, digits, and alphabetics from the
portable character set'.  Make sure the name part of
defun-prompt-regexp starts with a letter and allows underscores.

* lisp/progmodes/sh-script.el (defun-prompt-regexp):
Correct the function name part of the regexp.
---
 lisp/progmodes/sh-script.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 853a3500ee..24f242572f 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1619,9 +1619,9 @@ sh-mode
   (setq-local defun-prompt-regexp
               (concat
                "^\\("
-               "\\(function[ \t]\\)?[ \t]*[[:alnum:]]+[ \t]*([ \t]*)"
+               "\\(function[ \t]\\)?[ \t]*[[:alpha:]][[:alnum:]_]+[ \t]*([ \t]*)"
                "\\|"
-               "function[ \t]+[[:alnum:]]+[ \t]*\\(([ \t]*)\\)?"
+               "function[ \t]+[[:alpha:]][[:alnum:]_]+[ \t]*\\(([ \t]*)\\)?"
                "\\)[ \t]*"))
   (setq-local add-log-current-defun-function #'sh-current-defun-name)
   (add-hook 'completion-at-point-functions
-- 
2.11.0

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

From: Richard Stallman <rms <at> gnu.org>
To: Ola Nilsson <ola.nilsson <at> gmail.com>
Cc: 36317 <at> debbugs.gnu.org, ola.nilsson <at> gmail.com
Subject: Re: bug#36317: [PATCH] Correct the name part of defun-prompt-regex in
 sh-script-mode
Date: Sat, 22 Jun 2019 00:13:01 -0400

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > POSIX.1-2017 defines that functions should have a name that
  > 'consisting solely of underscores, digits, and alphabetics from the
  > portable character set'.

Ok.

			      Make sure the name part of
  > defun-prompt-regexp starts with a letter and allows underscores.

It should try to recognize anything that POSIX says is valid.
However, rejecting something just because POSIX says it is invalid
is a non-goal.

If the names that POSIX does not like actually work in some shells,
that is an _extension_.  Maybe some users use that extension.  If they
do use it, Emacs should highlight their code right.

Maybe shells don't support such function names.  If so, maybe the
change not to highlight them is good -- maybe.  Highlighting erroneous
function names might be helpful for the user.  When person runs the
script, and gets an error, person will change the name.  Until then,
highlighting the name during editing might still be helpful.

The point is, do not leap straight from "POSIX says this is invalid"
to "Emacs should not recognize it."  That is not the right way to
think about questions like this.

In the GNU Project, we treat standards as guides, not authorities.  We
follow standards when and as that serves users; we do not "obey" them.

See the GNU Coding Standards, section Non-GNU Standards.


-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

From: Ola Nilsson <ola.nilsson <at> gmail.com>
To: 36317 <at> debbugs.gnu.org
Cc: Ola Nilsson <ola.nilsson <at> gmail.com>
Subject: [PATCH v2] Allow underscore in defun-prompt-regex names for sh-script
Date: Sun, 23 Jun 2019 23:00:25 +0200

* lisp/progmodes/sh-script.el (defun-prompt-regexp):
Allow underscore in function names.
---
 lisp/progmodes/sh-script.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 853a3500ee..8a0ab20d70 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1619,9 +1619,9 @@ sh-mode
   (setq-local defun-prompt-regexp
               (concat
                "^\\("
-               "\\(function[ \t]\\)?[ \t]*[[:alnum:]]+[ \t]*([ \t]*)"
+               "\\(function[ \t]\\)?[ \t]*[[:alnum:]_]+[ \t]*([ \t]*)"
                "\\|"
-               "function[ \t]+[[:alnum:]]+[ \t]*\\(([ \t]*)\\)?"
+               "function[ \t]+[[:alnum:]_]+[ \t]*\\(([ \t]*)\\)?"
                "\\)[ \t]*"))
   (setq-local add-log-current-defun-function #'sh-current-defun-name)
   (add-hook 'completion-at-point-functions
-- 
2.11.0

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

From: Ola Nilsson <ola.nilsson <at> gmail.com>
To: rms <at> gnu.org
Cc: 36317 <at> debbugs.gnu.org
Subject: Re: bug#36317: [PATCH] Correct the name part of defun-prompt-regex in
 sh-script-mode
Date: Mon, 24 Jun 2019 22:23:18 +0200

On Sat, Jun 22, 2019 at 6:13 AM Richard Stallman <rms <at> gnu.org> wrote:
>
> It should try to recognize anything that POSIX says is valid.
> However, rejecting something just because POSIX says it is invalid
> is a non-goal.

Ok, that makes total sense and is probably mentioned in the POSIX specification.

> Maybe shells don't support such function names.  If so, maybe the
> change not to highlight them is good -- maybe.  Highlighting erroneous
> function names might be helpful for the user.  When person runs the
> script, and gets an error, person will change the name.  Until then,
> highlighting the name during editing might still be helpful.

This is not actually about highlighting, but function navigation.
beginning-of-defun, end-of-defun, and narrow-to-defun does not
behave as expected for functions with underscores in their names.

I've sent a V2 patch.

-- 
Ola Nilsson

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Ola Nilsson <ola.nilsson <at> gmail.com>
Cc: 36317 <at> debbugs.gnu.org
Subject: Re: bug#36317: [PATCH v2] Allow underscore in defun-prompt-regex
 names for sh-script
Date: Tue, 25 Jun 2019 10:01:04 -0400

Ola Nilsson <ola.nilsson <at> gmail.com> writes:

> * lisp/progmodes/sh-script.el (defun-prompt-regexp):
> Allow underscore in function names.

Do you think we should allow dashes as too (as suggested in Bug#21477)?

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

From: Ola Nilsson <ola.nilsson <at> gmail.com>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 36317 <at> debbugs.gnu.org
Subject: Re: bug#36317: [PATCH v2] Allow underscore in defun-prompt-regex
 names for sh-script
Date: Wed, 26 Jun 2019 22:58:06 +0200

On Tue, Jun 25, 2019 at 4:01 PM Noam Postavsky <npostavs <at> gmail.com> wrote:
>
> Ola Nilsson <ola.nilsson <at> gmail.com> writes:
>
> > * lisp/progmodes/sh-script.el (defun-prompt-regexp):
> > Allow underscore in function names.
>
> Do you think we should allow dashes as too (as suggested in Bug#21477)?

Bug#21477 will not be fixed by changing defun-prompt-regexp.

sh-script.el does not have a centralized way of dealing with function names.

* imenu (the problem in Bug#21477) does not use defun-prompt-regexp,
but rather its
  own regexp found in sh-imenu-generic-expression.  Funnily enough
those function
  names _do_ follow the posix standard (letters, digits, underscore,
must not start with
  a digit).
* sh-current-defun-name also has its own regexps, matching those of
  sh-imenu-generic-expression.
* sh-font-lock-keyword-var uses "\\sw+" which is a lot less strict with what
  characters can be included.
* sh-completion-at-point-function uses "[[:alnum:]_]" to skip to
beginning and end
  of a function name.

I assume there are some other cases that I missed.

As far as I can tell from a quick search of shell manuals and net
questions most shells stick to the posix spec. Bash is the exception
and seems to be very forgiving at least as long as you use the
'function name() {...}' format.

So I guess it boils down to how allowing we want to be.

-- 
Ola Nilsson

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Ola Nilsson <ola.nilsson <at> gmail.com>
Cc: 36317 <at> debbugs.gnu.org
Subject: Re: bug#36317: [PATCH v2] Allow underscore in defun-prompt-regex
 names for sh-script
Date: Thu, 27 Jun 2019 20:15:17 -0400

tags 36317 fixed
close 36317 27.1
quit

Ola Nilsson <ola.nilsson <at> gmail.com> writes:

> sh-script.el does not have a centralized way of dealing with function names.
>
> * imenu (the problem in Bug#21477) does not use defun-prompt-regexp,
> but rather its
>   own regexp found in sh-imenu-generic-expression.  Funnily enough
> those function
>   names _do_ follow the posix standard (letters, digits, underscore,
> must not start with
>   a digit).
> * sh-current-defun-name also has its own regexps, matching those of
>   sh-imenu-generic-expression.
> * sh-font-lock-keyword-var uses "\\sw+" which is a lot less strict with what
>   characters can be included.
> * sh-completion-at-point-function uses "[[:alnum:]_]" to skip to
> beginning and end
>   of a function name.
>
> I assume there are some other cases that I missed.

Darn, that's a lot more complicated than I was hoping.

> As far as I can tell from a quick search of shell manuals and net
> questions most shells stick to the posix spec. Bash is the exception
> and seems to be very forgiving at least as long as you use the
> 'function name() {...}' format.
>
> So I guess it boils down to how allowing we want to be.

IMO, allowing more things makes sense, but I've just gone ahead and push
your patch v2 to master, since it's clearly a step in the right
direction.  Cleaning up the rest of the mess would be nice too, but it
looks like a lot of untangling to be done.

fa3af359df 2019-06-27T20:02:54-04:00 "Allow underscore in defun-prompt-regex names for sh-script"
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=fa3af359df8754423a197682d31245ad88c02033

Previous Next

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