GNU bug report logs - #17183
can't insert a quote pair before another

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17183 in the body.
You can then email your comments to 17183 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: bug-gnu-emacs <at> gnu.org
Subject: can't insert a quote pair before another
Date: Fri, 04 Apr 2014 11:03:33 -0400

Package: Emacs
Version: 24.3.50


emacs -Q -f electric-pair-mode -f text-mode
SPC SPC SPC "
C-a "

The first " inserts a pair of double quotes, like I expect, but the
second refuses to insert anything and instead it jumps forward to after
the first double quote.  Maybe there are cases where that makes sense,
but that's much too clever for me.

The situation where I bumped into it was that I had the text

    I suggest you write "blabla" instead of

and I wanted to turn it into

    I suggest you write "blibli" and "blabla" instead of

so I put point after "write" and wanted to type

    SPC "blibli" SPC and

but instead I had to use C-q " twice, which was rather unpleasant.


        Stefan




In GNU Emacs 24.3.50.1 (i686-pc-linux-gnu, GTK+ Version 2.24.22)
 of 2014-01-07 on faina

Message #8 received at 17183 <at> debbugs.gnu.org (full text, mbox):

From: joaotavora <at> gmail.com (João Távora)
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 17183 <at> debbugs.gnu.org
Subject: Re: bug#17183: can't insert a quote pair before another
Date: Sun, 06 Apr 2014 02:26:15 +0100

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

> Package: Emacs
> Version: 24.3.50

> but instead I had to use C-q " twice, which was rather unpleasant.

Yes seen it. Rather unpleasant indeed. How does this look? 

=== modified file 'lisp/ChangeLog'
*** lisp/ChangeLog	2014-04-04 23:31:02 +0000
--- lisp/ChangeLog	2014-04-06 01:23:51 +0000
***************
*** 1,3 ****
--- 1,11 ----
+ 2014-04-06  João Távora  <joaotavora <at> gmail.com>
+ 
+ 	* elec-pair.el (electric-pair--skip-whitespace): With quote
+ 	syntax, ensure not outside string before insertion started. .
+ 	(electric-pair-post-self-insert-function): Pass char and syntax to
+ 	electric-pair--skip-whitespace. Save point instead of
+ 	`save-excursion'. (Bug#17183)
+ 
  2014-04-04  João Távora  <joaotavora <at> gmail.com>
  
  	* elec-pair.el:

=== modified file 'lisp/elec-pair.el'
*** lisp/elec-pair.el	2014-04-04 23:31:02 +0000
--- lisp/elec-pair.el	2014-04-06 01:19:47 +0000
***************
*** 151,163 ****
                        (const :tag "Newline" ?\n))
                   (list character)))
  
! (defun electric-pair--skip-whitespace ()
    "Skip whitespace forward, not crossing comment or string boundaries."
!   (let ((saved (point))
!         (string-or-comment (nth 8 (syntax-ppss))))
!     (skip-chars-forward (apply #'string electric-pair-skip-whitespace-chars))
!     (unless (eq string-or-comment (nth 8 (syntax-ppss)))
!       (goto-char saved))))
  
  (defvar electric-pair-text-syntax-table prog-mode-syntax-table
    "Syntax table used when pairing inside comments and strings.
--- 151,171 ----
                        (const :tag "Newline" ?\n))
                   (list character)))
  
! (defun electric-pair--skip-whitespace (char syntax)
    "Skip whitespace forward, not crossing comment or string boundaries."
!   (let* ((saved (point))
!          (ppss (syntax-ppss))
!          (string-or-comment (nth 8 ppss)))
!     (unless (and
!              (eq syntax ?\")
!              (unwind-protect
!                  (progn
!                    (delete-char -1)
!                    (not (nth 3 (syntax-ppss))))
!                (insert-char char)))
!       (skip-chars-forward (apply #'string electric-pair-skip-whitespace-chars))
!       (unless (eq string-or-comment (nth 8 (syntax-ppss)))
!         (goto-char saved)))))
  
  (defvar electric-pair-text-syntax-table prog-mode-syntax-table
    "Syntax table used when pairing inside comments and strings.
***************
*** 502,521 ****
                         (if (functionp electric-pair-skip-self)
                             (funcall electric-pair-skip-self last-command-event)
                           electric-pair-skip-self))
!                    (save-excursion
                       (when (setq skip-whitespace-info
                                   (if (functionp electric-pair-skip-whitespace)
                                       (funcall electric-pair-skip-whitespace)
                                     electric-pair-skip-whitespace))
!                        (electric-pair--skip-whitespace))
!                      (eq (char-after) last-command-event))))
           ;; This is too late: rather than insert&delete we'd want to only
           ;; skip (or insert in overwrite mode).  The difference is in what
           ;; goes in the undo-log and in the intermediate state which might
           ;; be visible to other post-self-insert-hook.  We'll just have to
           ;; live with it for now.
           (when skip-whitespace-info
!            (electric-pair--skip-whitespace))
           (delete-region (1- pos) (if (eq skip-whitespace-info 'chomp)
                                       (point)
                                     pos))
--- 510,531 ----
                         (if (functionp electric-pair-skip-self)
                             (funcall electric-pair-skip-self last-command-event)
                           electric-pair-skip-self))
!                    (let ((saved (point)))
                       (when (setq skip-whitespace-info
                                   (if (functionp electric-pair-skip-whitespace)
                                       (funcall electric-pair-skip-whitespace)
                                     electric-pair-skip-whitespace))
!                        (electric-pair--skip-whitespace last-command-event syntax))
!                      (prog1
!                          (eq (char-after) last-command-event)
!                        (goto-char saved)))))
           ;; This is too late: rather than insert&delete we'd want to only
           ;; skip (or insert in overwrite mode).  The difference is in what
           ;; goes in the undo-log and in the intermediate state which might
           ;; be visible to other post-self-insert-hook.  We'll just have to
           ;; live with it for now.
           (when skip-whitespace-info
!            (electric-pair--skip-whitespace last-command-event syntax))
           (delete-region (1- pos) (if (eq skip-whitespace-info 'chomp)
                                       (point)
                                     pos))

Message #11 received at 17183 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: joaotavora <at> gmail.com (João Távora)
Cc: 17183 <at> debbugs.gnu.org
Subject: Re: bug#17183: can't insert a quote pair before another
Date: Sun, 06 Apr 2014 08:39:26 -0400

> +             (unwind-protect
> +                 (progn
> +                   (delete-char -1)
> +                   (not (nth 3 (syntax-ppss))))
> +               (insert-char char)))

Yuck!  Why not (save-excursion (not (nth 3 (syntax-ppss (-1 (point))))))?


        Stefan

Message #14 received at 17183 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 17183 <at> debbugs.gnu.org
Subject: Re: bug#17183: can't insert a quote pair before another
Date: Sun, 6 Apr 2014 16:01:44 +0100

That's precisely what I proposed in my last message
that didn't yet make it to you (gmane authorization).

Also, we should probably use electric-pair--syntax-ppss
so that the trick works inside comments, too.

On Sun, Apr 6, 2014 at 1:39 PM, Stefan Monnier <monnier <at> iro.umontreal.ca> wrote:
>> +             (unwind-protect
>> +                 (progn
>> +                   (delete-char -1)
>> +                   (not (nth 3 (syntax-ppss))))
>> +               (insert-char char)))
>
> Yuck!  Why not (save-excursion (not (nth 3 (syntax-ppss (-1 (point))))))?
>
>
>         Stefan



-- 
João Távora

Message #17 received at submit <at> debbugs.gnu.org (full text, mbox):

From: joaotavora <at> gmail.com (João Távora)
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#17183: can't insert a quote pair before another
Date: Sun, 06 Apr 2014 13:46:59 +0100

joaotavora <at> gmail.com (João Távora) writes:

> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>
>> Package: Emacs
>> Version: 24.3.50
>
>> but instead I had to use C-q " twice, which was rather unpleasant.
>
> Yes seen it. Rather unpleasant indeed. How does this look?

Actually, this is slightly simpler. New tests should make my intention
clear as well

=== modified file 'lisp/ChangeLog'
*** lisp/ChangeLog	2014-04-05 18:33:55 +0000
--- lisp/ChangeLog	2014-04-06 12:43:30 +0000
***************
*** 1,3 ****
--- 1,10 ----
+ 2014-04-06  João Távora  <joaotavora <at> gmail.com>
+ 
+ 	* elec-pair.el (electric-pair--skip-whitespace): With quote
+ 	syntax, ensure not outside string before insertion started. .
+ 	(electric-pair-post-self-insert-function): Pass syntax to
+ 	electric-pair--skip-whitespace. (Bug#17183)
+ 
  2014-04-05  Glenn Morris  <rgm <at> gnu.org>
  
  	* help.el (view-lossage): Doc tweak.

=== modified file 'lisp/elec-pair.el'
*** lisp/elec-pair.el	2014-02-23 00:19:11 +0000
--- lisp/elec-pair.el	2014-04-06 12:33:49 +0000
***************
*** 151,163 ****
                        (const :tag "Newline" ?\n))
                   (list character)))
  
! (defun electric-pair--skip-whitespace ()
    "Skip whitespace forward, not crossing comment or string boundaries."
!   (let ((saved (point))
!         (string-or-comment (nth 8 (syntax-ppss))))
!     (skip-chars-forward (apply #'string electric-pair-skip-whitespace-chars))
!     (unless (eq string-or-comment (nth 8 (syntax-ppss)))
!       (goto-char saved))))
  
  (defvar electric-pair-text-syntax-table prog-mode-syntax-table
    "Syntax table used when pairing inside comments and strings.
--- 151,168 ----
                        (const :tag "Newline" ?\n))
                   (list character)))
  
! (defun electric-pair--skip-whitespace (syntax)
    "Skip whitespace forward, not crossing comment or string boundaries."
!   (let* ((saved (point))
!          (ppss (syntax-ppss))
!          (string-or-comment (nth 8 ppss)))
!     (unless (and
!              (eq syntax ?\")
!              (not (nth 3 (save-excursion
!                            (syntax-ppss (1- (point)))))))
!       (skip-chars-forward (apply #'string electric-pair-skip-whitespace-chars))
!       (unless (eq string-or-comment (nth 8 (syntax-ppss)))
!         (goto-char saved)))))
  
  (defvar electric-pair-text-syntax-table prog-mode-syntax-table
    "Syntax table used when pairing inside comments and strings.
***************
*** 489,495 ****
                                   (if (functionp electric-pair-skip-whitespace)
                                       (funcall electric-pair-skip-whitespace)
                                     electric-pair-skip-whitespace))
!                        (electric-pair--skip-whitespace))
                       (eq (char-after) last-command-event))))
           ;; This is too late: rather than insert&delete we'd want to only
           ;; skip (or insert in overwrite mode).  The difference is in what
--- 494,500 ----
                                   (if (functionp electric-pair-skip-whitespace)
                                       (funcall electric-pair-skip-whitespace)
                                     electric-pair-skip-whitespace))
!                        (electric-pair--skip-whitespace syntax))
                       (eq (char-after) last-command-event))))
           ;; This is too late: rather than insert&delete we'd want to only
           ;; skip (or insert in overwrite mode).  The difference is in what
***************
*** 497,503 ****
           ;; be visible to other post-self-insert-hook.  We'll just have to
           ;; live with it for now.
           (when skip-whitespace-info
!            (electric-pair--skip-whitespace))
           (delete-region (1- pos) (if (eq skip-whitespace-info 'chomp)
                                       (point)
                                     pos))
--- 502,508 ----
           ;; be visible to other post-self-insert-hook.  We'll just have to
           ;; live with it for now.
           (when skip-whitespace-info
!            (electric-pair--skip-whitespace syntax))
           (delete-region (1- pos) (if (eq skip-whitespace-info 'chomp)
                                       (point)
                                     pos))

=== modified file 'test/ChangeLog'
*** test/ChangeLog	2014-03-25 07:34:30 +0000
--- test/ChangeLog	2014-04-06 12:44:19 +0000
***************
*** 1,3 ****
--- 1,11 ----
+ 2014-04-06  João Távora  <joaotavora <at> gmail.com>
+ 
+ 	* automated/electric-tests.el (electric-pair-define-test-form):
+ 	More readable test docstrings.
+ 	(whitespace-skipping-for-quotes-not-ouside)
+ 	(whitespace-skipping-for-quotes-only-inside)
+ 	(whitespace-skipping-for-quotes-in-text-mode): New tests.
+ 
  2014-03-24  Barry O'Reilly  <gundaetiapo <at> gmail.com>
  
  	* automated/undo-tests.el (undo-test-marker-adjustment-nominal):

=== modified file 'test/automated/electric-tests.el'
*** test/automated/electric-tests.el	2014-01-01 07:43:34 +0000
--- test/automated/electric-tests.el	2014-04-06 12:41:35 +0000
***************
*** 114,121 ****
                                       mode
                                       extra-desc))
             ()
!          ,(format "With \"%s\", try input %c at point %d. \
! Should %s \"%s\" and point at %d"
                    fixture
                    char
                    (1+ pos)
--- 114,121 ----
                                       mode
                                       extra-desc))
             ()
!          ,(format "With |%s|, try input %c at point %d. \
! Should %s |%s| and point at %d"
                    fixture
                    char
                    (1+ pos)
***************
*** 341,346 ****
--- 341,371 ----
    :test-in-code nil
    :test-in-comments t)
  
+ (define-electric-pair-test whitespace-skipping-for-quotes-not-ouside
+   "  \"  \"" "\"-----" :expected-string "\"\"  \"  \""
+   :expected-point 2
+   :bindings '((electric-pair-skip-whitespace . chomp))
+   :test-in-strings nil
+   :test-in-code t
+   :test-in-comments nil)
+ 
+ (define-electric-pair-test whitespace-skipping-for-quotes-only-inside
+   "  \"  \"" "---\"--" :expected-string "  \"\""
+   :expected-point 5
+   :bindings '((electric-pair-skip-whitespace . chomp))
+   :test-in-strings nil
+   :test-in-code t
+   :test-in-comments nil)
+ 
+ (define-electric-pair-test whitespace-skipping-for-quotes-in-text-mode
+   "  \"  \"" "---\"--" :expected-string "  \"\"\"  \""
+   :expected-point 5
+   :modes '(text-mode)
+   :bindings '((electric-pair-skip-whitespace . chomp))
+   :test-in-strings nil
+   :test-in-code t
+   :test-in-comments nil)
+ 
  
  ;;; Pairing arbitrary characters
  ;;;

Message #20 received at 17183 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: joaotavora <at> gmail.com (João Távora)
Cc: 17183 <at> debbugs.gnu.org
Subject: Re: bug#17183: can't insert a quote pair before another
Date: Sun, 06 Apr 2014 15:39:09 -0400

> Actually, this is slightly simpler. New tests should make my intention
> clear as well

Looks good, thanks,


        Stefan

Message #25 received at 17183-done <at> debbugs.gnu.org (full text, mbox):

From: joaotavora <at> gmail.com (João Távora)
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 16981-done <at> debbugs.gnu.org, 17192-done <at> debbugs.gnu.org,
 17183-done <at> debbugs.gnu.org, Alan Mackenzie <acm <at> muc.de>
Subject: Re: bug#16981: 24.3.50;
 electric-pair-delete-adjacent-pairs broken in c-mode,
 python-mode,	maybe-others
Date: Mon, 07 Apr 2014 01:04:35 +0100

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

> Just take the corresponding diffs (IIRC you can use
> "bzr merge -r116925..r116926 .../trunk" for that) and commit them into
> emacs-24.
>
> Bzr doesn't really know about cherry picking, but if you put
> "backported" into your commit message, our bzrmerge.el script
> will know that this commit is already in the trunk and will know how to
> avoid the corresponding conflicts when merging emacs-24 back into trunk.
>
>> - Three different fixes (bugs 16981,17192 and 17183) not yet pushed that
>> I think you want me to only push to emacs-24, since that will be merged
>> back to trunk later.
>
> Right.

These three I did successfully, but failed miserably in the
backporting. Oh git where art thou... Now I'm downloading a new repo
(with sharing properly setup hopefully), since I must have done
something that messed up my bzr conf, apparently I shouldn't have moved
my repo dir.

Should be done tomorrow morning, so I'll backport r116926 and r116940
then.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.