bug#38257: 27.0.50; ERC does not match or highlight nick surrounded by parens

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#38257: 27.0.50; ERC does not match or highlight nick surrounded by parens

Amin Bandali-4

As the title says, ERC currently does not match
or highlight nicks when surrounded by a pair of
parens.  This has resulted in me not getting
notified of several important messages and
missing them.

I’ll send a patch fixing this shortly.

--
Amin Bandali
Free Software Activist | GNU Maintainer & Webmaster
GPG: BE62 7373 8E61 6D6D 1B3A  08E8 A21A 0202 4881 6103
https://bandali.eu.org



Reply | Threaded
Open this post in threaded view
|

bug#38257: Acknowledgement (27.0.50; ERC does not match or highlight nick surrounded by parens)

Amin Bandali-4
Here’s a patch that fixes the issue for me:


From 10f1fb7ccfb3e76dbd5df44f3f0e2780d24ba99b Mon Sep 17 00:00:00 2001
From: Amin Bandali <[hidden email]>
Date: Mon, 18 Nov 2019 10:24:48 -0500
Subject: [PATCH] Fix ERC not matching nicks surrounded parens (bug#38257)

* lisp/erc/erc-{button,match}.el (erc-{button,match}-syntax-table):
Omit parens, as they're not valid nick characters, per RFC 2812
section 2.3.1.
---
 lisp/erc/erc-button.el | 2 --
 lisp/erc/erc-match.el  | 2 --
 2 files changed, 4 deletions(-)

diff --git a/lisp/erc/erc-button.el b/lisp/erc/erc-button.el
index ec70260146..3dc4e92a46 100644
--- a/lisp/erc/erc-button.el
+++ b/lisp/erc/erc-button.el
@@ -223,8 +223,6 @@ erc-button-keymap
 
 (defvar erc-button-syntax-table
   (let ((table (make-syntax-table)))
-    (modify-syntax-entry ?\( "w" table)
-    (modify-syntax-entry ?\) "w" table)
     (modify-syntax-entry ?\[ "w" table)
     (modify-syntax-entry ?\] "w" table)
     (modify-syntax-entry ?\{ "w" table)
diff --git a/lisp/erc/erc-match.el b/lisp/erc/erc-match.el
index e9ed735516..e87076dbe3 100644
--- a/lisp/erc/erc-match.el
+++ b/lisp/erc/erc-match.el
@@ -246,8 +246,6 @@ erc-match-exclude-server-buffer
 ;; just put it in erc.el
 (defvar erc-match-syntax-table
   (let ((table (make-syntax-table)))
-    (modify-syntax-entry ?\( "w" table)
-    (modify-syntax-entry ?\) "w" table)
     (modify-syntax-entry ?\[ "w" table)
     (modify-syntax-entry ?\] "w" table)
     (modify-syntax-entry ?\{ "w" table)
--
2.23.0

Reply | Threaded
Open this post in threaded view
|

bug#38257: Acknowledgement (27.0.50; ERC does not match or highlight nick surrounded by parens)

Amin Bandali-4
Here’s a v2 that fixes the issue for
apostrophes as well, with a slightly tweaked
commit message.


From 1185e528641cbce618db93aa6604899e79300522 Mon Sep 17 00:00:00 2001
From: Amin Bandali <[hidden email]>
Date: Mon, 18 Nov 2019 10:24:48 -0500
Subject: [PATCH v2] Improve ERC's matching of nicks (bug#38257)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/erc/erc-{button,match}.el (erc-{button,match}-syntax-table):
Omit (, ), and '; as they're not valid nick characters, per RFC 2812
section 2.3.1.  This enables correct matching/highlighting of nicks
when they’re surrounded by parens, like (nick), and when adjacent to
an apostrophy, like nick's.
---
 lisp/erc/erc-button.el | 3 ---
 lisp/erc/erc-match.el  | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/lisp/erc/erc-button.el b/lisp/erc/erc-button.el
index ec70260146..002b99520a 100644
--- a/lisp/erc/erc-button.el
+++ b/lisp/erc/erc-button.el
@@ -223,14 +223,11 @@ erc-button-keymap
 
 (defvar erc-button-syntax-table
   (let ((table (make-syntax-table)))
-    (modify-syntax-entry ?\( "w" table)
-    (modify-syntax-entry ?\) "w" table)
     (modify-syntax-entry ?\[ "w" table)
     (modify-syntax-entry ?\] "w" table)
     (modify-syntax-entry ?\{ "w" table)
     (modify-syntax-entry ?\} "w" table)
     (modify-syntax-entry ?` "w" table)
-    (modify-syntax-entry ?' "w" table)
     (modify-syntax-entry ?^ "w" table)
     (modify-syntax-entry ?- "w" table)
     (modify-syntax-entry ?_ "w" table)
diff --git a/lisp/erc/erc-match.el b/lisp/erc/erc-match.el
index e9ed735516..336040a374 100644
--- a/lisp/erc/erc-match.el
+++ b/lisp/erc/erc-match.el
@@ -246,14 +246,11 @@ erc-match-exclude-server-buffer
 ;; just put it in erc.el
 (defvar erc-match-syntax-table
   (let ((table (make-syntax-table)))
-    (modify-syntax-entry ?\( "w" table)
-    (modify-syntax-entry ?\) "w" table)
     (modify-syntax-entry ?\[ "w" table)
     (modify-syntax-entry ?\] "w" table)
     (modify-syntax-entry ?\{ "w" table)
     (modify-syntax-entry ?\} "w" table)
     (modify-syntax-entry ?` "w" table)
-    (modify-syntax-entry ?' "w" table)
     (modify-syntax-entry ?^ "w" table)
     (modify-syntax-entry ?- "w" table)
     (modify-syntax-entry ?_ "w" table)
--
2.23.0

Reply | Threaded
Open this post in threaded view
|

bug#38257: Acknowledgement (27.0.50; ERC does not match or highlight nick surrounded by parens)

Lars Ingebrigtsen
Amin Bandali <[hidden email]> writes:

> * lisp/erc/erc-{button,match}.el (erc-{button,match}-syntax-table):
> Omit (, ), and '; as they're not valid nick characters, per RFC 2812
> section 2.3.1.  This enables correct matching/highlighting of nicks
> when they’re surrounded by parens, like (nick), and when adjacent to
> an apostrophy, like nick's.

I haven't tried this patch, but doesn't this syntax table apply to all
buttons in erc, including URLs?  So an URL like
http://foo.bar/zot'foo would no longer be recognised properly?  (I'm not
sure that it should, to be honest...)

--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



Reply | Threaded
Open this post in threaded view
|

bug#38257: Missed notif > broken buttons

Corwin Brust
In reply to this post by Amin Bandali-4
Chiming in to say that I've applied the patch in my local and I hope it will be applied/accepted. 

The functionality provided by buttons is essentially convenience.  Missing a notification can have real impact on my life.

Regards,
--
Corwin
corwin.brust (skype)
Reply | Threaded
Open this post in threaded view
|

bug#38257: Acknowledgement (27.0.50; ERC does not match or highlight nick surrounded by parens)

Amin Bandali-4
In reply to this post by Lars Ingebrigtsen
Lars Ingebrigtsen <[hidden email]> writes:

[...]
>
> I haven't tried this patch, but doesn't this syntax table apply to all
> buttons in erc, including URLs?  So an URL like
> http://foo.bar/zot'foo would no longer be recognised properly?  (I'm not
> sure that it should, to be honest...)

It does indeed, and it didn't quite occur to me
when writing the patch; whoops.

But like Corwin, I too personally much rather
be notified of my nick getting highlighted in
exchange for losing some convenience with
respect to clickable links.

That said, from a brief chat with Tom Tromey in
#erc on freenode today, I agree with him that
one probably shouldn't make changes to syntax
tables lightly, without first considering all
possible cases.  I wonder if you or Tom would
be so kind to do such an audit for this change.
I'd also be open to other ways of doing this if
there's a better way.  I guess in general it is
kind of tricky to deal with these characters in
URLs.  At least one person I talked to earlier
today said they would expect the apostrophe to
be part of the URL (i.e. not a word boundary).
It may be even more tricky or more subjective
for parens.

Thoughts?



Reply | Threaded
Open this post in threaded view
|

bug#38257: Acknowledgement (27.0.50; ERC does not match or highlight nick surrounded by parens)

Lars Ingebrigtsen
Amin Bandali <[hidden email]> writes:

> That said, from a brief chat with Tom Tromey in
> #erc on freenode today, I agree with him that
> one probably shouldn't make changes to syntax
> tables lightly, without first considering all
> possible cases.  I wonder if you or Tom would
> be so kind to do such an audit for this change.
> I'd also be open to other ways of doing this if
> there's a better way.  I guess in general it is
> kind of tricky to deal with these characters in
> URLs.  At least one person I talked to earlier
> today said they would expect the apostrophe to
> be part of the URL (i.e. not a word boundary).
> It may be even more tricky or more subjective
> for parens.
>
> Thoughts?

In general, you can't use syntax tables to give satisfactory URL DWIM
recognition.  Both of these things are common:

  There's a web page (http://foo.bar)

and

  There's a web page http://foo.bar/lala_(yes)

The latter is very common with Wikipedia URLs.

So I think for this to work with erc, I think it would make sense to
separate out the URL recognition in erc from other buttons, and base it
on browse-url-button-regexp instead, which has worked out these kinks
and is pretty DWIM.

And then you can change the syntax table to make the other, simpler
buttons (for nicks etc) work.

--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



Reply | Threaded
Open this post in threaded view
|

bug#38257: Acknowledgement (27.0.50; ERC does not match or highlight nick surrounded by parens)

Lars Ingebrigtsen
Lars Ingebrigtsen <[hidden email]> writes:

> So I think for this to work with erc, I think it would make sense to
> separate out the URL recognition in erc from other buttons, and base it
> on browse-url-button-regexp instead, which has worked out these kinks
> and is pretty DWIM.

(Note that I'm not familiar with how erc does URL recognition, so what I
wrote may be totally irrelevant.  :-))

--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



Reply | Threaded
Open this post in threaded view
|

bug#38257: Acknowledgement (27.0.50; ERC does not match or highlight nick surrounded by parens)

Amin Bandali-4
In reply to this post by Lars Ingebrigtsen
Lars Ingebrigtsen <[hidden email]> writes:

[...]

>
> So I think for this to work with erc, I think
> it would make sense to separate out the URL
> recognition in erc from other buttons, and
> base it on browse-url-button-regexp instead,
> which has worked out these kinks and is
> pretty DWIM.
>
> And then you can change the syntax table to
> make the other, simpler buttons (for nicks
> etc) work.

Nice, I think that would make sense.  FWIW, I'm
not familiar with how ERC does URL recognition
either, but I'll try to look into it soon-ish.
In the mean time, if anyone else is familiar
with this already, your help would be much
appreciated. :)

Also, I'll keep on using my patch on my local
checkout and will report back if I notice any
serious issues.  So far it's been working
pretty nicely here, minus the URL buttons.