bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Simon Law]

delete-selection-mode clobbers text before electric-pair-mode can get
to it. Since they are both built-in to Emacs, the modes should be aware
of each other.

Reproduction steps:

$ emacs -Q
M-x electric-pair-mode
M-x delete-selection-mode
;; Type "a line of text""
C-SPC
C-a
;; At this point, you should have the line of text selected
(

Expected result:

(a line of text)

Actual result:

)(


In GNU Emacs 24.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.2.0)
 of 2012-05-10 on charichuelo, modified by Debian
 (emacs-snapshot package, version 2:20120510-1~ppa1~oneiric1)
Windowing system distributor `The X.Org Foundation', version 11.0.11004000
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'
 '--mandir=/usr/share/man' '--with-pop=yes'
 '--enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/24.1.50/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.1.50/site-lisp:/usr/share/emacs/site-lisp'
 '--without-compress-info' '--with-crt-dir=/usr/lib/x86_64-linux-gnu/'
 '--with-x=yes' '--with-x-toolkit=gtk3' '--with-imagemagick=yes'
 'build_alias=x86_64-linux-gnu' 'host_alias=x86_64-linux-gnu'
 'CFLAGS=-DDEBIAN -DSITELOAD_PURESIZE_EXTRA=5000 -g -O2' 'LDFLAGS=-g
 -Wl,--as-needed -znocombreloc' 'CPPFLAGS=''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: en_CA.UTF-8
  value of $LC_CTYPE: en_CA.UTF-8
  value of $LC_MESSAGES: en_CA.UTF-8
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_CA.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Lisp Interaction

Minor modes in effect:
  outline-minor-mode: t
  whitespace-mode: t
  flyspell-mode: t
  fci-mode: t
  ido-ubiquitous-mode: t
  helm-dired-mode: Enable helm completion in Dired functions.
Bindings affected are C, R, S, H.
This is deprecated for Emacs24+ users, use `helm-mode' instead.
  helm-match-plugin-mode: t
  projectile-mode: t
  projectile-global-mode: t
  yas/global-mode: t
  yas/minor-mode: t
  icomplete-mode: t
  volatile-highlights-mode: t
  show-paren-mode: t
  shell-dirtrack-mode: t
  recentf-mode: t
  savehist-mode: t
  electric-pair-mode: t
  global-auto-revert-mode: t
  prelude-mode: t
  prelude-global-mode: t
  which-function-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Recent input:
n t a x SPC - t a b l e <M-backspace> <M-backspace>
c u r r e n t - s y n t a <M-backspace> <M-backspace>
l i s p - m o d e - s y n <return> <down-mouse-1> <mouse-movement>
<mouse-1> C-v C-v C-v C-v C-v C-v C-v C-v C-v C-v C-v
C-v C-v C-v C-v C-v M-< <help-echo> <down-mouse-1>
<mouse-1> C-s ( C-n C-n C-v C-v C-v C-v C-v C-v C-v
C-v C-v C-v C-v C-v C-v C-v C-v C-v C-v C-v C-v C-v
C-v M-v M-v C-x b <return> C-s s y n t a x - t a b
l e C-s C-n C-p C-n C-p C-p C-x o C-h v s y n t a x
- t a b l e C-g C-g C-n C-x o M-< C-s s y n t a x -
t a b l e C-s C-n C-p C-x o C-h f s y n t a x - t a
b l e <return> C-x o q C-x o C-x o M-: M-( a r e f
M-b C-b C-k l a s t - c o m m a n d - e v e n t <return>
<help-echo> M-: M-p C-k ( a r e f SPC ( s y n t a x
- t a b l e ) SPC ? ( <backspace> \ ( <return> C-x
o C-n C-n C-n C-n C-p C-p C-p M-v M-v C-v C-v C-p C-p
C-p C-p C-p C-p C-p C-p C-p C-p C-p C-n C-v C-v M-v
M-v <down-mouse-1> <mouse-movement> <mouse-1> <M-backspace>
M-x r e p o r t - b u g <return> C-g C-g M-x r e p
o r t - e m a c s - b u g <return>

Recent messages:
byte-code: End of buffer
Mark set
Mark saved where search started
(No changes need to be saved)
Type "q" in help window to restore its previous buffer.
(No changes need to be saved)
13 (#o15, #xd)
Character to escape: (
(4 . 41)
Quit
Quit

Load-path shadows:
/usr/share/emacs/24.1.50/site-lisp/debian-startup hides
/usr/share/emacs/site-lisp/debian-startup
/home/sfllaw/.emacs.d/elpa/python-20120430/python hides
/usr/share/emacs/24.1.50/lisp/progmodes/python
/home/sfllaw/.emacs.d/elpa/magit-20120507/.dir-locals hides
/usr/share/emacs/24.1.50/lisp/gnus/.dir-locals

Features:
(shadow sort mail-extr emacsbug message rfc822 mml mml-sec mm-decode
mm-bodies mm-encode mailabbrev gmm-utils mailheader sendmail cua-base
jka-compr cus-start cus-load add-log flymake-cursor debug hippie-exp
multi-isearch rainbow-mode eldoc paredit prelude-emacs-lisp-autoloads
prelude-lisp prelude-lisp-autoloads autoload find-func pp python
mule-util executable time-stamp vc-git mail-utils network-stream
starttls url-http tls mail-parse rfc2231 rfc2047 rfc2045 ietf-drums
url-gw url-cache url-auth noutline outline whitespace flyspell ispell
melpa multi-term term disp-table ehelp python-el-fgallina-expansions
flymake fill-column-indicator ido-ubiquitous prelude-global-keybindings
prelude-editor re-builder midnight helm-projectile helm-config helm-misc
helm-files image-dired dired-x dired-aux ffap helm-tags helm-bookmark
helm-adaptative helm-info helm-net browse-url xml url url-proxy
url-privacy url-expand url-methods url-history url-cookie url-domsuf
url-util url-parse url-vars mailcap helm-plugin helm-locate helm-help
helm-match-plugin helm-grep helm-regexp grep helm-external helm-buffers
rx helm-elscreen helm-utils dired compile helm projectile yasnippet
help-mode view assoc expand-region expand-region-custom
expand-region-core icomplete ido etags volatile-highlights hl-line paren
windmove tramp-cache tramp-sh tramp tramp-compat auth-source gnus-util
mm-util mail-prsvr password-cache shell pcomplete comint ansi-color ring
format-spec tramp-loaddefs recentf tree-widget wid-edit savehist
saveplace uniquify electric autorevert delsel prelude-mode easy-mmode
easymenu edmacro kmacro prelude-core repeat thingatpt prelude-ui
zenburn-theme prelude-packages ack-and-a-half-autoloads
expand-region-autoloads flymake-css-autoloads flymake-cursor-autoloads
gist-autoloads gh-autoloads eieio helm-projectile-autoloads
helm-autoloads ido-ubiquitous-autoloads less-css-mode-autoloads
logito-autoloads magithub-autoloads magit-autoloads
markdown-mode-autoloads mediawiki-autoloads melpa-autoloads byte-opt
warnings bytecomp byte-compile cconv macroexp advice help-fns
advice-preload multi-term-autoloads paredit-autoloads pcache-autoloads
finder-inf pony-mode-autoloads prelude-programming-autoloads which-func
imenu projectile-autoloads python-autoloads python-magic-autoloads
rainbow-mode-autoloads volatile-highlights-autoloads yasnippet-autoloads
zenburn-theme-autoloads package cl time-date tooltip ediff-hook vc-hooks
lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment lisp-mode register page menu-bar
rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax
facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak
czech european ethiopic indian cyrillic chinese case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer loaddefs button faces
cus-face files text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget hashtable-print-readable backquote
make-network-process dbusbind dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)


-- 
Cheers,
Simon - http://ca.linkedin.com/in/sfllaw/

bug#11520: Workaround

[Simon Law]

I use the following defadvice as a hack to workaround this bug:

(defadvice delete-active-region (around electric-pair-or-delete-active-region
                                        activate)
  "Prevent delete-active-region from clobbering electric-pair-mode."
  (unless (and electric-pair-mode
               (or (eq ?\( (char-syntax last-command-event))
                   (assq last-command-event electric-pair-pairs)))
    ad-do-it)
  t)

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Stefan Monnier]

> $ emacs -Q
> M-x electric-pair-mode
> M-x delete-selection-mode
> ;; Type "a line of text""
> C-SPC
> C-a
> ;; At this point, you should have the line of text selected
> (

> Expected result:
> (a line of text)
> Actual result:
> )(

Thanks for the recipe.  I'm not sure yet how best to solve
this problem.  Basically, the issue is that delete-selection-mode
operates before the ( is processed, so it has to "guess" what ( will do.

Most likely the cleanest solution will be to add some
"delete-selection-predicates" hook that electric-pair-mode can use to
explain when to inhibit delete-selection.


        Stefan

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Drew Adams]

> Thanks for the recipe.  I'm not sure yet how best to solve
> this problem.  Basically, the issue is that delete-selection-mode
> operates before the ( is processed, so it has to "guess" what 
> ( will do.
> 
> Most likely the cleanest solution will be to add some
> "delete-selection-predicates" hook that electric-pair-mode can use to
> explain when to inhibit delete-selection.

I hesitate to jump in here.  I like `d-s' mode, and I hope we do not radically
alter its design (which keys off of certain command symbol properties, and which
takes place pre-command).

I don't claim expertise here; I just like the way `d-s' handles and is easy to
configure.  It essentially hooks into specific commands, taking effect just
before they do.

`e-p-m' does not change the bindings of self-inserting keys - they are still
bound to `self-insert-command'.  And `d-s-m' treats that command as it should:
the self-insertion is preceded by deletion of the region.

`d-s' mode determines the particular behavior from the command.  Its behavior is
easily changeable by setting different properties on the command symbol.  But a
given command (e.g. `self-insert-command') has a single delete-selection
behavior.

`d-s' takes effect _before_ the affected command (on `pre-command-hook'), not
after it.  `e-p', on the contrary, does its thing _after_ the command (there is
apparently only one such command, so far: `self-insert-command').

`e-p' depends on `post-self-insert-hook', which is new in Emacs 24.  I see that
this hook is also used by other things in Emacs 24.  Dunno whether those other
cases might also be problematic for use with d-s mode.

`e-p', like `d-s', is command-driven - it is associated with only one command,
in a hard-coded way rather than by looking up command-symbol properties.  Its
behavior is thus not variable depending on the command (only one command, only
one behavior).  But this difference is not really important here, I think.

The real difference is when the special effect takes place: before or after the
affected command (`self-insert-command', in this case).

Since `d-s' does its thing (deletion) before the relevant command, it is natural
that it take effect pre-command.  Since `e-p' does its thing (insertion) after
the relevant command (`self-insert-command' only), it is natural that it take
effect post-command.

But as you say, Stefan, a pre-command approach like d-s knows nothing about what
might be expected to happen after the command is done.

It could be possible to let `e-p' know what `d-s' has already done, but I do not
see how it is possible/practical to let `d-s' know what `e-p' will do.  We could
make `d-s' aware that `e-p' mode is turned on, and we could even try to let it
know exactly what `e-p' mode does (not a very modular approach, but perhaps
doable).

But knowing that, `d-s' would have to not only test for `self-insert-command'
but also test the character to be inserted.  We would end up, I'm afraid,
duplicating nearly all of the `e-p' logic inside `d-s'.  (But perhaps I don't
understand well what you had in mind.)

Anyway, that mismatch (pre-command control vs post) is I guess the problem here.
If so, it might be seen to be more general than `d-s' vs `e-p'.

I don't really have a solution.  (Mainly I have a concern that `d-s' mode not be
altered radically.)

Wrt your proposal: I don't quite understand.  `e-p' wants to inhibit `d-s'
deletion only in the case where a `(' (or whatever else, such as `"') is
inserted.  It does not want to inhibit it generally, correct?

If so, how can it possibly inhibit it in only some cases, if it does not even
take effect until after the `(' command (= `self-insert-command') is finished?

I don't see how `e-p', acting post-command, can _inhibit_ something that happens
pre-command.  `d-s' could perhaps be made aware that `e-p' is coming up, but to
make `d-s' DTRT I would think that it would need to know nearly everything about
`e-p'.

`e-p' could, however undo/reverse what `d-s' did.  In the case of killing this
would be simple, I imagine, but it might not be quite so trivial in other `d-s'
cases.  Perhaps `d-s' could save some "undoing" info for the eventuality that
some post-command action (e.g. `e-p') might want to reverse what `d-s' did.

That's probably what I would propose: have `e-p' somehow undo what `d-s' did.

But I'm sure you know more about this than I.  My real concern is that we not
lose any of the basic design of `d-s': pre-command & driven by properties on
command symbols.

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Simon Law]

On Sun, May 20, 2012 at 12:31 PM, Drew Adams <drew.adams@oracle.com> wrote:
> But as you say, Stefan, a pre-command approach like d-s knows nothing about what
> might be expected to happen after the command is done.
>
> It could be possible to let `e-p' know what `d-s' has already done, but I do not
> see how it is possible/practical to let `d-s' know what `e-p' will do.  We could
> make `d-s' aware that `e-p' mode is turned on, and we could even try to let it
> know exactly what `e-p' mode does (not a very modular approach, but perhaps
> doable).
>
> But knowing that, `d-s' would have to not only test for `self-insert-command'
> but also test the character to be inserted.  We would end up, I'm afraid,
> duplicating nearly all of the `e-p' logic inside `d-s'.  (But perhaps I don't
> understand well what you had in mind.)
>
> Anyway, that mismatch (pre-command control vs post) is I guess the problem here.
> If so, it might be seen to be more general than `d-s' vs `e-p'.

It seems to me that delete-selection-mode already knows about four
types of commands: 'yank, 'supercede, 'kill, and non-nil. It looks
like it could take a function, which will inhibit
delete-selection-mode if it returns nil, or return any of the other
commands. This way, electric-pair-mode can register its own behaviour
with delete-selection-mode when it's activated.

-- 
Cheers,
Simon - http://ca.linkedin.com/in/sfllaw/

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Simon Law]

I now use the following workaround, in order to preserve
string-wrapping behaviour:

(defadvice delete-active-region (around electric-pair-or-delete-active-region
                                        activate)
  "Prevent delete-active-region from clobbering electric-pair-mode."
  (unless (and electric-pair-mode
               (or (memq (char-syntax last-command-event) '(?\( ?\"))
                   (assq last-command-event electric-pair-pairs)))
    ad-do-it)
  t)

-- 
Cheers,
Simon - http://ca.linkedin.com/in/sfllaw/

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Chong Yidong]

Simon Law <sfllaw@sfllaw.ca> writes:

> $ emacs -Q
> M-x electric-pair-mode
> M-x delete-selection-mode
> ;; Type "a line of text""
> C-SPC
> C-a
> ;; At this point, you should have the line of text selected
> (
>
> Actual result:
>
> )(

Fixed in trunk.  Thanks.

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Simon Law]

On Sat, Jul 14, 2012 at 1:23 AM, Chong Yidong <cyd@gnu.org> wrote:
> Simon Law <sfllaw@sfllaw.ca> writes:
>
>> $ emacs -Q
>> M-x electric-pair-mode
>> M-x delete-selection-mode
>> ;; Type "a line of text""
>> C-SPC
>> C-a
>> ;; At this point, you should have the line of text selected
>> (
>>
>> Actual result:
>>
>> )(
>
> Fixed in trunk.  Thanks.

Hi Chong,

I tested your fix and it makes electric-pair-mode insert pairs in the
right order now.

But the bug is that delete-selection-mode destroys
electric-pair-mode's auto-wrapping ability. Unfortunately, your fix
does not address that.

Would you like me to come up with a patch that implements my
suggestions above in the BTS?

Thanks,
- Simon

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Chong Yidong]

Simon Law <sfllaw@sfllaw.ca> writes:

> I tested your fix and it makes electric-pair-mode insert pairs in the
> right order now.
>
> But the bug is that delete-selection-mode destroys
> electric-pair-mode's auto-wrapping ability. Unfortunately, your fix
> does not address that.

Hmm, I guess I misunderstood the bug.  I'll re-open it.

Trying to work up a patch that does the right thing is not so
straightforward, though, as electric-pair-post-self-insert-function does
not have a simple triggering condition.

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Simon Law]

On Sun, Jul 15, 2012 at 10:39 AM, Chong Yidong <cyd@gnu.org> wrote:
> Simon Law <sfllaw@sfllaw.ca> writes:
>
>> I tested your fix and it makes electric-pair-mode insert pairs in the
>> right order now.
>>
>> But the bug is that delete-selection-mode destroys
>> electric-pair-mode's auto-wrapping ability. Unfortunately, your fix
>> does not address that.
>
> Hmm, I guess I misunderstood the bug.  I'll re-open it.
>
> Trying to work up a patch that does the right thing is not so
> straightforward, though, as electric-pair-post-self-insert-function does
> not have a simple triggering condition.

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
understands the triggering condition. The triggering condition would
be extracted from electric-pair-post-self-insert-function so it could
be used in two places.

Sound good?

P.S. I don't have an bzr checkout of Emacs right now. Would this be
something useful to target against Emacs 24 as a bugfix?

-- 
Cheers,
Simon - http://ca.linkedin.com/in/sfllaw/

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Stefan Monnier]

> That way, electric-pair-mode can override the (put
> 'self-insert-command 'delete-selection ...) with its own function that
> understands the triggering condition. The triggering condition would
> be extracted from electric-pair-post-self-insert-function so it could
> be used in two places.

That sounds like a reasonable plan, yes.

An alternative is to change self-insert-command so that it pays
attention to delete-selection-mode (and provides some hook for
electric-pair-mode to indicate when it would use the region).

> P.S. I don't have an bzr checkout of Emacs right now. Would this be
> something useful to target against Emacs 24 as a bugfix?

Yes.



        Stefan


PS: I don't much like delete-selection-mode because of its implementation
strategy (using a pre-command-hook along with symbol properties, as
opposed to modifying the commands themselves).  I can fully understand
why it was done this way, but I'd be happy to see it changed, along the
same lines as what we did for the shift-select-mode.

AFAICT, of the various `delete-selection' properties, `kill' is only
used for `open-line' (for no good reason) and could be eliminated.
`supersede' is only used by command which should obey
`delete-active-region' or close enough, `yank' is only used for commands
which end up calling `yank' (so adding code to the `yank' command would
cover them), and t is used for commands which end up calling
`self-insert-command' (with a few minor exceptions like open-line).

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Drew Adams]

> I don't much like delete-selection-mode because of its 
> implementation strategy (using a pre-command-hook along with
> symbol properties, as opposed to modifying the commands
> themselves).  I can fully understand why it was done this
> way, but I'd be happy to see it changed, along the
> same lines as what we did for the shift-select-mode.

FWIW, I am not enthused by that prospect.

> AFAICT, of the various `delete-selection' properties, `kill' is only
> used for `open-line' (for no good reason) and could be eliminated.
> `supersede' is only used by command which should obey
> `delete-active-region' or close enough, `yank' is only used 
> for commands which end up calling `yank' (so adding code to the
> `yank' command would cover them), and t is used for commands
> which end up calling `self-insert-command' (with a few minor
> exceptions like open-line).

Delete-selection mode is not only for use by end users out of the box with
vanilla Emacs.  It is especially designed to be easy to use to adapt code,
modes, and general interactive behavior, for both users and other libraries.

In this respect it is similar to the general programming functionality provided
by thingatpt.el.  It would be short-sighted to look only to how the existing
code uses thing-at-point features and ignore how they can be (are designed to
be) used more generally.  Same with delete-selection mode.

When will Emacs Dev stop looking only to the existing vanilla code that is
distributed to determine the "only" uses of some existing functionality.  That
last paragraph of yours is a classic: feature A seems only to be used for X,Y,Z
in the Emacs code, so let's simplify things by amputating the feature and
hard-coding its uses into the existing code.

The design of delete-selection mode is for its potential uses, not just its
existing uses in the current code that you maintain.  Such proposed amputation
is like looking at how the character `(' is actually used in one file and
proposing to change the possibility to use it more generally, limiting it to its
uses in that one file.

Just one opinion - from a longtime delete-selection mode user and someone who
has also used it in programming.  (Likewise thing at point.)

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Stefan Monnier]

> 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.
- 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.


        Stefan

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Simon Law]

[-- 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.
 

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Stefan Monnier]

> Attached is a patch against trunk.

Thanks, I really appreciate the addition of docstrings.
it's got a few cosmetic problems, so I've installed it and then
installed a subsequent patch on top of it.

Here are the issues I noticed:
- a hook is a symbol whose value contains a list of functions (modulo
  a few subtleties).  The functions themselves are not hooks.
  E.g. delete-selection-pre-hook is not a hook, just a function that's
  typically added on a particular hook.
  And the symbol symbol is named with "-hook" (rather than with
  "-hooks") for normal hooks and "-functions" for abnormal hooks.
- symbols in docstrings are written `foo' rather than 'foo.
- docstrings normally don't end with a line-separator (i.e. the closing
  " should not be on its own line).
- there's no self-insert-iso in Emacs, so I'd rather not touch it.
- use the imperative for the first line of a docstring.
- `keep' is not a value handled specially by delete-selection-helper,
  so it is handled as a hook, luckily there's no function on this hook
  (an unbound symbol is treated by `run-hook' as a symbol bound to nil)
  so it ends up behaving like nil, which is indeed what we need; so the
  end behavior is correct, but only "by accident".


        Stefan

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Simon Law]

On Mon, Oct 22, 2012 at 8:46 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> Attached is a patch against trunk.
>
> Thanks, I really appreciate the addition of docstrings.
> it's got a few cosmetic problems, so I've installed it and then
> installed a subsequent patch on top of it.
>
> Here are the issues I noticed:
> - a hook is a symbol whose value contains a list of functions (modulo
>   a few subtleties).  The functions themselves are not hooks.
>   E.g. delete-selection-pre-hook is not a hook, just a function that's
>   typically added on a particular hook.

Yeah, I wasn't sure about delete-selection-pre-hook, but that's what
the function was already called, so I wasn't going to change the API
there.

>   And the symbol symbol is named with "-hook" (rather than with
>   "-hooks") for normal hooks and "-functions" for abnormal hooks.

Ah, I read that it was either one or the other in
http://www.gnu.org/software/emacs/manual/html_node/elisp/Hooks.html.
Perhaps that documentation should discourage the -hooks convention?

> - symbols in docstrings are written `foo' rather than 'foo.

In delete-selection-helper, you mean? Yes, that appears to be a
copy-paste error.

> - docstrings normally don't end with a line-separator (i.e. the closing
>   " should not be on its own line).

Whoops. Thanks for catching that.

> - there's no self-insert-iso in Emacs, so I'd rather not touch it.

delete-selection-mode.el used to touch self-insert-iso, as does
cua-rect.el. Is this something to clean up, or is there something I'm
missing?

> - use the imperative for the first line of a docstring.

Will do.

> - `keep' is not a value handled specially by delete-selection-helper,
>   so it is handled as a hook, luckily there's no function on this hook
>   (an unbound symbol is treated by `run-hook' as a symbol bound to nil)
>   so it ends up behaving like nil, which is indeed what we need; so the
>   end behavior is correct, but only "by accident".

Gosh, I don't know how that snuck in there. That was a think-o.

Finally, I noticed that you created self-insert-uses-region-functions,
but there is no defvar or docstring for that. For future reference, is
it normal that packages have hidden hooks like that?

Thanks for the cleanup!

-- 
Cheers,
Simon - http://ca.linkedin.com/in/sfllaw/

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Stefan Monnier]

> Yeah, I wasn't sure about delete-selection-pre-hook, but that's what
> the function was already called, so I wasn't going to change the API
> there.

Yes, the function name is better left as is (and really isn't bad), the
error was in the docstring (where you described the function as
a normal hook).

>> And the symbol symbol is named with "-hook" (rather than with
>> "-hooks") for normal hooks and "-functions" for abnormal hooks.
> Ah, I read that it was either one or the other in
> http://www.gnu.org/software/emacs/manual/html_node/elisp/Hooks.html.

Historically, "-hooks" was used, but they've (all?) been renamed to "-hook".

> Perhaps that documentation should discourage the -hooks convention?

Yes, thanks, the wording wasn't clear enough.  Fixed.

>> - there's no self-insert-iso in Emacs, so I'd rather not touch it.
> delete-selection-mode.el used to touch self-insert-iso, as does
> cua-rect.el.  Is this something to clean up, or is there something I'm
> missing?

I don't know where this self-insert-iso comes from, so we should just
remove it, but I'll keep this change for after the feature freeze.
For now, I simply left the setting as it used to be.

> Finally, I noticed that you created self-insert-uses-region-functions,
> but there is no defvar or docstring for that.  For future reference, is
> it normal that packages have hidden hooks like that?

No, it's not normal.  Thanks for catching this missing commit.


        Stefan

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Stefan Monnier]

>> > Finally, I noticed that you created self-insert-uses-region-functions,
>> > but there is no defvar or docstring for that.  For future reference, is
>> > it normal that packages have hidden hooks like that?
>> No, it's not normal.  Thanks for catching this missing commit.
> ping.

Done,


        Stefan

bug#11520: 24.1.50; delete-selection-mode conflicts with electric-pair-mode

[Drew Adams]

> > Finally, I noticed that you created self-insert-uses-region-functions,
> > but there is no defvar or docstring for that.  For future reference, is
> > it normal that packages have hidden hooks like that?
> 
> No, it's not normal.  Thanks for catching this missing commit.

ping