bug#17558: 24.4.50; global-subword-mode breaks ERC

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Dima Kogan]

Hi. I'm using a very recent build of emacs from the VCS. With this build
ERC gets very confused if global-subword-mode is on. I believe it worked
fine just a few months ago.

Recipe:

1. emacs -Q
2. (global-subword-mode)
3. M-x erc
4. Default values for the server, nickname, password, etc are fine to
show the issue

I observe the following two error repeated many times:

error in process filter: erc-button-add-nickname-buttons: Wrong type argument: integer-or-marker-p, nil
error in process filter: Wrong type argument: integer-or-marker-p, nil

ERC kinda works even with those issues, but the data comes back from the
server sporadically and it's not really usable.

Emacs details:

In GNU Emacs 24.4.50.1 (x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2014-05-21 on lilbastard, modified by Debian
 (emacs-snapshot package, version 2:20140521-1)
Windowing system distributor `The X.Org Foundation', version 11.0.11501000
System Description:	Debian GNU/Linux testing (jessie)

Configured using:
 `configure --build x86_64-linux-gnu --host x86_64-linux-gnu
 --prefix=/usr --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var --infodir=/usr/share/info/emacs-snapshot
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/24.4.50/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.4.50/site-lisp:/usr/share/emacs/site-lisp
 --with-crt-dir=/usr/lib/x86_64-linux-gnu/ --with-x=yes
 --with-x-toolkit=lucid 'CFLAGS=-DDEBIAN -DSITELOAD_PURESIZE_EXTRA=5000
 -g -O2' CPPFLAGS=-D_FORTIFY_SOURCE=2 'LDFLAGS=-g -Wl,--as-needed
 -znocombreloc''

Configured features:
XAW3D XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF
GSETTINGS NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF
XFT ZLIB

Important settings:
  value of $LC_ALL: C
  value of $LANG: C
  locale-coding-system: nil

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

On 05/23/2014 02:19 AM, Dima Kogan wrote:
> Hi. I'm using a very recent build of emacs from the VCS. With this build
> ERC gets very confused if global-subword-mode is on. I believe it worked
> fine just a few months ago.
> 
> Recipe:
> 
> 1. emacs -Q
> 2. (global-subword-mode)
> 3. M-x erc
> 4. Default values for the server, nickname, password, etc are fine to
> show the issue

Thanks for the repro. This bug is probably due to my changing how
subword-mode works: now, forward-word will stop at subword boundaries,
and I'm guessing ERC was relying on the old behavior. I can try to take
a look at it if nobody who knows ERC better wants to try first.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Dima Kogan]

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

Daniel Colascione <dancol@dancol.org> writes:

> On 05/23/2014 02:19 AM, Dima Kogan wrote:
>> Hi. I'm using a very recent build of emacs from the VCS. With this build
>> ERC gets very confused if global-subword-mode is on. I believe it worked
>> fine just a few months ago.
>> 
>> Recipe:
>> 
>> 1. emacs -Q
>> 2. (global-subword-mode)
>> 3. M-x erc
>> 4. Default values for the server, nickname, password, etc are fine to
>> show the issue
>
> Thanks for the repro. This bug is probably due to my changing how
> subword-mode works: now, forward-word will stop at subword boundaries,
> and I'm guessing ERC was relying on the old behavior. I can try to take
> a look at it if nobody who knows ERC better wants to try first.

Here's a patch. It works, but I don't like how un-future-proof it is. An
uncareful coder can simply use (forward-word) somewhere and get things
to break again. Is there a better way?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ERC-no-longer-gets-confused-by-subword-mode.patch --]
[-- Type: text/x-diff, Size: 3354 bytes --]

From 655e7b9b5735bd62aac104645b5a224636ab97ff Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Tue, 30 Dec 2014 23:29:21 -0800
Subject: [PATCH] ERC no longer gets confused by subword-mode

In commit 6ddc44225e743e2b2a0d5c192f50aefd7a4a915b subword-mode was
integrated into the syntax table instead of simply remapping the
interactive motion bindings as was done previously.  This had the
unintended effect of changing the behavior of lisp programs that touch
words.  In the case of ERC, it completely broke it: emacs now throws an
error when ERC is launched, making it unusable when subword-mode is
active.

This commit temporarily disables subword-mode so that ERC functions that
touch words do so the "normal" way.

Closes: #17558
---
 lisp/erc/erc-backend.el |  9 ++++++++-
 lisp/erc/erc-button.el  | 23 +++++++++++++++--------
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index bedb20f..fa95d7e 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -480,7 +480,14 @@ Currently this is called by `erc-send-input'."
   (with-temp-buffer
     (insert str)
     (goto-char (point-min))
-    (upcase-word 1)
+
+    ;; If we're in subword-mode then functions operating on words act
+    ;; differently. Here I temporarily disable subword-mode before
+    ;; touching the words
+    (let ((find-word-boundary-function-table
+           (if (boundp 'subword-empty-char-table)
+               subword-empty-char-table find-word-boundary-function-table)))
+      (upcase-word 1))
     (buffer-string)))
 
 (defun erc-server-setup-periodical-ping (buffer)
diff --git a/lisp/erc/erc-button.el b/lisp/erc/erc-button.el
index 10e7318..8343425 100644
--- a/lisp/erc/erc-button.el
+++ b/lisp/erc/erc-button.el
@@ -300,14 +300,21 @@ specified by `erc-button-alist'."
     (when (or (eq t form)
               (eval form))
       (goto-char (point-min))
-      (while (forward-word 1)
-        (setq bounds (bounds-of-thing-at-point 'word))
-        (setq word (buffer-substring-no-properties
-                    (car bounds) (cdr bounds)))
-        (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
-                  (and erc-channel-users (erc-get-channel-user word)))
-          (erc-button-add-button (car bounds) (cdr bounds)
-                                 fun t (list word)))))))
+
+      ;; If we're in subword-mode then functions operating on words
+      ;; act differently. Here I temporarily disable subword-mode
+      ;; before touching the words
+      (let ((find-word-boundary-function-table
+             (if (boundp 'subword-empty-char-table)
+                 subword-empty-char-table find-word-boundary-function-table)))
+        (while (forward-word 1)
+          (setq bounds (bounds-of-thing-at-point 'word))
+          (setq word (buffer-substring-no-properties
+                      (car bounds) (cdr bounds)))
+          (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
+                    (and erc-channel-users (erc-get-channel-user word)))
+            (erc-button-add-button (car bounds) (cdr bounds)
+                                   fun t (list word))))))))
 
 (defun erc-button-add-buttons-1 (regexp entry)
   "Search through the buffer for matches to ENTRY and add buttons."
-- 
2.1.3

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Dima Kogan]

[-- Attachment #1: Type: text/plain, Size: 911 bytes --]


Daniel Colascione <dancol@dancol.org> writes:

> On 05/23/2014 02:19 AM, Dima Kogan wrote:
>> Hi. I'm using a very recent build of emacs from the VCS. With this build
>> ERC gets very confused if global-subword-mode is on. I believe it worked
>> fine just a few months ago.
>> 
>> Recipe:
>> 
>> 1. emacs -Q
>> 2. (global-subword-mode)
>> 3. M-x erc
>> 4. Default values for the server, nickname, password, etc are fine to
>> show the issue
>
> Thanks for the repro. This bug is probably due to my changing how
> subword-mode works: now, forward-word will stop at subword boundaries,
> and I'm guessing ERC was relying on the old behavior. I can try to take
> a look at it if nobody who knows ERC better wants to try first.


Here's a patch. It works, but I don't like how un-future-proof it is. An
uncareful coder can simply use (forward-word) somewhere and get things
to break again. Is there a better way?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ERC-no-longer-gets-confused-by-subword-mode.patch --]
[-- Type: text/x-diff, Size: 3354 bytes --]

From 655e7b9b5735bd62aac104645b5a224636ab97ff Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Tue, 30 Dec 2014 23:29:21 -0800
Subject: [PATCH] ERC no longer gets confused by subword-mode

In commit 6ddc44225e743e2b2a0d5c192f50aefd7a4a915b subword-mode was
integrated into the syntax table instead of simply remapping the
interactive motion bindings as was done previously.  This had the
unintended effect of changing the behavior of lisp programs that touch
words.  In the case of ERC, it completely broke it: emacs now throws an
error when ERC is launched, making it unusable when subword-mode is
active.

This commit temporarily disables subword-mode so that ERC functions that
touch words do so the "normal" way.

Closes: #17558
---
 lisp/erc/erc-backend.el |  9 ++++++++-
 lisp/erc/erc-button.el  | 23 +++++++++++++++--------
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index bedb20f..fa95d7e 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -480,7 +480,14 @@ Currently this is called by `erc-send-input'."
   (with-temp-buffer
     (insert str)
     (goto-char (point-min))
-    (upcase-word 1)
+
+    ;; If we're in subword-mode then functions operating on words act
+    ;; differently. Here I temporarily disable subword-mode before
+    ;; touching the words
+    (let ((find-word-boundary-function-table
+           (if (boundp 'subword-empty-char-table)
+               subword-empty-char-table find-word-boundary-function-table)))
+      (upcase-word 1))
     (buffer-string)))
 
 (defun erc-server-setup-periodical-ping (buffer)
diff --git a/lisp/erc/erc-button.el b/lisp/erc/erc-button.el
index 10e7318..8343425 100644
--- a/lisp/erc/erc-button.el
+++ b/lisp/erc/erc-button.el
@@ -300,14 +300,21 @@ specified by `erc-button-alist'."
     (when (or (eq t form)
               (eval form))
       (goto-char (point-min))
-      (while (forward-word 1)
-        (setq bounds (bounds-of-thing-at-point 'word))
-        (setq word (buffer-substring-no-properties
-                    (car bounds) (cdr bounds)))
-        (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
-                  (and erc-channel-users (erc-get-channel-user word)))
-          (erc-button-add-button (car bounds) (cdr bounds)
-                                 fun t (list word)))))))
+
+      ;; If we're in subword-mode then functions operating on words
+      ;; act differently. Here I temporarily disable subword-mode
+      ;; before touching the words
+      (let ((find-word-boundary-function-table
+             (if (boundp 'subword-empty-char-table)
+                 subword-empty-char-table find-word-boundary-function-table)))
+        (while (forward-word 1)
+          (setq bounds (bounds-of-thing-at-point 'word))
+          (setq word (buffer-substring-no-properties
+                      (car bounds) (cdr bounds)))
+          (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
+                    (and erc-channel-users (erc-get-channel-user word)))
+            (erc-button-add-button (car bounds) (cdr bounds)
+                                   fun t (list word))))))))
 
 (defun erc-button-add-buttons-1 (regexp entry)
   "Search through the buffer for matches to ENTRY and add buttons."
-- 
2.1.3

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Stefan Monnier]

>> Thanks for the repro. This bug is probably due to my changing how
>> subword-mode works: now, forward-word will stop at subword boundaries,
>> and I'm guessing ERC was relying on the old behavior. I can try to take
>> a look at it if nobody who knows ERC better wants to try first.

Indeed, with the new implementation of subword-mode, most of the
word-operating commands should be marked as "interactive use only",
since their behavior is too unreliable for use in Lisp code.

> -    (upcase-word 1)
> +
> +    ;; If we're in subword-mode then functions operating on words act
> +    ;; differently. Here I temporarily disable subword-mode before
> +    ;; touching the words
> +    (let ((find-word-boundary-function-table
> +           (if (boundp 'subword-empty-char-table)
> +               subword-empty-char-table find-word-boundary-function-table)))
> +      (upcase-word 1))

Yuck.  I suggest you use upcase-region instead (and probably something
like skip-char-forward to find the word's boundaries).

> -      (while (forward-word 1)
> -        (setq bounds (bounds-of-thing-at-point 'word))
> -        (setq word (buffer-substring-no-properties
> -                    (car bounds) (cdr bounds)))
> -        (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
> -                  (and erc-channel-users (erc-get-channel-user word)))
> -          (erc-button-add-button (car bounds) (cdr bounds)
> -                                 fun t (list word)))))))
> +
> +      ;; If we're in subword-mode then functions operating on words
> +      ;; act differently. Here I temporarily disable subword-mode
> +      ;; before touching the words
> +      (let ((find-word-boundary-function-table
> +             (if (boundp 'subword-empty-char-table)
> +                 subword-empty-char-table find-word-boundary-function-table)))
> +        (while (forward-word 1)
> +          (setq bounds (bounds-of-thing-at-point 'word))
> +          (setq word (buffer-substring-no-properties
> +                      (car bounds) (cdr bounds)))
> +          (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
> +                    (and erc-channel-users (erc-get-channel-user word)))
> +            (erc-button-add-button (car bounds) (cdr bounds)
> +                                   fun t (list word))))))))
 
Similarly here, we should use "word" operations at all.
Maybe forward-sexp can be used instead of forward-word, or else
skip-syntax-forward or skip-chars-forward.

The call to (bounds-of-thing-at-point 'word) should be avoided as well
(we already know the end of the bounds is (point) anyway and we can
probably find the other bound at the same time as we skip-chars-forward.


        Stefan

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Dima Kogan]

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> -    (upcase-word 1)
>> +
>> +    ;; If we're in subword-mode then functions operating on words act
>> +    ;; differently. Here I temporarily disable subword-mode before
>> +    ;; touching the words
>> +    (let ((find-word-boundary-function-table
>> +           (if (boundp 'subword-empty-char-table)
>> +               subword-empty-char-table find-word-boundary-function-table)))
>> +      (upcase-word 1))
>
> Yuck.  I suggest you use upcase-region instead (and probably something
> like skip-char-forward to find the word's boundaries).

OK. New patch attached. It's a bit less ugly, maybe; still pretty
verbose. Marking the -word functions as interactive-only would be a
great thing to do.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ERC-no-longer-gets-confused-by-subword-mode.patch --]
[-- Type: text/x-diff, Size: 3392 bytes --]

From 3017aba1093c8f57d3a3b7193692d62a56b68d3b Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Tue, 30 Dec 2014 23:29:21 -0800
Subject: [PATCH] ERC no longer gets confused by subword-mode

In commit 6ddc44225e743e2b2a0d5c192f50aefd7a4a915b subword-mode was
integrated into the syntax table instead of simply remapping the
interactive motion bindings as was done previously.  This had the
unintended effect of changing the behavior of lisp programs that touch
words.  In the case of ERC, it completely broke it: emacs now throws an
error when ERC is launched, making it unusable when subword-mode is
active.

This commit replaces the word-oriented calls with ones that navigate
the buffer using syntax classes

Closes: #17558
---
 lisp/erc/erc-backend.el | 11 ++++++++++-
 lisp/erc/erc-button.el  | 30 ++++++++++++++++++++++--------
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 1ef494c..0e80438 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -480,7 +480,16 @@ Currently this is called by `erc-send-input'."
   (with-temp-buffer
     (insert str)
     (goto-char (point-min))
-    (upcase-word 1)
+
+    ;; this is (upcase-word 1), but working even with subword-mode
+    ;; active
+    (skip-syntax-forward "^w")
+    (let*
+        ((word-start (point))
+         (word-end
+          (progn (skip-syntax-forward "w") (point))))
+      (upcase-region word-start word-end))
+
     (buffer-string)))
 
 (defun erc-server-setup-periodical-ping (buffer)
diff --git a/lisp/erc/erc-button.el b/lisp/erc/erc-button.el
index 10e7318..d194627 100644
--- a/lisp/erc/erc-button.el
+++ b/lisp/erc/erc-button.el
@@ -300,14 +300,28 @@ specified by `erc-button-alist'."
     (when (or (eq t form)
               (eval form))
       (goto-char (point-min))
-      (while (forward-word 1)
-        (setq bounds (bounds-of-thing-at-point 'word))
-        (setq word (buffer-substring-no-properties
-                    (car bounds) (cdr bounds)))
-        (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
-                  (and erc-channel-users (erc-get-channel-user word)))
-          (erc-button-add-button (car bounds) (cdr bounds)
-                                 fun t (list word)))))))
+
+      (while
+          (progn
+
+            ;; I move forward a word (independent of subword-mode) ...
+            (skip-syntax-forward "^w")
+            (let*
+                ((word-start (point))
+                 (word-end
+                  (progn (skip-syntax-forward "w") (point))))
+
+              ;; ... if the word was empty we're at the end of buffer ...
+              (and (/= word-start word-end)
+
+                   ;; ... otherwise, we do stuff with this word
+                   (progn
+                     (setq word (buffer-substring-no-properties
+                                 word-start word-end))
+                     (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
+                               (and erc-channel-users (erc-get-channel-user word)))
+                       (erc-button-add-button word-start word-end
+                                              fun t (list word)))))))))))
 
 (defun erc-button-add-buttons-1 (regexp entry)
   "Search through the buffer for matches to ENTRY and add buttons."
-- 
2.1.3

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Glenn Morris]

Stefan Monnier wrote:

> Indeed, with the new implementation of subword-mode, most of the
> word-operating commands should be marked as "interactive use only",
> since their behavior is too unreliable for use in Lisp code.

Sounds to me like it will be a PITA to review/replace every
non-interactive usage of those commands.

Are people certain they want to go down this road?

(At time of writing, I don't see any mention of forward-word etc no
longer being safe in NEWS.)

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 911 bytes --]

On 04/27/2015 01:03 PM, Glenn Morris wrote:
> Stefan Monnier wrote:
> 
>> Indeed, with the new implementation of subword-mode, most of the
>> word-operating commands should be marked as "interactive use only",
>> since their behavior is too unreliable for use in Lisp code.
> 
> Sounds to me like it will be a PITA to review/replace every
> non-interactive usage of those commands.
> 
> Are people certain they want to go down this road?

I am. It's the only way we can make sure that interactive commands that
move by words *indirectly* do the right thing in the presence of user
customizations.

> (At time of writing, I don't see any mention of forward-word etc no
> longer being safe in NEWS.)

ERC is the only component I've heard of breaking. I don't think
forward-word has *ever* been "safe" in the sense that you're suggesting.
Modes almost always *actually* want sexp movement.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Drew Adams]

> >> Indeed, with the new implementation of subword-mode, most of the
> >> word-operating commands should be marked as "interactive use
> >> only", since their behavior is too unreliable for use in Lisp code.
> >
> > Sounds to me like it will be a PITA to review/replace every
> > non-interactive usage of those commands.
> > Are people certain they want to go down this road?
> 
> I am. It's the only way we can make sure that interactive commands
> that move by words *indirectly* do the right thing in the presence of
> user customizations.

Mille excuses - I have not been following this thread.

Just what is meant by "non-interactive use of `forward-word'
and `backward-word'"?  Any call to either of them in Lisp code?
And what is meant by "*indirectly*".

Will you be telling users (e.g. in NEWS) precisely *how* they
need to modify existing non-interactive calls to those functions?
Saying that they should be used only interactively from now on
does not tell users how to fix existing code that calls them.

And just what is unsafe about calling these two functions from
Lisp?

Sorry, but this is not clear to me.  (And why not create new
functions/commands, instead of changing the meaning/behavior
of these longstanding ones?)

> > (At time of writing, I don't see any mention of forward-word etc
> > no longer being safe in NEWS.)
> 
> ERC is the only component I've heard of breaking. I don't think
> forward-word has *ever* been "safe" in the sense that you're
> suggesting.  Modes almost always *actually* want sexp movement.

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 2437 bytes --]

On 04/27/2015 04:24 PM, Drew Adams wrote:
>>>> Indeed, with the new implementation of subword-mode, most of the
>>>> word-operating commands should be marked as "interactive use
>>>> only", since their behavior is too unreliable for use in Lisp code.
>>>
>>> Sounds to me like it will be a PITA to review/replace every
>>> non-interactive usage of those commands.
>>> Are people certain they want to go down this road?
>>
>> I am. It's the only way we can make sure that interactive commands
>> that move by words *indirectly* do the right thing in the presence of
>> user customizations.
> 
> Mille excuses - I have not been following this thread.
> 
> Just what is meant by "non-interactive use of `forward-word'
> and `backward-word'"?  Any call to either of them in Lisp code?
> And what is meant by "*indirectly*".

Lisp code has no right to expect that "words" mean anything in
particular. The scheme for detecting word boundaries is a user
customization point. find-word-boundary-function-table has been
supported for a while now, if not heavily used.

> Will you be telling users (e.g. in NEWS) precisely *how* they
> need to modify existing non-interactive calls to those functions?
> Saying that they should be used only interactively from now on
> does not tell users how to fix existing code that calls them.

Code can either use sexp movement or bind
find-word-boundary-function-table (which has been in Emacs for ages, by
the way) to a value they expect.

> And just what is unsafe about calling these two functions from
> Lisp?

Nothing: what's unsafe is expecting that users haven't customized word
boundaries.

> 
> Sorry, but this is not clear to me.  (And why not create new
> functions/commands, instead of changing the meaning/behavior
> of these longstanding ones?)

Because lots of packages invoke word movement commands on the user's
behalf, expecting that movement happens by words. By changing word
boundaries, we make subword mode work as expected everywhere instead of
making everyone that deals with word movement handle the possibility of
subword-mode separately.

The behavior of forward-word hasn't changed. We now make use of an Emacs
core feature that was not heavily used before. Code that relied on this
core feature going unused has always been broken. It's worth mentioning
in NEWS, sure, but I'm against just rebinding the movement commands.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Drew Adams]

> Lisp code has no right to expect that "words" mean anything in
> particular.

Dunno whether it has any "rights" at all. ;-)  Is it a bad idea that
some Lisp code treats sequences of word-constituent chars as a group
in some way?  Should no Lisp code do that?

> The scheme for detecting word boundaries is a user customization
> point. find-word-boundary-function-table has been supported for
> a while now, if not heavily used.

I know nothing about that `*-table' - mea culpa.  But what are \b
and \B for in Elisp regexps, if not for matching word boundaries?

> Code can either use sexp movement or bind
> find-word-boundary-function-table (which has been in Emacs for ages,
> by the way) to a value they expect.
> 
> what's unsafe is expecting that users haven't customized
> word boundaries.

Not sure what that means.  But if a user changes a char so that it
becomes or stops being a word-constituent char, that customization
is handled by \b, \B, \w, and \W.

If the magic table complicates the picture, does it also change
how \b, \B, \w, and \W act?  Are they sometimes broken, depending
on user customization via that table?

> lots of packages invoke word movement commands on the user's
> behalf, expecting that movement happens by words. By changing word
> boundaries, we make subword mode work as expected everywhere instead
> of making everyone that deals with word movement handle the possibility
> of subword-mode separately.

Is this perhaps all about `subword-mode'?  Just wondering.

> The behavior of forward-word hasn't changed. We now make use of an
> Emacs core feature that was not heavily used before. Code that relied
> on this core feature going unused has always been broken. It's worth
> mentioning in NEWS, sure, but I'm against just rebinding the
> movement commands.

Sounds complicated, but I won't try to deal with it here & now.

Seems to me that both users and code can decide on the syntax
classes for given characters, and code should be able to move
over or otherwise manipulate "words" defined as sequences
of word-constituent chars.  If that's what's being proscribed
now, then I might be disappointed.

But it is probably more complex than my naive understanding of
these things sees.  Perhaps things are complicated because of the
existence of subword mode?  I will anyway wait for NEWS, to see
what, if anything, I need to change in my code.

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Glenn Morris]

Daniel Colascione wrote:

> I am. It's the only way we can make sure that interactive commands that
> move by words *indirectly* do the right thing in the presence of user
> customizations.

Then I think the onus is on... someone to do the work that this entails:
Fix erc (this report is one year old and has a patch); make (belated)
NEWS entries; update the documentation relating to word motion commands;
mark said commands interactive-only; address all resulting compilation
warnings in the Emacs tree.

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Stefan Monnier]

> mark said commands interactive-only; address all resulting compilation
                                               ^^^
                                               most
> warnings in the Emacs tree.


        Stefan "let's not push it too far"

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 891 bytes --]

On 04/28/2015 09:50 AM, Glenn Morris wrote:
> Daniel Colascione wrote:
> 
>> I am. It's the only way we can make sure that interactive commands that
>> move by words *indirectly* do the right thing in the presence of user
>> customizations.
> 
> Then I think the onus is on... someone to do the work that this entails:
> Fix erc (this report is one year old and has a patch); 

So let's apply the patch to ERC.

> make (belated)
> NEWS entries; update the documentation relating to word motion commands;

I'll do that.

> mark said commands interactive-only; address all resulting compilation
> warnings in the Emacs tree.

The functions aren't interactive-only though. It's perfectly legitimate
for lisp code to want to move by words --- it's just that the user gets
to define what "word" means. ispell-word, for example, really ought to
use word motion commands.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Dima Kogan]

Daniel Colascione <dancol@dancol.org> writes:

> On 04/28/2015 09:50 AM, Glenn Morris wrote:
>
>> Then I think the onus is on... someone to do the work that this entails:
>> Fix erc (this report is one year old and has a patch); 
>
> So let's apply the patch to ERC.

Hi. Can somebody please apply this patch? ERC from HEAD is completely
broken without it.

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Lars Ingebrigtsen]

Dima Kogan <dima@secretsauce.net> writes:

> Hi. Can somebody please apply this patch? ERC from HEAD is completely
> broken without it.

I've got a couple of comments about the code...

>      (goto-char (point-min))
> -    (upcase-word 1)
> +
> +    ;; this is (upcase-word 1), but working even with subword-mode
> +    ;; active
> +    (skip-syntax-forward "^w")
> +    (let*
> +        ((word-start (point))
> +         (word-end
> +          (progn (skip-syntax-forward "w") (point))))
> +      (upcase-region word-start word-end))
> +

If you had a function `erc-forward-word' that did all the syntax
skipping, you could basically just say

(upcase-region (point) (erc-forward-word))

> -      (while (forward-word 1)
> -        (setq bounds (bounds-of-thing-at-point 'word))
> -        (setq word (buffer-substring-no-properties
> -                    (car bounds) (cdr bounds)))
> -        (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
> -                  (and erc-channel-users (erc-get-channel-user word)))
> -          (erc-button-add-button (car bounds) (cdr bounds)
> -                                 fun t (list word)))))))
> +
> +      (while
> +          (progn
> +
> +            ;; I move forward a word (independent of subword-mode) ...
> +            (skip-syntax-forward "^w")
> +            (let*
> +                ((word-start (point))
> +                 (word-end
> +                  (progn (skip-syntax-forward "w") (point))))
> +
> +              ;; ... if the word was empty we're at the end of buffer ...
> +              (and (/= word-start word-end)
> +
> +                   ;; ... otherwise, we do stuff with this word
> +                   (progn
> +                     (setq word (buffer-substring-no-properties
> +                                 word-start word-end))
> +                     (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
> +                               (and erc-channel-users (erc-get-channel-user word)))
> +                       (erc-button-add-button word-start word-end
> +                                              fun t (list word)))))))))))

Similarly here, you could use that erc-forward-word to avoid rewriting
this code so much.  It would be just

(while (erc-forward-word)
  (setq bound-stuff ...)
  )

Or something.  I think.

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

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Dima Kogan]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> If you had a function `erc-forward-word' that did all the syntax
> skipping, you could basically just say
>
> (upcase-region (point) (erc-forward-word))

Sure, that's fine. Are you suggesting that such a function be written,
and then this patch will be accepted? This isn't erc-specific, so
something named "forward-word-nosubword" probably would be better

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Lars Ingebrigtsen]

Dima Kogan <dima@secretsauce.net> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> If you had a function `erc-forward-word' that did all the syntax
>> skipping, you could basically just say
>>
>> (upcase-region (point) (erc-forward-word))
>
> Sure, that's fine. Are you suggesting that such a function be written,
> and then this patch will be accepted?

Yup.

> This isn't erc-specific, so something named "forward-word-nosubword"
> probably would be better

I think that might be somewhat controversial.  As a start, let's keep
this contained to erc, and we can move stuff around afterwards, if
needed.

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

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Dima Kogan]

[-- Attachment #1: Type: text/plain, Size: 2939 bytes --]

Hi. New patch attached. Notes inline


Lars Ingebrigtsen <larsi@gnus.org> writes:

> I've got a couple of comments about the code...
>
>>      (goto-char (point-min))
>> -    (upcase-word 1)
>> +
>> +    ;; this is (upcase-word 1), but working even with subword-mode
>> +    ;; active
>> +    (skip-syntax-forward "^w")
>> +    (let*
>> +        ((word-start (point))
>> +         (word-end
>> +          (progn (skip-syntax-forward "w") (point))))
>> +      (upcase-region word-start word-end))
>> +
>
> If you had a function `erc-forward-word' that did all the syntax
> skipping, you could basically just say
>
> (upcase-region (point) (erc-forward-word))

Sure. New patch does this.


>> -      (while (forward-word 1)
>> -        (setq bounds (bounds-of-thing-at-point 'word))
>> -        (setq word (buffer-substring-no-properties
>> -                    (car bounds) (cdr bounds)))
>> -        (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
>> -                  (and erc-channel-users (erc-get-channel-user word)))
>> -          (erc-button-add-button (car bounds) (cdr bounds)
>> -                                 fun t (list word)))))))
>> +
>> +      (while
>> +          (progn
>> +
>> +            ;; I move forward a word (independent of subword-mode) ...
>> +            (skip-syntax-forward "^w")
>> +            (let*
>> +                ((word-start (point))
>> +                 (word-end
>> +                  (progn (skip-syntax-forward "w") (point))))
>> +
>> +              ;; ... if the word was empty we're at the end of buffer ...
>> +              (and (/= word-start word-end)
>> +
>> +                   ;; ... otherwise, we do stuff with this word
>> +                   (progn
>> +                     (setq word (buffer-substring-no-properties
>> +                                 word-start word-end))
>> +                     (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
>> +                               (and erc-channel-users (erc-get-channel-user word)))
>> +                       (erc-button-add-button word-start word-end
>> +                                              fun t (list word)))))))))))
>
> Similarly here, you could use that erc-forward-word to avoid rewriting
> this code so much.  It would be just
>
> (while (erc-forward-word)
>   (setq bound-stuff ...)
>   )

That would be nice, but no. This chunk of code was affected by
subword-mode in two ways:

1. (forward-word 1)
2. (bounds-of-thing-at-point 'word)

The proposed change only takes care of #1, and I don't see an obvious
way to make it take care of both. I can do (while (erc-forward-word)
...), but then to get the word, I'd need to backtrack to the previous
word marker, and it's not obvious to me that this would be an
improvement over the existing change in the patch.

The attached patch thus has no changes to this hunk. Let me know if you
think of a nicer way to do this.

dima



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ERC-no-longer-gets-confused-by-subword-mode.patch --]
[-- Type: text/x-diff, Size: 3551 bytes --]

From 28d49bfe6581a108b68c9b85dd57c77ecc94125c Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Tue, 30 Dec 2014 23:29:21 -0800
Subject: [PATCH] ERC no longer gets confused by subword-mode

In commit 6ddc44225e743e2b2a0d5c192f50aefd7a4a915b subword-mode was
integrated into the syntax table instead of simply remapping the
interactive motion bindings as was done previously.  This had the
unintended effect of changing the behavior of lisp programs that touch
words.  In the case of ERC, it completely broke it: emacs now throws an
error when ERC is launched, making it unusable when subword-mode is
active.

This commit replaces the word-oriented calls with ones that navigate
the buffer using syntax classes

Closes: #17558
---
 lisp/erc/erc-backend.el |  8 +++++++-
 lisp/erc/erc-button.el  | 30 ++++++++++++++++++++++--------
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index ec45dcf..c315c47 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -474,13 +474,19 @@ Currently this is called by `erc-send-input'."
                      nil t))
       (split-string (buffer-string) "\n"))))
 
+(defun erc-forward-word ()
+  "Moves forward one word, ignoring any subword settings.  If no
+subword-mode is active, then this is (forward-word)."
+  (skip-syntax-forward "^w")
+  (> (skip-syntax-forward "w") 0))
+
 ;; Used by CTCP functions
 (defun erc-upcase-first-word (str)
   "Upcase the first word in STR."
   (with-temp-buffer
     (insert str)
     (goto-char (point-min))
-    (upcase-word 1)
+    (upcase-region (point) (erc-forward-word))
     (buffer-string)))
 
 (defun erc-server-setup-periodical-ping (buffer)
diff --git a/lisp/erc/erc-button.el b/lisp/erc/erc-button.el
index 0e4c709..653488c 100644
--- a/lisp/erc/erc-button.el
+++ b/lisp/erc/erc-button.el
@@ -300,14 +300,28 @@ specified by `erc-button-alist'."
     (when (or (eq t form)
               (eval form))
       (goto-char (point-min))
-      (while (forward-word 1)
-        (setq bounds (bounds-of-thing-at-point 'word))
-        (setq word (buffer-substring-no-properties
-                    (car bounds) (cdr bounds)))
-        (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
-                  (and erc-channel-users (erc-get-channel-user word)))
-          (erc-button-add-button (car bounds) (cdr bounds)
-                                 fun t (list word)))))))
+
+      (while
+          (progn
+
+            ;; I move forward a word (independent of subword-mode) ...
+            (skip-syntax-forward "^w")
+            (let*
+                ((word-start (point))
+                 (word-end
+                  (progn (skip-syntax-forward "w") (point))))
+
+              ;; ... if the word was empty we're at the end of buffer ...
+              (and (/= word-start word-end)
+
+                   ;; ... otherwise, we do stuff with this word
+                   (progn
+                     (setq word (buffer-substring-no-properties
+                                 word-start word-end))
+                     (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
+                               (and erc-channel-users (erc-get-channel-user word)))
+                       (erc-button-add-button word-start word-end
+                                              fun t (list word)))))))))))
 
 (defun erc-button-add-buttons-1 (regexp entry)
   "Search through the buffer for matches to ENTRY and add buttons."
-- 
2.1.4

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Lars Magne Ingebrigtsen]

Dima Kogan <dima@secretsauce.net> writes:

> That would be nice, but no. This chunk of code was affected by
> subword-mode in two ways:
>
> 1. (forward-word 1)
> 2. (bounds-of-thing-at-point 'word)
>
> The proposed change only takes care of #1, and I don't see an obvious
> way to make it take care of both. I can do (while (erc-forward-word)
> ...), but then to get the word, I'd need to backtrack to the previous
> word marker, and it's not obvious to me that this would be an
> improvement over the existing change in the patch.
>
> The attached patch thus has no changes to this hunk. Let me know if you
> think of a nicer way to do this.

[...]

> -      (while (forward-word 1)
> -        (setq bounds (bounds-of-thing-at-point 'word))
> -        (setq word (buffer-substring-no-properties
> -                    (car bounds) (cdr bounds)))
> -        (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
> -                  (and erc-channel-users (erc-get-channel-user word)))
> -          (erc-button-add-button (car bounds) (cdr bounds)
> -                                 fun t (list word)))))))
> +
> +      (while
> +          (progn
> +
> +            ;; I move forward a word (independent of subword-mode) ...
> +            (skip-syntax-forward "^w")
> +            (let*
> +                ((word-start (point))
> +                 (word-end
> +                  (progn (skip-syntax-forward "w") (point))))
> +

This still seems very difficult to understand.  The old version has a
clear loop condition

> -      (while (forward-word 1)

while in the new version everything is inside a progn, making it kinda
difficult to follow.  Would it be possible to write a version of
(bounds-of-thing-at-point 'word) that's subword-mode agostic
(erc-word-at-point) to preserve the logic?

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

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Dima Kogan]

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

>> The attached patch thus has no changes to this hunk. Let me know if you
>> think of a nicer way to do this.
>
> This still seems very difficult to understand.  The old version has a
> clear loop condition
>
>> -      (while (forward-word 1)
>
> while in the new version everything is inside a progn, making it kinda
> difficult to follow.

Aaah, I see the complaint now. New patch attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ERC-no-longer-gets-confused-by-subword-mode.patch --]
[-- Type: text/x-diff, Size: 3805 bytes --]

From ba2f53e864cc36a3a9296104975407274b59b601 Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Tue, 30 Dec 2014 23:29:21 -0800
Subject: [PATCH] ERC no longer gets confused by subword-mode

In commit 6ddc44225e743e2b2a0d5c192f50aefd7a4a915b subword-mode was
integrated into the syntax table instead of simply remapping the
interactive motion bindings as was done previously.  This had the
unintended effect of changing the behavior of lisp programs that touch
words.  In the case of ERC, it completely broke it: emacs now throws an
error when ERC is launched, making it unusable when subword-mode is
active.

This commit replaces the word-oriented calls with ones that navigate
the buffer using syntax classes

Closes: #17558
---
 lisp/erc/erc-backend.el | 28 +++++++++++++++++++++++++++-
 lisp/erc/erc-button.el  | 16 ++++++++--------
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index ec45dcf..4925c96 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -474,13 +474,39 @@ Currently this is called by `erc-send-input'."
                      nil t))
       (split-string (buffer-string) "\n"))))
 
+(defun erc-forward-word ()
+  "Moves forward one word, ignoring any subword settings.  If no
+subword-mode is active, then this is (forward-word)."
+  (skip-syntax-forward "^w")
+  (> (skip-syntax-forward "w") 0))
+
+(defun erc-word-at-arg-p (pos)
+  "Reports whether the char after a given POS has word syntax.
+If POS is out of range, the value is nil."
+  (let ((c (char-after pos)))
+    (if c
+        (eq ?w (char-syntax c))
+      nil)))
+
+(defun erc-bounds-of-word-at-point ()
+  "Returns the bounds of a word at point, or nil if we're not at
+a word.  If no subword-mode is active, then this
+is (bounds-of-thing-at-point 'word)."
+  (if (or (erc-word-at-arg-p (point))
+          (erc-word-at-arg-p (1- (point))))
+      (save-excursion
+        (let* ((start (progn (skip-syntax-backward "w") (point)))
+               (end   (progn (skip-syntax-forward  "w") (point))))
+          (cons start end)))
+    nil))
+
 ;; Used by CTCP functions
 (defun erc-upcase-first-word (str)
   "Upcase the first word in STR."
   (with-temp-buffer
     (insert str)
     (goto-char (point-min))
-    (upcase-word 1)
+    (upcase-region (point) (progn (erc-forward-word) (point)))
     (buffer-string)))
 
 (defun erc-server-setup-periodical-ping (buffer)
diff --git a/lisp/erc/erc-button.el b/lisp/erc/erc-button.el
index 0e4c709..a59ad22 100644
--- a/lisp/erc/erc-button.el
+++ b/lisp/erc/erc-button.el
@@ -300,14 +300,14 @@ specified by `erc-button-alist'."
     (when (or (eq t form)
               (eval form))
       (goto-char (point-min))
-      (while (forward-word 1)
-        (setq bounds (bounds-of-thing-at-point 'word))
-        (setq word (buffer-substring-no-properties
-                    (car bounds) (cdr bounds)))
-        (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
-                  (and erc-channel-users (erc-get-channel-user word)))
-          (erc-button-add-button (car bounds) (cdr bounds)
-                                 fun t (list word)))))))
+      (while (erc-forward-word)
+        (when (setq bounds (erc-bounds-of-word-at-point))
+          (setq word (buffer-substring-no-properties
+                      (car bounds) (cdr bounds)))
+          (when (or (and (erc-server-buffer-p) (erc-get-server-user word))
+                    (and erc-channel-users (erc-get-channel-user word)))
+            (erc-button-add-button (car bounds) (cdr bounds)
+                                   fun t (list word))))))))
 
 (defun erc-button-add-buttons-1 (regexp entry)
   "Search through the buffer for matches to ENTRY and add buttons."
-- 
2.1.4

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Dima Kogan]

Dima Kogan <dima@secretsauce.net> writes:

> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:
>
>>> The attached patch thus has no changes to this hunk. Let me know if you
>>> think of a nicer way to do this.
>>
>> This still seems very difficult to understand.  The old version has a
>> clear loop condition
>>
>>> -      (while (forward-word 1)
>>
>> while in the new version everything is inside a progn, making it kinda
>> difficult to follow.
>
> Aaah, I see the complaint now. New patch attached.

Hi. This is a gentle ping. Did you get a chance to look at the new
patch, by chance? Thanks!

bug#17558: 24.4.50; global-subword-mode breaks ERC

[Lars Ingebrigtsen]

Dima Kogan <dima@secretsauce.net> writes:

> Aaah, I see the complaint now. New patch attached.

Looks good; applying.

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