* bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-09 16:57 bug#26037: 25.1; perl-mode add syntax support for subroutine signatures Evgeni Kolev
@ 2017-03-11 14:26 ` npostavs
2017-03-15 2:08 ` npostavs
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: npostavs @ 2017-03-11 14:26 UTC (permalink / raw)
To: Evgeni Kolev; +Cc: 26037
severity 26037 wishlist
quit
Evgeni Kolev <evgeni.d.kolev@gmail.com> writes:
> perl 5.20 (released May 2014) added experimental support for subroutine
> signatures. So this is valid perl code:
>
> sub test ($param) {
> ...
> }
>
> However, perl-mode's syntax rules treat everything between ( and ) as
> punctuation (syntax class "."). As a result (thing-at-point 'word)
> doesn't return $param when the point is on $param because $param is
> considered punctuation. The patch below tries to address this by
> using syntax class "@" inside the parens.
>
> diff --git a/lisp/progmodes/perl-mode.el b/lisp/progmodes/perl-mode.el
> index a516f07..2b9d9ad 100644
> --- a/lisp/progmodes/perl-mode.el
> +++ b/lisp/progmodes/perl-mode.el
> @@ -258,7 +258,7 @@
> ;; Funny things in `sub' arg-specs like `sub myfun ($)' or `sub ($)'.
> ;; Be careful not to match "sub { (...) ... }".
> ("\\<sub\\(?:[\s\t\n]+\\(?:\\sw\\|\\s_\\)+\\)?[\s\t\n]*(\\([^)]+\\))"
> - (1 "."))
> + (1 "@"))
Don't we still want 'sub ($)' and such to be marked with punctuation
syntax?
^ permalink raw reply [flat|nested] 14+ messages in thread* bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-09 16:57 bug#26037: 25.1; perl-mode add syntax support for subroutine signatures Evgeni Kolev
2017-03-11 14:26 ` npostavs
@ 2017-03-15 2:08 ` npostavs
2017-03-16 0:49 ` npostavs
2017-03-28 3:34 ` bug#26037: [Евгени Колев] " npostavs
3 siblings, 0 replies; 14+ messages in thread
From: npostavs @ 2017-03-15 2:08 UTC (permalink / raw)
To: Evgeni Kolev, 26037
[-- Attachment #1: Type: text/plain, Size: 42 bytes --]
[Please keep 26037@debbugs.gnu.org on CC]
[-- Attachment #2: Type: message/rfc822, Size: 2511 bytes --]
[-- Attachment #2.1.1: Type: text/plain, Size: 576 bytes --]
Hi, thanks for your reply!
The '$' inside 'sub ($)' should be treated as punctuation (or symbol,
sorry, I'm not 100% sure, see below) due to the "@" syntax class - if
I'm not mistaken, "@" will let the syntax "fall-through" to the
standard syntax table; the standard syntax table, in turn, should
define '$' and other perl sigils as punctuation.
However, I'm not sure how to verify that the '$' in 'sub ($)' is
punctuation or symbol.
I'm using this document for reference to syntax classes:
https://www.gnu.org/software/emacs/manual/html_node/elisp/Syntax-Class-Table.html
[-- Attachment #3: Type: text/plain, Size: 237 bytes --]
I believe the standard syntax table is fundamental mode's syntax table,
not perl's. Otherwise you might as well just delete the entry entirely.
You can check any character's current syntax by moving point to it, and
doing C-u C-x =.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-09 16:57 bug#26037: 25.1; perl-mode add syntax support for subroutine signatures Evgeni Kolev
2017-03-11 14:26 ` npostavs
2017-03-15 2:08 ` npostavs
@ 2017-03-16 0:49 ` npostavs
2017-03-18 14:24 ` Евгени Колев
2017-03-28 3:34 ` bug#26037: [Евгени Колев] " npostavs
3 siblings, 1 reply; 14+ messages in thread
From: npostavs @ 2017-03-16 0:49 UTC (permalink / raw)
To: 26037; +Cc: evgenysw
[-- Attachment #1: Type: text/plain, Size: 60 bytes --]
[Please use Reply All to keep 26037@debbugs.gnu.org on CC]
[-- Attachment #2: Type: message/rfc822, Size: 2671 bytes --]
[-- Attachment #2.1.1: Type: text/plain, Size: 859 bytes --]
OK, I see.
After a bit of testing with C-u C-x =, I see that with my proposed
change the sigils in 'sub test ($)' will *not* have punctuation class
as they did before the chagne.
I see these options:
1. I can try to improve the proposed change to preserve backward
compatibility - I can try to add another rule which will strictly
match only the allowed prototype-sigils $%&*;@[\]
2. discard my proposed change as it might have undesired side effects -
at this point, I don't see anything in perl-mode which depends on
these prototype-sigils having punctioation class, however, I could be
missing something.
3. accept my proposed change as-is, but we must be sure nothing will
be broken as a result. I'm sure syntax highlighting will not be
broken (I've tested with different themes, different perl sub
definitions).
Please let me know what you think.
[-- Attachment #3: Type: text/plain, Size: 190 bytes --]
I ran git blame against the relevant lines, and turned up Bug#18502.
Please check if your change breaks the indentation test case at
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=18502#5.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-16 0:49 ` npostavs
@ 2017-03-18 14:24 ` Евгени Колев
2017-03-18 15:58 ` npostavs
0 siblings, 1 reply; 14+ messages in thread
From: Евгени Колев @ 2017-03-18 14:24 UTC (permalink / raw)
To: npostavs; +Cc: 26037
[-- Attachment #1: Type: text/plain, Size: 2698 bytes --]
Yes, my change does break the indentation test case.
So changing the class doesn't seem like a good option.
After giving some thought to this, I think we can instead
make the regex more strict so that it matches only the
allowed characters in perl's prototypes. The allowed chars
are: $%&*;@[\]
This is what I have in mind - diff is pasted below.
However, please note the change below doesn't handle
these 3 chars [ / ] I'll add them if you think this is the correct
approach:
diff --git a/lisp/progmodes/perl-mode.el b/lisp/progmodes/perl-mode.el
index a516f07..840aa4e 100644
--- a/lisp/progmodes/perl-mode.el
+++ b/lisp/progmodes/perl-mode.el
@@ -257,7 +257,7 @@
(1 (prog1 "\"" (perl-syntax-propertize-special-constructs end))))
;; Funny things in `sub' arg-specs like `sub myfun ($)' or `sub ($)'.
;; Be careful not to match "sub { (...) ... }".
- ("\\<sub\\(?:[\s\t\n]+\\(?:\\sw\\|\\s_\\)+\\)?[\s\t\n]*(\\([^)]+\\))"
+
("\\<sub\\(?:[\s\t\n]+\\(?:\\sw\\|\\s_\\)+\\)?[\s\t\n]*(\\([$%&*;@]+\\))"
(1 "."))
;; Turn __DATA__ trailer into a comment.
("^\\(_\\)_\\(?:DATA\\|END\\)__[
\t]*\\(?:\\(\n\\)#.-\\*-.*perl.*-\\*-\\|\n.*\\)"
On Thu, Mar 16, 2017 at 2:49 AM, <npostavs@users.sourceforge.net> wrote:
> [Please use Reply All to keep 26037@debbugs.gnu.org on CC]
>
>
>
> ---------- Forwarded message ----------
> From: Evgeni Kolev <evgeni.d.kolev@gmail.com>
> To: npostavs@users.sourceforge.net
> Cc:
> Bcc:
> Date: Wed, 15 Mar 2017 12:00:32 +0200
> Subject: Re: bug#26037: 25.1; perl-mode add syntax support for subroutine
> signatures
> OK, I see.
>
> After a bit of testing with C-u C-x =, I see that with my proposed
> change the sigils in 'sub test ($)' will *not* have punctuation class
> as they did before the chagne.
>
> I see these options:
>
> 1. I can try to improve the proposed change to preserve backward
> compatibility - I can try to add another rule which will strictly
> match only the allowed prototype-sigils $%&*;@[\]
>
> 2. discard my proposed change as it might have undesired side effects -
> at this point, I don't see anything in perl-mode which depends on
> these prototype-sigils having punctioation class, however, I could be
> missing something.
>
> 3. accept my proposed change as-is, but we must be sure nothing will
> be broken as a result. I'm sure syntax highlighting will not be
> broken (I've tested with different themes, different perl sub
> definitions).
>
> Please let me know what you think.
>
>
>
> I ran git blame against the relevant lines, and turned up Bug#18502.
> Please check if your change breaks the indentation test case at
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=18502#5.
>
>
[-- Attachment #2: Type: text/html, Size: 3827 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread* bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-18 14:24 ` Евгени Колев
@ 2017-03-18 15:58 ` npostavs
2017-03-26 17:29 ` Евгени Колев
0 siblings, 1 reply; 14+ messages in thread
From: npostavs @ 2017-03-18 15:58 UTC (permalink / raw)
To: Евгени Колев
Cc: 26037
Евгени Колев <evgenysw@gmail.com> writes:
>
> After giving some thought to this, I think we can instead
> make the regex more strict so that it matches only the
> allowed characters in perl's prototypes. The allowed chars
> are: $%&*;@[\]
>
> This is what I have in mind - diff is pasted below.
> However, please note the change below doesn't handle
> these 3 chars [ / ] I'll add them if you think this is the correct
> approach:
Yeah, that looks right to me (although I don't know perl very well).
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-18 15:58 ` npostavs
@ 2017-03-26 17:29 ` Евгени Колев
2017-03-26 18:04 ` npostavs
0 siblings, 1 reply; 14+ messages in thread
From: Евгени Колев @ 2017-03-26 17:29 UTC (permalink / raw)
To: npostavs; +Cc: 26037
[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]
The regex is now updated to include all the chars $%&*;@[\]
I got the list of chars from here
http://perldoc.perl.org/perlsub.html#Prototypes
This is the diff, please let me if it can be improved:
diff --git a/lisp/progmodes/perl-mode.el b/lisp/progmodes/perl-mode.el
index a516f07..43eb462 100644
--- a/lisp/progmodes/perl-mode.el
+++ b/lisp/progmodes/perl-mode.el
@@ -257,7 +257,7 @@
(1 (prog1 "\"" (perl-syntax-propertize-special-constructs end))))
;; Funny things in `sub' arg-specs like `sub myfun ($)' or `sub ($)'.
;; Be careful not to match "sub { (...) ... }".
- ("\\<sub\\(?:[\s\t\n]+\\(?:\\sw\\|\\s_\\)+\\)?[\s\t\n]*(\\([^)]+\\))"
+
("\\<sub\\(?:[\s\t\n]+\\(?:\\sw\\|\\s_\\)+\\)?[\s\t\n]*(\\([][$%&*;@\\]+\\))"
(1 "."))
;; Turn __DATA__ trailer into a comment.
("^\\(_\\)_\\(?:DATA\\|END\\)__[
\t]*\\(?:\\(\n\\)#.-\\*-.*perl.*-\\*-\\|\n.*\\)"
On Sat, Mar 18, 2017 at 5:58 PM, <npostavs@users.sourceforge.net> wrote:
> Евгени Колев <evgenysw@gmail.com> writes:
>
> >
> > After giving some thought to this, I think we can instead
> > make the regex more strict so that it matches only the
> > allowed characters in perl's prototypes. The allowed chars
> > are: $%&*;@[\]
> >
> > This is what I have in mind - diff is pasted below.
> > However, please note the change below doesn't handle
> > these 3 chars [ / ] I'll add them if you think this is the correct
> > approach:
>
> Yeah, that looks right to me (although I don't know perl very well).
>
[-- Attachment #2: Type: text/html, Size: 2402 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread* bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-26 17:29 ` Евгени Колев
@ 2017-03-26 18:04 ` npostavs
0 siblings, 0 replies; 14+ messages in thread
From: npostavs @ 2017-03-26 18:04 UTC (permalink / raw)
To: Евгени Колев
Cc: 26037
Евгени Колев <evgenysw@gmail.com> writes:
> The regex is now updated to include all the chars $%&*;@[\]
>
> I got the list of chars from here
> http://perldoc.perl.org/perlsub.html#Prototypes
It looks like "+" is another possible character:
The + prototype is a special alternative to $ that will act like
\[@%] when given a literal array or hash variable, but will
otherwise force scalar context on the argument. This is useful for
functions which should accept either a literal array or an array
reference as the argument:
sub mypush (+@) {
>
> This is the diff, please let me if it can be improved:
>
> ;; Funny things in `sub' arg-specs like `sub myfun ($)' or `sub ($)'.
> ;; Be careful not to match "sub { (...) ... }".
> - ("\\<sub\\(?:[\s\t\n]+\\(?:\\sw\\|\\s_\\)+\\)?[\s\t\n]*(\\([^)]+\\))"
> + ("\\<sub\\(?:[\s\t\n]+\\(?:\\sw\\|\\s_\\)+\\)?[\s\t\n]*(\\([][$%&*;@\\]+\\))"
Please update the comment too.
And if it's not too much trouble could you add a commit message as
described in CONTRIBUTE? (And then post the output from 'git
format-patch' instead of 'git diff')
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26037: [Евгени Колев] Re: bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-09 16:57 bug#26037: 25.1; perl-mode add syntax support for subroutine signatures Evgeni Kolev
` (2 preceding siblings ...)
2017-03-16 0:49 ` npostavs
@ 2017-03-28 3:34 ` npostavs
2017-03-28 3:41 ` npostavs
3 siblings, 1 reply; 14+ messages in thread
From: npostavs @ 2017-03-28 3:34 UTC (permalink / raw)
To: 26037
Cc: Евгени Колев
[-- Attachment #1: Type: text/plain, Size: 60 bytes --]
[Please use Reply All to keep 26037@debbugs.gnu.org on Cc]
[-- Attachment #2: Type: message/rfc822, Size: 2544 bytes --]
[-- Attachment #2.1.1: Type: text/plain, Size: 1710 bytes --]
Sure! Here's output of format-patch, with `+' added to the regex and
updated comment, and again, please let me know if it can be improved:
From 250e022caf3889a1778820bac1ad072240fa93d4 Mon Sep 17 00:00:00 2001
From: Evgeni Kolev <evgenysw@gmail.com>
Date: Mon, 27 Mar 2017 09:30:10 +0300
Subject: [PATCH] Propertize only perl prototype chars `][$%&*;+@\' as
punctuation
As a result, variables in signatures such as `sub add ($a, $b) are not
treated as punctuation.
* lisp/progmodes/perl-mode.el (perl-syntax-propertize-function):
Strictly match only prototype characters as punctuation. (Bug#26037)
Copyright-paperwork-exempt: yes
---
lisp/progmodes/perl-mode.el | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/lisp/progmodes/perl-mode.el b/lisp/progmodes/perl-mode.el
index a516f07..1bcc743 100644
--- a/lisp/progmodes/perl-mode.el
+++ b/lisp/progmodes/perl-mode.el
@@ -255,9 +255,10 @@
;; format statements
("^[ \t]*format.*=[ \t]*\\(\n\\)"
(1 (prog1 "\"" (perl-syntax-propertize-special-constructs end))))
- ;; Funny things in `sub' arg-specs like `sub myfun ($)' or `sub ($)'.
- ;; Be careful not to match "sub { (...) ... }".
- ("\\<sub\\(?:[\s\t\n]+\\(?:\\sw\\|\\s_\\)+\\)?[\s\t\n]*(\\([^)]+\\))"
+ ;; Propertize perl prototype chars `][$%&*;+@\' as punctioation
+ ;; in `sub' arg-specs like `sub myfun ($)' or `sub ($)'. Be
+ ;; careful not to match "sub { (...) ... }".
+
("\\<sub\\(?:[\s\t\n]+\\(?:\\sw\\|\\s_\\)+\\)?[\s\t\n]*(\\([][$%&*;+@\\]+\\))"
(1 "."))
;; Turn __DATA__ trailer into a comment.
("^\\(_\\)_\\(?:DATA\\|END\\)__[
\t]*\\(?:\\(\n\\)#.-\\*-.*perl.*-\\*-\\|\n.*\\)"
--
2.10.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* bug#26037: [Евгени Колев] Re: bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-28 3:34 ` bug#26037: [Евгени Колев] " npostavs
@ 2017-03-28 3:41 ` npostavs
2017-03-29 6:28 ` Евгени Колев
0 siblings, 1 reply; 14+ messages in thread
From: npostavs @ 2017-03-28 3:41 UTC (permalink / raw)
To: 26037
Cc: Евгени Колев
npostavs@users.sourceforge.net writes:
> + ;; Propertize perl prototype chars `][$%&*;+@\' as punctioation
^^^^
Typo.
> + ;; in `sub' arg-specs like `sub myfun ($)' or `sub ($)'. Be
> + ;; careful not to match "sub { (...) ... }".
Perhaps you could mention that we're trying not match subroutine signatures?
^ permalink raw reply [flat|nested] 14+ messages in thread* bug#26037: [Евгени Колев] Re: bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-28 3:41 ` npostavs
@ 2017-03-29 6:28 ` Евгени Колев
2017-03-29 23:46 ` npostavs
0 siblings, 1 reply; 14+ messages in thread
From: Евгени Колев @ 2017-03-29 6:28 UTC (permalink / raw)
To: npostavs; +Cc: 26037
[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]
Sure, here's the updated patch:
From de1c6f786b8366423cca7edce1ecfabe763567d0 Mon Sep 17 00:00:00 2001
From: Evgeni Kolev <evgenysw@gmail.com>
Date: Mon, 27 Mar 2017 09:30:10 +0300
Subject: [PATCH] Propertize only perl prototype chars `][$%&*;+@\' as
punctuation
As a result, variables in signatures such as `sub add ($a, $b) are not
treated as punctuation.
* lisp/progmodes/perl-mode.el (perl-syntax-propertize-function):
Strictly match only prototype characters as punctuation. (Bug#26037)
Copyright-paperwork-exempt: yes
---
lisp/progmodes/perl-mode.el | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/lisp/progmodes/perl-mode.el b/lisp/progmodes/perl-mode.el
index a516f07..45628e3 100644
--- a/lisp/progmodes/perl-mode.el
+++ b/lisp/progmodes/perl-mode.el
@@ -255,9 +255,11 @@
;; format statements
("^[ \t]*format.*=[ \t]*\\(\n\\)"
(1 (prog1 "\"" (perl-syntax-propertize-special-constructs end))))
- ;; Funny things in `sub' arg-specs like `sub myfun ($)' or `sub ($)'.
- ;; Be careful not to match "sub { (...) ... }".
- ("\\<sub\\(?:[\s\t\n]+\\(?:\\sw\\|\\s_\\)+\\)?[\s\t\n]*(\\([^)]+\\))"
+ ;; Propertize perl prototype chars `$%&*;+@\[]' as punctuation
+ ;; in `sub' arg-specs like `sub myfun ($)' and `sub ($)'. But
+ ;; don't match subroutine signatures like `sub add ($a, $b)', or
+ ;; anonymous subs like "sub { (...) ... }".
+
("\\<sub\\(?:[\s\t\n]+\\(?:\\sw\\|\\s_\\)+\\)?[\s\t\n]*(\\([][$%&*;+@\\]+\\))"
(1 "."))
;; Turn __DATA__ trailer into a comment.
("^\\(_\\)_\\(?:DATA\\|END\\)__[
\t]*\\(?:\\(\n\\)#.-\\*-.*perl.*-\\*-\\|\n.*\\)"
--
2.10.0
[-- Attachment #2: Type: text/html, Size: 2383 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread* bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-29 6:28 ` Евгени Колев
@ 2017-03-29 23:46 ` npostavs
2017-04-01 21:40 ` npostavs
0 siblings, 1 reply; 14+ messages in thread
From: npostavs @ 2017-03-29 23:46 UTC (permalink / raw)
To: Евгени Колев
Cc: 26037
tags 26037 patch
quit
Евгени Колев <evgenysw@gmail.com> writes:
> Sure, here's the updated patch:
Thanks, looks good to me, I will push to master in a few days.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-03-29 23:46 ` npostavs
@ 2017-04-01 21:40 ` npostavs
2017-04-03 7:49 ` Евгени Колев
0 siblings, 1 reply; 14+ messages in thread
From: npostavs @ 2017-04-01 21:40 UTC (permalink / raw)
To: Евгени Колев
Cc: 26037
tags 26037 fixed
close 26037 26.1
quit
npostavs@users.sourceforge.net writes:
>
> Евгени Колев <evgenysw@gmail.com> writes:
>
>> Sure, here's the updated patch:
>
> Thanks, looks good to me, I will push to master in a few days.
Pushed to master [1: a184a7edc5] (I fixed a missing double space between
sentences and a missing ' in the commit message). Thanks for working on
this.
PS gmail line-wrapped your patch, it's better to send as attachment when
using gmail.
1: 2017-04-01 17:32:18 -0400 a184a7edc58e1e053aa317a0f162df7e225597e1
Propertize only perl prototype chars `][$%&*;+@\' as punctuation
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26037: 25.1; perl-mode add syntax support for subroutine signatures
2017-04-01 21:40 ` npostavs
@ 2017-04-03 7:49 ` Евгени Колев
0 siblings, 0 replies; 14+ messages in thread
From: Евгени Колев @ 2017-04-03 7:49 UTC (permalink / raw)
To: npostavs; +Cc: 26037
[-- Attachment #1: Type: text/plain, Size: 816 bytes --]
Thanks, and thanks for working on improving emacs!
On Sun, Apr 2, 2017 at 12:40 AM, <npostavs@users.sourceforge.net> wrote:
> tags 26037 fixed
> close 26037 26.1
> quit
>
> npostavs@users.sourceforge.net writes:
>
> >
> > Евгени Колев <evgenysw@gmail.com> writes:
> >
> >> Sure, here's the updated patch:
> >
> > Thanks, looks good to me, I will push to master in a few days.
>
> Pushed to master [1: a184a7edc5] (I fixed a missing double space between
> sentences and a missing ' in the commit message). Thanks for working on
> this.
>
> PS gmail line-wrapped your patch, it's better to send as attachment when
> using gmail.
>
> 1: 2017-04-01 17:32:18 -0400 a184a7edc58e1e053aa317a0f162df7e225597e1
> Propertize only perl prototype chars `][$%&*;+@\' as punctuation
>
>
[-- Attachment #2: Type: text/html, Size: 1383 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread