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

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

[Ola Nilsson]

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

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

[Richard Stallman]

[[[ 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)

bug#36317: [PATCH v2] Allow underscore in defun-prompt-regex names for sh-script

[Ola Nilsson]

* 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

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

[Ola Nilsson]

On Sat, Jun 22, 2019 at 6:13 AM Richard Stallman <rms@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

bug#36317: [PATCH v2] Allow underscore in defun-prompt-regex names for sh-script

[Noam Postavsky]

Ola Nilsson <ola.nilsson@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#36317: [PATCH v2] Allow underscore in defun-prompt-regex names for sh-script

[Ola Nilsson]

On Tue, Jun 25, 2019 at 4:01 PM Noam Postavsky <npostavs@gmail.com> wrote:
>
> Ola Nilsson <ola.nilsson@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

bug#36317: [PATCH v2] Allow underscore in defun-prompt-regex names for sh-script

[Noam Postavsky]

tags 36317 fixed
close 36317 27.1
quit

Ola Nilsson <ola.nilsson@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