bug#10022: 24.0.91; `isearch-mouse-2' relies on `x-get-selection' - NG for Windows etc.

[Drew Adams]

> > Dunno whether a binding of `ignore' makes better sense for
> > vanilla Emacs here too (e.g., in case someone binds
> > `down-mouse-2' globally).
> 
> To the extent that isearch only binds mouse-2 in the minibuffer (yes,
> it technically also binds it in the main buffer, but to a command that
> just delegates to the normal binding), I don't think 
> down-mouse-2 should default to `ignore', except maybe in the minibuffer.

What I meant was that a nil binding means pick up any currently effective (e.g.
global) binding.  If someone binds `down-mouse-2' globally (as I do) then that
binding will be picked for Isearch also, and it might not be appropriate there.

The question is whether you want Isearch to ignore a `down-mouse-2' action or to
pick up any `down-mouse-2' action that might be defined (e.g. globally).  IOW,
should Isearch decide that it wants to impose the `down-mouse-2' behavior or
should it let whatever behavior is defined elsewhere carry over to Isearch as
well.

Your call.

> > That means that `isearch-mouse-2' barfs when it calls
> > `isearch-yank-selection'.  That function tries to yank the string
> > returned by `x-get-selection', but that function returns nil, not a
> > string.  The PRIMARY selection is not set, at least in my context.
> > This function apparently depend ons it being set.  Seems like a
> > bug, to me.
> 
> AFAIK it's called "isearch-yank-x-selection",

Yes, that's what I meant.

> so it really doesn't seem like a bug for it to fail when there is
> no GUI selection

The bug would presumably be that `x-get-selection' returns nil here, but I would
suppose that `isearch-yank-x-selection' might want to handle such an eventuality
more gracefully.  Anyway, there does not seem to be a problem for Emacs 24 -
only older versions.

> > (when (and transient-mark-mode (/= (region-beginning) (region-end)))
> >   (x-set-selection 'PRIMARY (buffer-substring-no-properties
> >                              (region-beginning) (region-end)))
> >   (deactivate-mark)
> >   (isearch-yank-x-selection))
> 
> That presumes the region was meant as a "selection".  I don't see why
> isearch should make such an assumption.

I gave a later version that does not use that - see the definition of
`isearch-mouse-2' I sent.  The part that corresponds to the ordinary, vanilla
behavior is identical to the vanilla Emacs code.  The part that uses the region
as a selection is per user choice and is a separate feature: letting you click
mouse-2 anywhere (not necessarily in the echo area), to insert the region text.

> > Consider this only an FYI.  I suspect there is a problem, 
> > and that the `isearch-mouse-2' code should not depend on the PRIMARY 
> > selection being set, but you decide.
> 
> IIUC the intention is for it to perform a task similar to 
> `yank', so it needs to take the string from some kind of GUI
> selection, or from the kill-ring, but taking it straight from the
> region seems a bit much.

Agreed.  You misunderstood.  Taking it from the region is for a separate,
alternative behavior (user choice).

> > The binding of `select-active-region' is needed, however.
> > Without that (and with a customized setting of nil), 
> > `get-x-selection' returns nil - the problem I mentioned earlier.
> 
> But that overrides an explicit request from the user (in her 
> .emacs) to not automatically consider the region as a selection.

Again, this is per a user choice, via option `isearchp-mouse-2-flag'.  The
intent of such a choice is precisely to use the region as a selection here (and
here only).  Again, this is outside what vanilla Emacs does or should do.

> So, while it might make sense when coupled with a new option such as
> isearchp-mouse-2-flag, I don't see what Emacs-24.1 could do to help.

Agreed.  There is no change needed for vanilla Emacs 24.  The only changes I
have made are (a) for a non-nil option `isearchp-mouse-2-flag' (yank the
non-empty region) and (b) for older Emacs versions (ensure the primary selection
is set, so it can be yanked).

This is why this is all only FYI (except perhaps for the `down-mouse-2' binding
question - your call).