bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Drew Adams]

Regression after Emacs 24.3.

emacs -Q

Visit dired.el. Double-click on the opening paren of

(defun dired-get-filename ...

Only part of the defun is selected.  It seems that a defun needs to fit
entirely within the window (i.e., be completely visible) for mouse
selection by double-clicking to work correctly now.



In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
 of 2014-05-17 on ODIEONE
Bzr revision: 117119 eggert@cs.ucla.edu-20140517081131-ugu7ociaoec2xk7y
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/snapshot/trunk
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -g3'
 LDFLAGS=-Lc:/Devel/emacs/lib 'CPPFLAGS=-DGC_MCHECK=1
 -Ic:/Devel/emacs/include''

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Drew Adams]

This is still a regression in this build:

In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
 of 2014-05-25 on ODIEONE
Repository revision: 117153 tsdh@gnu.org-20140525174054-vzeh4zeg00a1ley8
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/snapshot/trunk
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -g3'
 LDFLAGS=-Lc:/Devel/emacs/lib 'CPPFLAGS=-DGC_MCHECK=1
 -Ic:/Devel/emacs/include''

Not that this makes use of Emacs more error-prone.  Mouse selection
of a sexp by double-clicking should not be affected by the current
window limits.  The exact sexp should be selected - not less than
the sexp and not more than the sexp.

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Drew Adams]

> Not that this makes use of Emacs more error-prone.
  ^^^
  Note

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Stephen Berman]

On Fri, 23 May 2014 10:00:17 -0700 (PDT) Drew Adams <drew.adams@oracle.com> wrote:

> Regression after Emacs 24.3.
>
> emacs -Q
>
> Visit dired.el. Double-click on the opening paren of
>
> (defun dired-get-filename ...
>
> Only part of the defun is selected.  It seems that a defun needs to fit
> entirely within the window (i.e., be completely visible) for mouse
> selection by double-clicking to work correctly now.
>
>
> In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
>  of 2014-05-17 on ODIEONE
> Bzr revision: 117119 eggert@cs.ucla.edu-20140517081131-ugu7ociaoec2xk7y
> Windowing system distributor `Microsoft Corp.', version 6.1.7601

I also noticed this problem with builds on GNU/Linux, starting with
trunk bzr 117109 (though prior to that build I had been building only
from the emacs-24 branch for a while, so I don't know when the problem
first appeared in trunk, and I haven't had time to bisect).  AFAICT it
happens in any mode, not just Emacs Lisp mode.  I don't see the problem
with builds from the emacs-24 branch.

Steve Berman

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Drew Adams]

> From: Stephen Berman
>
> I also noticed this problem with builds on GNU/Linux, starting with
> trunk bzr 117109 (though prior to that build I had been building only
> from the emacs-24 branch for a while, so I don't know when the problem
> first appeared in trunk, and I haven't had time to bisect).  AFAICT it
> happens in any mode, not just Emacs Lisp mode.  I don't see the problem
> with builds from the emacs-24 branch.

I don't see it with this build, so it seems to have been introduced
between 4/29 and 5/17.  Unfortunately, I have no build between those
two, to check in more detail.

In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
 of 2014-04-29 on ODIEONE
Repository revision: 117031 monnier@iro.umontreal.ca-20140429151607-qnkgbymwfaj5ut08
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/snapshot/trunk
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -g3'
 LDFLAGS=-Lc:/Devel/emacs/lib 'CPPFLAGS=-DGC_MCHECK=1
 -Ic:/Devel/emacs/include''

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> I don't see it with this build, so it seems to have been introduced
> between 4/29 and 5/17.  Unfortunately, I have no build between those
> two, to check in more detail.

It seems to have been introduced by this commit:

44812aa45a823e344811f4f38966c3e5c9b5118f
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Sun May 11 01:49:14 2014 -0400


    * lisp/mouse.el: Use the normal toplevel loop while dragging.
    (mouse-set-point): Handle multi-clicks.
    (mouse-set-region): Handle multi-clicks for drags.
    (mouse-drag-region): Update call accordingly.
    (mouse-drag-track): Remove `do-mouse-drag-region-post-process' hack.
    Use the normal event loop instead of a local while/read-event loop.
    (global-map): Remove redundant bindings for double/triple-mouse-1.
    * lisp/xt-mouse.el (xterm-mouse-translate-1): Only process one event at a time.
    Generate synthetic down events when the protocol only sends up events.
    (xterm-mouse-last): Remove.
    (xterm-mouse--read-event-sequence-1000): Use xterm-mouse-last-down
    terminal parameter instead.
    (xterm-mouse--set-click-count): New function.
    (xterm-mouse-event): Detect/generate double/triple clicks.
    * lisp/reveal.el (reveal-close-old-overlays): Don't close while
    dragging.


Michael.

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> 44812aa45a823e344811f4f38966c3e5c9b5118f
> Author: Stefan Monnier <monnier@iro.umontreal.ca>
> Date:   Sun May 11 01:49:14 2014 -0400

BTW, evaluating the defun of `mouse-drag-track' from before that commit
seems to fix the problem.

Michael.

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Stefan Monnier]

> Visit dired.el. Double-click on the opening paren of
> (defun dired-get-filename ...
> Only part of the defun is selected.

Hmm... I can't reproduce it here (using "emacs -Q"):
- after opening dired.el I first have to search for "defun dired-get-filename"
  because it's not immediately visible.
- once it's in sight, if I double-click on it, the whole expression is
  selected&highlighted, point is moved to right after the matching close
  paren, and as a consequence the window is redrawn to show the end of
  the expression (since the beginning is now outside of the window).

I never use this feature, but I see the exact same behavior in
Emacs-24.3, so I assume it's the way it's always worked.

Can you reproduce the problem with "emacs -Q"?

> It seems to have been introduced by this commit:
> 44812aa45a823e344811f4f38966c3e5c9b5118f
> Author: Stefan Monnier <monnier@iro.umontreal.ca>
> Date:   Sun May 11 01:49:14 2014 -0400
>     * lisp/mouse.el: Use the normal toplevel loop while dragging.

Yes, it clearly the likely culprit.


        Stefan

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Drew Adams]

> Hmm... I can't reproduce it here (using "emacs -Q"):
> - after opening dired.el I first have to search for "defun dired-get-
> filename"
>   because it's not immediately visible.
> - once it's in sight, if I double-click on it, the whole expression is
>   selected&highlighted, point is moved to right after the matching close
>   paren, and as a consequence the window is redrawn to show the end of
>   the expression (since the beginning is now outside of the window).
> 
> I never use this feature, but I see the exact same behavior in
> Emacs-24.3, so I assume it's the way it's always worked.
> 
> Can you reproduce the problem with "emacs -Q"?

Please read the bug report.  The recipe starts with "emacs -Q".

And read the followup immediately afterward, including the part
about the defun needing to extend outside the window.

In fact, by looking closely you can often see the defun being first
selected correctly and then the region changing to select either
only part of it or more than it.  It seems that something, perhaps
related to window recentering, does not respect keeping the region
as it has been defined.

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Michael Heerdegen]

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

> > Visit dired.el. Double-click on the opening paren of
> > (defun dired-get-filename ...
> > Only part of the defun is selected.
>
> Hmm... I can't reproduce it here (using "emacs -Q"):
> - after opening dired.el I first have to search for "defun
> dired-get-filename"
>   because it's not immediately visible.
> - once it's in sight, if I double-click on it, the whole expression is
>   selected&highlighted, point is moved to right after the matching close
>   paren, and as a consequence the window is redrawn to show the end of
>   the expression (since the beginning is now outside of the window).
>
> I never use this feature, but I see the exact same behavior in
> Emacs-24.3, so I assume it's the way it's always worked.
>
> Can you reproduce the problem with "emacs -Q"?

Yes.  There is no misunderstanding, you obviously don't see what we
see.  Are you sure you tried with trunk, and not emacs-24?

Here's what I found out in the meantime:

In contrast to before the above commit, the second click now leads to a call
to `mouse-set-region'.  I tried to debug-on-entry `mouse-set-region' in
both emacs -Q and tried our recipe:

 - Without your commit, the command is not run (no debugger)
 
 - With your commit, the debugger pops up.  When the debugger pops up,
   the window already has been scrolled, and point is indeed at the end
   of the sexp.  After `mouse-set-region' has finished, point has been
   replaced according to the position under the mouse (after scrolling).

In general, in current trunk, when window scrolling happens, point is
now always positioned in the line under the mouse, instead of after the
sexp.  Seems like something happens similar to what is described in the
old code:

  ;; Don't run the up-event handler if the window
  ;; start changed in a redisplay after the
  ;; mouse-set-point for the down-mouse event at
  ;; the beginning of this function.  When the
  ;; window start has changed, the up-mouse event
  ;; contains a different position due to the new
  ;; window contents, and point is set again.


Michael.

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Stefan Monnier]

> Are you sure you tried with trunk, and not emacs-24?

Duh!  Yes I see it now, sorry for being dense.
Let's see how to fix it, now,


        Stefan

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Stefan Monnier]

> In general, in current trunk, when window scrolling happens, point is
> now always positioned in the line under the mouse, instead of after the
> sexp.  Seems like something happens similar to what is described in the
> old code:

>   ;; Don't run the up-event handler if the window
>   ;; start changed in a redisplay after the
>   ;; mouse-set-point for the down-mouse event at
>   ;; the beginning of this function.  When the
>   ;; window start has changed, the up-mouse event
>   ;; contains a different position due to the new
>   ;; window contents, and point is set again.

Indeed, it's something along these lines, although the above code is not
reached anyway in this kind of "double click on a paren" situation.

The problem is that Emacs generates a drag event even though there's not
been a drag per-se, and the end-posn of the drag is based on the "after
scroll" display, whereas the user usually hasn't had the time to really
see this display before releasing his mouse button.

The old code used a hack to work around the problem, and I installed
another hack that does something morally very similar, although it gets
there in a fairly different way.

In both the old and the new code you can still get the undesired
behavior if you move your mouse slightly between the second down-mouse
and the second up-mouse.


        Stefan

bug#17562: 24.4.50; REGRESSION: mouse double-click on `(` does not select whole Lisp sexp

[Michael Heerdegen]

Hi Stefan,

> The old code used a hack to work around the problem, and I installed
> another hack that does something morally very similar, although it gets
> there in a fairly different way.
>
> In both the old and the new code you can still get the undesired
> behavior if you move your mouse slightly between the second down-mouse
> and the second up-mouse.

For me it works sufficiently well with your hack, thanks.

Michael.