bug#19505: 25.0.50; Defining a chomp function breaks electric pairing

bug#19505: 25.0.50; Defining a chomp function breaks electric pairing

[Tom Willemse]

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


The attached patch fixes the situation where a function `chomp' exists
and `electric-pair-skip-whitespace' has the value `chomp'. Since that
specific symbol is mentioned as a special case for this variable, it
seems that it shouldn't try to call a function named as such.

Basically, defining a function named `chomp' will break electric pairing
if the `electric-pair-skip-whitespace' is set to chomp.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Always-treat-chomp-as-a-symbol.patch --]
[-- Type: text/x-diff, Size: 1548 bytes --]

From d7e6e8f2ff3572afb4131ea64966b2a9253f9ac6 Mon Sep 17 00:00:00 2001
From: Tom Willemse <tom@ryuslash.org>
Date: Sun, 4 Jan 2015 05:37:02 +0100
Subject: [PATCH] Always treat chomp as a symbol

---
 lisp/ChangeLog    | 5 +++++
 lisp/elec-pair.el | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index aa79a8d..b031f25 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,8 @@
+2015-01-04  Tom Willemse  <tom@ryuslash.org>
+
+	* elec-pair.el (electric-pair-post-self-insert-function): Make
+	sure chomp isn't used as a function.
+
 2015-01-04  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Less 'make' chatter for lisp dir
diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el
index 2cede62..65c3dbf 100644
--- a/lisp/elec-pair.el
+++ b/lisp/elec-pair.el
@@ -475,7 +475,8 @@ (defun electric-pair-post-self-insert-function
                      (when (and (not (and unconditional
                                           (eq syntax ?\")))
                                 (setq skip-whitespace-info
-                                      (if (functionp electric-pair-skip-whitespace)
+                                      (if (and (not (eq electric-pair-skip-whitespace 'chomp))
+                                               (functionp electric-pair-skip-whitespace))
                                           (funcall electric-pair-skip-whitespace)
                                         electric-pair-skip-whitespace)))
                        (electric-pair--skip-whitespace))
-- 
2.2.1

bug#19505: 25.0.50; Defining a chomp function breaks electric pairing

[Stefan Monnier]

> The attached patch fixes the situation where a function `chomp' exists
> and `electric-pair-skip-whitespace' has the value `chomp'.  Since that
> specific symbol is mentioned as a special case for this variable, it
> seems that it shouldn't try to call a function named as such.

Maybe a better option is to use the value `:chomp' instead of `chomp',
since defining a function `:chomp' should be considered an error anyway,


        Stefan

bug#19505: 25.0.50; Defining a chomp function breaks electric pairing

[Tom Willemse]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> The attached patch fixes the situation where a function `chomp' exists
>> and `electric-pair-skip-whitespace' has the value `chomp'.  Since that
>> specific symbol is mentioned as a special case for this variable, it
>> seems that it shouldn't try to call a function named as such.
>
> Maybe a better option is to use the value `:chomp' instead of `chomp',
> since defining a function `:chomp' should be considered an error anyway,

Certainly an option, but wouldn't that break people's configuration in
an annoying way?

bug#19505: 25.0.50; Defining a chomp function breaks electric pairing

[Tom Willemse]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> The attached patch fixes the situation where a function `chomp' exists
>> and `electric-pair-skip-whitespace' has the value `chomp'.  Since that
>> specific symbol is mentioned as a special case for this variable, it
>> seems that it shouldn't try to call a function named as such.
>
> Maybe a better option is to use the value `:chomp' instead of `chomp',
> since defining a function `:chomp' should be considered an error anyway,

I've attached a patch with the change as you suggested. It seems to me
this change might need an entry in the NEWS file, since it changes the
value of `electric-pair-skip-whitespace' from a symbol to a keyword,
people might be confused that their configuration suddenly stopped
working. I just had no idea how to describe this in a NEWS-friendly way.

Perhaps a test should be written for this as well? If someone could
suggest where to put such a test I would happily (try to) write it.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-a-keyword-not-symbol-to-delete-whitespace.patch --]
[-- Type: text/x-diff, Size: 2392 bytes --]

From c9dfaff53b11f6db401d8ed7a7d25e1efb45c461 Mon Sep 17 00:00:00 2001
From: Tom Willemse <tom@ryuslash.org>
Date: Tue, 6 Jan 2015 20:37:29 +0100
Subject: [PATCH] Use a keyword, not symbol, to delete whitespace

* elec-pair.el (electric-pair-skip-whitespace): Use `:chomp' instead
of `chomp'.
(electric-pair-post-self-insert-function): Check for a keyword
`:chomp' instead of the symbol `chomp' to prevent naming conflicts.
---
 lisp/ChangeLog    | 8 ++++++++
 lisp/elec-pair.el | 6 +++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 76ba2cd..cd7cd2a 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,11 @@
+2015-01-06  Tom Willemse  <tom@ryuslash.org>
+
+	* elec-pair.el (electric-pair-skip-whitespace): Use `:chomp'
+	instead of `chomp'.
+	(electric-pair-post-self-insert-function): Check for a keyword
+	`:chomp' instead of the symbol `chomp' to prevent naming
+	conflicts.
+
 2015-01-05  Dmitry Gutov  <dgutov@yandex.ru>
 
 	* progmodes/xref.el (xref--insert-xrefs): Add `help-echo' property
diff --git a/lisp/elec-pair.el b/lisp/elec-pair.el
index 2cede62..270f4a3 100644
--- a/lisp/elec-pair.el
+++ b/lisp/elec-pair.el
@@ -129,7 +129,7 @@ (defcustom electric-pair-skip-whitespace
 The specific kind of whitespace skipped is given by the variable
 `electric-pair-skip-whitespace-chars'.
 
-The symbol `chomp' specifies that the skipped-over whitespace
+The keyword `:chomp' specifies that the skipped-over whitespace
 should be deleted.
 
 Can also be a function of no arguments, in which case that function's
@@ -138,7 +138,7 @@ (defcustom electric-pair-skip-whitespace
   :group 'electricity
   :type '(choice
           (const :tag "Yes, jump over whitespace" t)
-          (const :tag "Yes, and delete whitespace" chomp)
+          (const :tag "Yes, and delete whitespace" :chomp)
           (const :tag "No, no whitespace skipping" nil)
           function))
 
@@ -487,7 +487,7 @@ (defun electric-pair-post-self-insert-function
          ;; live with it for now.
          (when skip-whitespace-info
            (electric-pair--skip-whitespace))
-         (delete-region (1- pos) (if (eq skip-whitespace-info 'chomp)
+         (delete-region (1- pos) (if (eq skip-whitespace-info :chomp)
                                      (point)
                                    pos))
          (forward-char))
-- 
2.2.1

bug#19505: 25.0.50; Defining a chomp function breaks electric pairing

[Glenn Morris]

Version: 25.1

Tom Willemse wrote:

> The attached patch fixes the situation where a function `chomp' exists
> and `electric-pair-skip-whitespace' has the value `chomp'. Since that
> specific symbol is mentioned as a special case for this variable, it
> seems that it shouldn't try to call a function named as such.

Thanks; applied.
(While it might have been nicer to use :chomp, it doesn't seem worth an
incompatible change to me.)

BTW, you have a few "tiny changes" installed in Emacs now.
If you expect to contribute more, please consider completing the form at 

http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future