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/