From: Simon Law <sfllaw@sfllaw.ca>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: Chong Yidong <cyd@gnu.org>, 11520@debbugs.gnu.org
Subject: bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode
Date: Sun, 21 Oct 2012 19:12:55 -0400 [thread overview]
Message-ID: <CABMJL3L7SXmixpr327pf9kKduPny39xjgXmykRYugTjgQdDDzA@mail.gmail.com> (raw)
In-Reply-To: <jwv1uh8ppzb.fsf-monnier+emacs@gnu.org>
[-- Attachment #1: Type: text/plain, Size: 1960 bytes --]
Attached is a patch against trunk.
I have addressed your concerns below:
On Mon, Oct 8, 2012 at 6:25 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> I proposed making delete-selection-pre-hook understand a function as
>> one of the legitimate types. If it were a function, it gets called and
>> its results would be interpreted as type, either: 'yank, 'kill,
>> 'supersede, t, or nil.
>
>> That way, electric-pair-mode can override the (put
>> 'self-insert-command 'delete-selection ...) with its own function that
>
> That's going in the right direction, but I have two problems with that:
> - I don't want electric-pair-mode to decide of the whole
> self-insert-command behavior. I.e. self-insert-command should have
> a `delete-selection' property that is not specific to
> electric-pair-mode, so that if someone implements some other
> post-self-insert-hook that also interacts with
> selection-selection-mode, they should be able to cooperate.
> IOW, we need self-insert-command's delete-section property to be a new
> function that runs a new hook on which electric-foo-mode can add their
> respective function.
There is a new abnormal hook called delete-selection-self-insert-hooks
that delete-selection-mode attaches, by default, onto
self-insert-command and self-insert-iso.
Conceivably, the other functions may also need their own hooks, but I
couldn't think of a good use case right now. Therefore, YAGNI. It is
trivial to add this later.
> - this new hook should be good enough for delete-selection-mode,
> obviously, but it should also be good enough for a replacement
> of delete-selection-mode that works differently.
> I guess the functions on that hook would mostly need to return
> a boolean indicating whether they're going to make use of the region.
Instead of booleans, those functions return one of the types that
delete-selection understands.
--
Cheers,
Simon - http://ca.linkedin.com/in/sfllaw/
[-- Attachment #2: 11520.0.patch --]
[-- Type: application/octet-stream, Size: 9863 bytes --]
diff --git a/lisp/delsel.el b/lisp/delsel.el
index a643567..09f58e0 100644
--- a/lisp/delsel.el
+++ b/lisp/delsel.el
@@ -47,6 +47,9 @@
;; non-nil
;; The normal case: delete the active region prior to executing
;; the command which will insert replacement text.
+;; hooks
+;; For commands which need to dynamically determine this behaviour.
+;; Each hook should return one of the above values or nil.
;;; Code:
@@ -71,66 +74,106 @@ any selection."
(transient-mark-mode t)))
(defun delete-active-region (&optional killp)
+ "Delete the active region.
+If KILLP in not-nil, the active region is killed instead of deleted."
(if killp
(kill-region (point) (mark))
(delete-region (point) (mark)))
t)
+(defun delete-selection-helper (type)
+ "Deletes selection according to TYPE:
+ 'yank
+ For commands which do a yank; ensures the region about to be
+ deleted isn't yanked.
+ 'supersede
+ Delete the active region and ignore the current command,
+ i.e. the command will just delete the region.
+ 'kill
+ `kill-region' is used on the selection, rather than
+ `delete-region'. (Text selected with the mouse will typically
+ be yankable anyhow.)
+ non-nil
+ The normal case: delete the active region prior to executing
+ the command which will insert replacement text.
+ hooks
+ For commands which need to dynamically determine this behaviour.
+ Each hook should return one of the above values or nil."
+ (condition-case data
+ (cond ((eq type 'kill)
+ (delete-active-region t))
+ ((eq type 'yank)
+ ;; Before a yank command, make sure we don't yank the
+ ;; head of the kill-ring that really comes from the
+ ;; currently active region we are going to delete.
+ ;; That would make yank a no-op.
+ (when (and (string= (buffer-substring-no-properties
+ (point) (mark))
+ (car kill-ring))
+ (fboundp 'mouse-region-match)
+ (mouse-region-match))
+ (current-kill 1))
+ (delete-active-region))
+ ((eq type 'supersede)
+ (let ((empty-region (= (point) (mark))))
+ (delete-active-region)
+ (unless empty-region
+ (setq this-command 'ignore))))
+ ((and (symbolp type) (not (booleanp type)))
+ (delete-selection-helper
+ (run-hook-with-args-until-success type)))
+ (type
+ (delete-active-region)
+ (if (and overwrite-mode
+ (eq this-command 'self-insert-command))
+ (let ((overwrite-mode nil))
+ (self-insert-command
+ (prefix-numeric-value current-prefix-arg))
+ (setq this-command 'ignore)))))
+ ;; If ask-user-about-supersession-threat signals an error,
+ ;; stop safe_run_hooks from clearing out pre-command-hook.
+ (file-supersession (message "%s" (cadr data)) (ding))
+ (text-read-only
+ ;; This signal may come either from `delete-active-region' or
+ ;; `self-insert-command' (when `overwrite-mode' is non-nil).
+ ;; To avoid clearing out `pre-command-hook' we handle this case
+ ;; by issuing a simple message. Note, however, that we do not
+ ;; handle all related problems: When read-only text ends before
+ ;; the end of the region, the latter is not deleted but any
+ ;; subsequent insertion will succeed. We could avoid this case
+ ;; by doing a (setq this-command 'ignore) here. This would,
+ ;; however, still not handle the case where read-only text ends
+ ;; precisely where the region starts: In that case the deletion
+ ;; would succeed but the subsequent insertion would fail with a
+ ;; text-read-only error. To handle that case we would have to
+ ;; investigate text properties at both ends of the region and
+ ;; skip the deletion when inserting text is forbidden there.
+ (message "Text is read-only") (ding))))
+
(defun delete-selection-pre-hook ()
+ "Normal hook run before commands that delete selections are executed.
+Commands which will delete the selection need a 'delete-selection
+property on their symbols; commands which insert text but don't
+have this property won't delete the selection.
+
+See `delete-selection-helper'.
+"
(when (and delete-selection-mode transient-mark-mode mark-active
(not buffer-read-only))
(let ((type (and (symbolp this-command)
(get this-command 'delete-selection))))
- (condition-case data
- (cond ((eq type 'kill)
- (delete-active-region t))
- ((eq type 'yank)
- ;; Before a yank command, make sure we don't yank the
- ;; head of the kill-ring that really comes from the
- ;; currently active region we are going to delete.
- ;; That would make yank a no-op.
- (when (and (string= (buffer-substring-no-properties
- (point) (mark))
- (car kill-ring))
- (fboundp 'mouse-region-match)
- (mouse-region-match))
- (current-kill 1))
- (delete-active-region))
- ((eq type 'supersede)
- (let ((empty-region (= (point) (mark))))
- (delete-active-region)
- (unless empty-region
- (setq this-command 'ignore))))
- (type
- (delete-active-region)
- (if (and overwrite-mode
- (eq this-command 'self-insert-command))
- (let ((overwrite-mode nil))
- (self-insert-command
- (prefix-numeric-value current-prefix-arg))
- (setq this-command 'ignore)))))
- ;; If ask-user-about-supersession-threat signals an error,
- ;; stop safe_run_hooks from clearing out pre-command-hook.
- (file-supersession (message "%s" (cadr data)) (ding))
- (text-read-only
- ;; This signal may come either from `delete-active-region' or
- ;; `self-insert-command' (when `overwrite-mode' is non-nil).
- ;; To avoid clearing out `pre-command-hook' we handle this case
- ;; by issuing a simple message. Note, however, that we do not
- ;; handle all related problems: When read-only text ends before
- ;; the end of the region, the latter is not deleted but any
- ;; subsequent insertion will succeed. We could avoid this case
- ;; by doing a (setq this-command 'ignore) here. This would,
- ;; however, still not handle the case where read-only text ends
- ;; precisely where the region starts: In that case the deletion
- ;; would succeed but the subsequent insertion would fail with a
- ;; text-read-only error. To handle that case we would have to
- ;; investigate text properties at both ends of the region and
- ;; skip the deletion when inserting text is forbidden there.
- (message "Text is read-only") (ding))))))
-
-(put 'self-insert-command 'delete-selection t)
-(put 'self-insert-iso 'delete-selection t)
+ (delete-selection-helper type))))
+
+(defun delete-selection-self-insert-function ()
+ t)
+
+(defvar delete-selection-self-insert-hooks
+ '(delete-selection-self-insert-function)
+ "Abnormal hook run before commands that insert characters.
+This hook should return a TYPE that `delete-selection-helper' understands.")
+
+(put 'self-insert-command 'delete-selection 'delete-selection-self-insert-hooks)
+(put 'self-insert-iso 'delete-selection 'delete-selection-self-insert-hooks)
(put 'yank 'delete-selection 'yank)
(put 'clipboard-yank 'delete-selection 'yank)
diff --git a/lisp/electric.el b/lisp/electric.el
index 3108a0e..e6fa1df 100644
--- a/lisp/electric.el
+++ b/lisp/electric.el
@@ -301,14 +301,17 @@ This can be convenient for people who find it easier to hit ) than C-f."
:version "24.1"
:type 'boolean)
+(defun electric-pair-syntax (command-event)
+ (and electric-pair-mode
+ (let ((x (assq command-event electric-pair-pairs)))
+ (cond
+ (x (if (eq (car x) (cdr x)) ?\" ?\())
+ ((rassq command-event electric-pair-pairs) ?\))
+ (t (char-syntax command-event))))))
+
(defun electric-pair-post-self-insert-function ()
(let* ((syntax (and (eq (char-before) last-command-event) ; Sanity check.
- electric-pair-mode
- (let ((x (assq last-command-event electric-pair-pairs)))
- (cond
- (x (if (eq (car x) (cdr x)) ?\" ?\())
- ((rassq last-command-event electric-pair-pairs) ?\))
- (t (char-syntax last-command-event))))))
+ (electric-pair-syntax last-command-event)))
;; FIXME: when inserting the closer, we should maybe use
;; self-insert-command, although it may prove tricky running
;; post-self-insert-hook recursively, and we wouldn't want to trigger
@@ -355,6 +358,12 @@ This can be convenient for people who find it easier to hit ) than C-f."
(eq (char-syntax (following-char)) ?w)))
(save-excursion (insert closer))))))
+(defun electric-pair-delete-selection-self-insert-function ()
+ (let ((syntax (electric-pair-syntax last-command-event)))
+ (if (and (memq syntax '(?\( ?\" ?\$)) (use-region-p))
+ 'keep
+ t)))
+
;;;###autoload
(define-minor-mode electric-pair-mode
"Toggle automatic parens pairing (Electric Pair mode).
@@ -370,10 +379,16 @@ See options `electric-pair-pairs' and `electric-pair-skip-self'."
:global t
:group 'electricity
(if electric-pair-mode
- (add-hook 'post-self-insert-hook
- #'electric-pair-post-self-insert-function)
+ (progn
+ (require 'delsel)
+ (add-hook 'post-self-insert-hook
+ #'electric-pair-post-self-insert-function)
+ (add-hook 'delete-selection-self-insert-hooks
+ #'electric-pair-delete-selection-self-insert-function))
(remove-hook 'post-self-insert-hook
- #'electric-pair-post-self-insert-function)))
+ #'electric-pair-post-self-insert-function)
+ (remove-hook 'delete-selection-self-insert-hooks
+ #'electric-pair-delete-selection-self-insert-function)))
;; Automatically add newlines after/before/around some chars.
next prev parent reply other threads:[~2012-10-21 23:12 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-19 20:07 bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode Simon Law
2012-05-19 22:41 ` bug#11520: Workaround Simon Law
2012-05-20 14:54 ` bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode Stefan Monnier
2012-05-20 16:31 ` Drew Adams
2012-05-21 4:19 ` Simon Law
2012-06-11 21:24 ` Simon Law
2012-07-14 5:23 ` Chong Yidong
2012-07-14 6:42 ` Simon Law
2012-07-15 14:39 ` Chong Yidong
2012-07-15 16:33 ` Simon Law
2012-07-18 12:13 ` Stefan Monnier
2012-07-18 13:55 ` Drew Adams
2012-10-08 22:25 ` Stefan Monnier
2012-10-21 23:12 ` Simon Law [this message]
2012-10-22 12:46 ` Stefan Monnier
2012-10-23 1:07 ` Simon Law
2012-10-23 15:10 ` Stefan Monnier
[not found] ` <b47bddfc-e35c-4ab2-8f4f-3e0a51bec33c@default>
2014-08-18 15:21 ` Stefan Monnier
[not found] ` <<jwvboft8hsz.fsf-monnier+emacs@gnu.org>
2014-07-24 17:34 ` Drew Adams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABMJL3L7SXmixpr327pf9kKduPny39xjgXmykRYugTjgQdDDzA@mail.gmail.com \
--to=sfllaw@sfllaw.ca \
--cc=11520@debbugs.gnu.org \
--cc=cyd@gnu.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.