[PATCH] emacs: show: make id links respect window

[PATCH] emacs: show: make id links respect window

[Mark Walters]

There is a bug in current notmuch: if you have multiple windows in one
frame and click an id button link in a show buffer that does not
contain point then the link is opened in the window containing point.

This reads the mouse event to make sure that the correct window is
used for the link.
---

I think this is a bug but that could be debated. It is particularly
easy to trigger with notmuch pick because that uses the split pane
while focus usually remains in the `pick' pane rather than the `show'
pane.

The lisp is not pretty but seems to work.

Best wishes

Mark




 emacs/notmuch-show.el |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5751d98..5664ea3 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1077,6 +1077,11 @@ buttons for a corresponding notmuch search."
 	(make-text-button (first link) (second link)
 			  'action `(lambda (arg)
 				     (notmuch-show ,(third link)))
+			  'mouse-action `(lambda (arg)
+					   (let* ((event last-input-event)
+						  (window (car (cadr event))))
+					     (select-window window)
+					     (notmuch-show ,(third link))))
 			  'follow-link t
 			  'help-echo "Mouse-1, RET: search for this message"
 			  'face goto-address-mail-face)))))
-- 
1.7.9.1

Re: [PATCH] emacs: show: make id links respect window

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> I think this is a bug but that could be debated. It is particularly
> easy to trigger with notmuch pick because that uses the split pane
> while focus usually remains in the `pick' pane rather than the `show'
> pane.

I can imagine that people would want/like the "open in other window" 
effect of the current code, even if the reason is a bug.

>  			  'action `(lambda (arg)
>  				     (notmuch-show ,(third link)))

Can you (or someone) explain why backquote is needed here? Is this some
kind of workaround for lack of lexical scope? If so, maybe a comment.

d

Re: [PATCH] emacs: show: make id links respect window

[Mark Walters]

On Mon, 24 Dec 2012, David Bremner <david@tethera.net> wrote:
> Mark Walters <markwalters1009@gmail.com> writes:
>
>> I think this is a bug but that could be debated. It is particularly
>> easy to trigger with notmuch pick because that uses the split pane
>> while focus usually remains in the `pick' pane rather than the `show'
>> pane.
>
> I can imagine that people would want/like the "open in other window" 
> effect of the current code, even if the reason is a bug.

That's definitely possible. I generally expect a mouse click to select
the window I click and this feels counter intuitive. I think that some
people might like an option "open this link in a new window" but I would
guess that would like that whether they clicked or pressed RET on the
button.

>>  			  'action `(lambda (arg)
>>  				     (notmuch-show ,(third link)))

> Can you (or someone) explain why backquote is needed here? Is this some
> kind of workaround for lack of lexical scope? If so, maybe a comment.

I think this is a reasonably standard use: we want "(third link)"  to be
evaluated when the button is made, but we want the rest of the action to
be evaluated when the action is called. 

Best wishes

Mark

>
> d

Re: [PATCH] emacs: show: make id links respect window

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

>> I can imagine that people would want/like the "open in other window" 
>> effect of the current code, even if the reason is a bug.
>
> That's definitely possible. I generally expect a mouse click to select
> the window I click and this feels counter intuitive. I think that some
> people might like an option "open this link in a new window" but I would
> guess that would like that whether they clicked or pressed RET on the
> button.

I don't care much either way myself, but before we change notmuch-show
behaviour (effectively) to accomodate notmuch-pick, I'd like a bit more
feedback from other people.

d

Re: [PATCH] emacs: show: make id links respect window

[Tomi Ollila]

On Wed, Dec 26 2012, David Bremner <david@tethera.net> wrote:

> Mark Walters <markwalters1009@gmail.com> writes:
>
>>> I can imagine that people would want/like the "open in other window" 
>>> effect of the current code, even if the reason is a bug.
>>
>> That's definitely possible. I generally expect a mouse click to select
>> the window I click and this feels counter intuitive. I think that some
>> people might like an option "open this link in a new window" but I would
>> guess that would like that whether they clicked or pressed RET on the
>> button.
>
> I don't care much either way myself, but before we change notmuch-show
> behaviour (effectively) to accomodate notmuch-pick, I'd like a bit more
> feedback from other people.

I tested the old behaviour -- split frame to 2 windows, one containing
*scratch* and one *notmuch-hello*. Then I searched for 'obsoletes',
chose last message (id:1356936162-2589-1-git-send-email-amdragon@mit.edu)
moved point to *scratch* buffer and clicked the id: link -- and indeed,
the *scratch* window was replaced.

I didn't look or test Mark's patch as he stated:
"The lisp is not pretty but seems to work." ;)
... well, not entirely because of that but I trust it opens the message
in window where the clicked link were and keeps point where it used to
be before clicking (in case point was in different window).

So, personally I'd like to see this fixed but I'm even less qualified
to comment the implementation in the patch.


> d

Tomi

Re: [PATCH] emacs: show: make id links respect window

[Mark Walters]

On Sat, 05 Jan 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Wed, Dec 26 2012, David Bremner <david@tethera.net> wrote:
>
>> Mark Walters <markwalters1009@gmail.com> writes:
>>
>>>> I can imagine that people would want/like the "open in other window" 
>>>> effect of the current code, even if the reason is a bug.
>>>
>>> That's definitely possible. I generally expect a mouse click to select
>>> the window I click and this feels counter intuitive. I think that some
>>> people might like an option "open this link in a new window" but I would
>>> guess that would like that whether they clicked or pressed RET on the
>>> button.
>>
>> I don't care much either way myself, but before we change notmuch-show
>> behaviour (effectively) to accomodate notmuch-pick, I'd like a bit more
>> feedback from other people.
>
> I tested the old behaviour -- split frame to 2 windows, one containing
> *scratch* and one *notmuch-hello*. Then I searched for 'obsoletes',
> chose last message (id:1356936162-2589-1-git-send-email-amdragon@mit.edu)
> moved point to *scratch* buffer and clicked the id: link -- and indeed,
> the *scratch* window was replaced.
>
> I didn't look or test Mark's patch as he stated:
> "The lisp is not pretty but seems to work." ;)
> ... well, not entirely because of that but I trust it opens the message
> in window where the clicked link were and keeps point where it used to
> be before clicking (in case point was in different window).

It doesn't quite do this: point moves to the window that was clicked. It
is just as easy to do as Tomi says (patch below but I should resend if
people like it so the commit message gets picked up).

Incidentally, I would be interested to know what people expect the
following to do: go to notmuch hello and then search. Now display the
results in two windows simultaneously (either split the frame into two
windows (c-x 2) or use 2 frames (C-x 5 2)) and then press q.

What actually happens is that q runs kill buffer so it disappears in
both windows: one of which will fall back to notmuch-hello and one of
which will fall back to some other window (eg scratch)

Best wishes

Mark

 emacs/notmuch-show.el |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5751d98..00b9b56 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1077,6 +1077,11 @@ buttons for a corresponding notmuch search."
 	(make-text-button (first link) (second link)
 			  'action `(lambda (arg)
 				     (notmuch-show ,(third link)))
+			  'mouse-action `(lambda (arg)
+					   (let* ((event last-input-event)
+						  (window (car (cadr event))))
+					     (with-selected-window window
+					       (notmuch-show ,(third link)))))
 			  'follow-link t
 			  'help-echo "Mouse-1, RET: search for this message"
 			  'face goto-address-mail-face)))))
-- 
1.7.9.1

Re: [PATCH] emacs: show: make id links respect window

[Tomi Ollila]

On Sun, Jan 06 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> On Sat, 05 Jan 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>> On Wed, Dec 26 2012, David Bremner <david@tethera.net> wrote:
>>
>>> Mark Walters <markwalters1009@gmail.com> writes:
>>>
>>>>> I can imagine that people would want/like the "open in other window" 
>>>>> effect of the current code, even if the reason is a bug.
>>>>
>>>> That's definitely possible. I generally expect a mouse click to select
>>>> the window I click and this feels counter intuitive. I think that some
>>>> people might like an option "open this link in a new window" but I would
>>>> guess that would like that whether they clicked or pressed RET on the
>>>> button.
>>>
>>> I don't care much either way myself, but before we change notmuch-show
>>> behaviour (effectively) to accomodate notmuch-pick, I'd like a bit more
>>> feedback from other people.
>>
>> I tested the old behaviour -- split frame to 2 windows, one containing
>> *scratch* and one *notmuch-hello*. Then I searched for 'obsoletes',
>> chose last message (id:1356936162-2589-1-git-send-email-amdragon@mit.edu)
>> moved point to *scratch* buffer and clicked the id: link -- and indeed,
>> the *scratch* window was replaced.
>>
>> I didn't look or test Mark's patch as he stated:
>> "The lisp is not pretty but seems to work." ;)
>> ... well, not entirely because of that but I trust it opens the message
>> in window where the clicked link were and keeps point where it used to
>> be before clicking (in case point was in different window).
>
> It doesn't quite do this: point moves to the window that was clicked. It
> is just as easy to do as Tomi says (patch below but I should resend if
> people like it so the commit message gets picked up).

It seems to be the default (and iniuitively expected) behaviour that
point moves to the window that was clicked... so that's ok :)

> Incidentally, I would be interested to know what people expect the
> following to do: go to notmuch hello and then search. Now display the
> results in two windows simultaneously (either split the frame into two
> windows (c-x 2) or use 2 frames (C-x 5 2)) and then press q.
>
> What actually happens is that q runs kill buffer so it disappears in
> both windows: one of which will fall back to notmuch-hello and one of
> which will fall back to some other window (eg scratch)

I think that is just expected behaviour, there are e.g.. c-x b and c-x 0
which should be familiar to every emacs users...

> Best wishes
>
> Mark

Tomi

PS: did you change anything in the patch below ?

>
>  emacs/notmuch-show.el |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 5751d98..00b9b56 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1077,6 +1077,11 @@ buttons for a corresponding notmuch search."
>  	(make-text-button (first link) (second link)
>  			  'action `(lambda (arg)
>  				     (notmuch-show ,(third link)))
> +			  'mouse-action `(lambda (arg)
> +					   (let* ((event last-input-event)
> +						  (window (car (cadr event))))
> +					     (with-selected-window window
> +					       (notmuch-show ,(third link)))))
>  			  'follow-link t
>  			  'help-echo "Mouse-1, RET: search for this message"
>  			  'face goto-address-mail-face)))))
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: show: make id links respect window

[Mark Walters]

On Sun, 06 Jan 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sun, Jan 06 2013, Mark Walters <markwalters1009@gmail.com> wrote:
>
>> On Sat, 05 Jan 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>>> On Wed, Dec 26 2012, David Bremner <david@tethera.net> wrote:
>>>
>>>> Mark Walters <markwalters1009@gmail.com> writes:
>>>>
>>>>>> I can imagine that people would want/like the "open in other window" 
>>>>>> effect of the current code, even if the reason is a bug.
>>>>>
>>>>> That's definitely possible. I generally expect a mouse click to select
>>>>> the window I click and this feels counter intuitive. I think that some
>>>>> people might like an option "open this link in a new window" but I would
>>>>> guess that would like that whether they clicked or pressed RET on the
>>>>> button.
>>>>
>>>> I don't care much either way myself, but before we change notmuch-show
>>>> behaviour (effectively) to accomodate notmuch-pick, I'd like a bit more
>>>> feedback from other people.
>>>
>>> I tested the old behaviour -- split frame to 2 windows, one containing
>>> *scratch* and one *notmuch-hello*. Then I searched for 'obsoletes',
>>> chose last message (id:1356936162-2589-1-git-send-email-amdragon@mit.edu)
>>> moved point to *scratch* buffer and clicked the id: link -- and indeed,
>>> the *scratch* window was replaced.
>>>
>>> I didn't look or test Mark's patch as he stated:
>>> "The lisp is not pretty but seems to work." ;)
>>> ... well, not entirely because of that but I trust it opens the message
>>> in window where the clicked link were and keeps point where it used to
>>> be before clicking (in case point was in different window).
>>
>> It doesn't quite do this: point moves to the window that was clicked. It
>> is just as easy to do as Tomi says (patch below but I should resend if
>> people like it so the commit message gets picked up).
>
> It seems to be the default (and iniuitively expected) behaviour that
> point moves to the window that was clicked... so that's ok :)
>
>> Incidentally, I would be interested to know what people expect the
>> following to do: go to notmuch hello and then search. Now display the
>> results in two windows simultaneously (either split the frame into two
>> windows (c-x 2) or use 2 frames (C-x 5 2)) and then press q.
>>
>> What actually happens is that q runs kill buffer so it disappears in
>> both windows: one of which will fall back to notmuch-hello and one of
>> which will fall back to some other window (eg scratch)
>
> I think that is just expected behaviour, there are e.g.. c-x b and c-x 0
> which should be familiar to every emacs users...

My instinct would be that the window I press q in should go to the
previous buffer in that window but the other window should stay where it
was. But if others are happy as it that is obviously fine.


>> Best wishes
>>
>> Mark
>
> Tomi
>
> PS: did you change anything in the patch below ?

Yes: the new version uses (with-selected-window window ...) compared
with (select-window window) (...)

So the old version switches point to the clicked window, the new version
does not. I am happy either way: the click does normally move point, but
other buttons such as invisibility buttons do not move point when
mouse-clicked so....

Best wishes


Mark


>>
>>  emacs/notmuch-show.el |    5 +++++
>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
>> index 5751d98..00b9b56 100644
>> --- a/emacs/notmuch-show.el
>> +++ b/emacs/notmuch-show.el
>> @@ -1077,6 +1077,11 @@ buttons for a corresponding notmuch search."
>>  	(make-text-button (first link) (second link)
>>  			  'action `(lambda (arg)
>>  				     (notmuch-show ,(third link)))
>> +			  'mouse-action `(lambda (arg)
>> +					   (let* ((event last-input-event)
>> +						  (window (car (cadr event))))
>> +					     (with-selected-window window
>> +					       (notmuch-show ,(third link)))))
>>  			  'follow-link t
>>  			  'help-echo "Mouse-1, RET: search for this message"
>>  			  'face goto-address-mail-face)))))
>> -- 
>> 1.7.9.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: show: make id links respect window

[Austin Clements]

On Wed, 19 Dec 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> There is a bug in current notmuch: if you have multiple windows in one
> frame and click an id button link in a show buffer that does not
> contain point then the link is opened in the window containing point.
>
> This reads the mouse event to make sure that the correct window is
> used for the link.

Personally I prefer that point move to the clicked window, as this patch
does.  The fact that it's even possible for point not to move to the
clicked window I find strange.

> ---
>
> I think this is a bug but that could be debated. It is particularly
> easy to trigger with notmuch pick because that uses the split pane
> while focus usually remains in the `pick' pane rather than the `show'
> pane.
>
> The lisp is not pretty but seems to work.
>
> Best wishes
>
> Mark
>
>
>
>
>  emacs/notmuch-show.el |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 5751d98..5664ea3 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1077,6 +1077,11 @@ buttons for a corresponding notmuch search."
>  	(make-text-button (first link) (second link)
>  			  'action `(lambda (arg)
>  				     (notmuch-show ,(third link)))
> +			  'mouse-action `(lambda (arg)
> +					   (let* ((event last-input-event)
> +						  (window (car (cadr event))))

Better would be (posn-window (event-start event)).  If you use the
accessors, I don't think the let bindings are really necessary since the
code becomes self-documenting.

> +					     (select-window window)
> +					     (notmuch-show ,(third link))))

Would it be better to (button-activate arg) so you don't have to
duplicate the action code?  (Then you also wouldn't need the
quasiquoting and unquoting.)

>  			  'follow-link t
>  			  'help-echo "Mouse-1, RET: search for this message"
>  			  'face goto-address-mail-face)))))
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch