* [PATCH] emacs: show: make buttons select window
@ 2013-01-07 18:24 Mark Walters
2013-01-07 20:22 ` Austin Clements
0 siblings, 1 reply; 5+ messages in thread
From: Mark Walters @ 2013-01-07 18:24 UTC (permalink / raw)
To: notmuch
Emacs has two button type objects: widgets (as used for saved searches
in notmuch-hello) and buttons as used by parts/citations and id links
in notmuch-show. These two behave subtly differently when clicked with
the mouse: widgets select the window clicked before running the
action, buttons do not.
This patch makes all of these behave the same: clicking always selects
the clicked window. It does this by defining a notmuch-button-type
supertype that the other notmuch buttons can inherit from. This
supertype binds the mouse-action to select the window and then
activate the button.
---
This is a new patch attempting to do the same as
id:1355958602-16752-1-git-send-email-markwalters1009@gmail.com
This version changes all notmuch buttons to select the appropriate
window before applying the action. This brings the buttons in line
with widgets (as used in notmuch hello) and whatever is used for
http:// links.
I think that buttons should at least run the action in the clicked
window: whether point should remain there is less clear. This version
does leave point there as this is what widgets and links do (but this
would be easy to change).
I don't know whether we want to do this for 0.15: the change for id
links would be nice, the other cases are less important. If preferred
I can provide a patch fixing that single instance.
Finally, if anyone who uses the crypto stuff could check that it works
for crypto buttons that would be nice as I do not have crypto setup.
Best wishes
Mark
emacs/notmuch-crypto.el | 3 ++-
emacs/notmuch-lib.el | 5 +++++
emacs/notmuch-show.el | 4 +++-
emacs/notmuch-wash.el | 3 ++-
4 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index 83e5d37..173a3e7 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -76,7 +76,8 @@ mode."
(define-button-type 'notmuch-crypto-status-button-type
'action (lambda (button) (message (button-get button 'help-echo)))
'follow-link t
- 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")
+ 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."
+ :supertype 'notmuch-button-type)
(defun notmuch-crypto-insert-sigstatus-button (sigstatus from)
(let* ((status (plist-get sigstatus :status))
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 0407f8a..1573e32 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -97,6 +97,11 @@ For example, if you wanted to remove an \"inbox\" tag and add an
:group 'notmuch-search
:group 'notmuch-show)
+(define-button-type 'notmuch-button-type
+ 'mouse-action (lambda (button)
+ (select-window (posn-window (event-start last-input-event)))
+ (button-activate button)))
+
(defun notmuch-version ()
"Return a string with the notmuch version number."
(let ((long-string
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5751d98..059194d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -469,7 +469,8 @@ message at DEPTH in the current thread."
'action 'notmuch-show-part-button-default
'keymap 'notmuch-show-part-button-map
'follow-link t
- 'face 'message-mml)
+ 'face 'message-mml
+ :supertype 'notmuch-button-type)
(defvar notmuch-show-part-button-map
(let ((map (make-sparse-keymap)))
@@ -1075,6 +1076,7 @@ buttons for a corresponding notmuch search."
;; Remove the overlay created by goto-address-mode
(remove-overlays (first link) (second link) 'goto-address t)
(make-text-button (first link) (second link)
+ :type 'notmuch-button-type
'action `(lambda (arg)
(notmuch-show ,(third link)))
'follow-link t
diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index d6db4fa..826b6f4 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -115,7 +115,8 @@ lower).")
(define-button-type 'notmuch-wash-button-invisibility-toggle-type
'action 'notmuch-wash-toggle-invisible-action
'follow-link t
- 'face 'font-lock-comment-face)
+ 'face 'font-lock-comment-face
+ :supertype 'notmuch-button-type)
(define-button-type 'notmuch-wash-button-citation-toggle-type
'help-echo "mouse-1, RET: Show citation"
--
1.7.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] emacs: show: make buttons select window
2013-01-07 18:24 [PATCH] emacs: show: make buttons select window Mark Walters
@ 2013-01-07 20:22 ` Austin Clements
2013-01-07 21:07 ` [PATCH v2] " Mark Walters
0 siblings, 1 reply; 5+ messages in thread
From: Austin Clements @ 2013-01-07 20:22 UTC (permalink / raw)
To: Mark Walters; +Cc: notmuch
Quoth Mark Walters on Jan 07 at 6:24 pm:
> Emacs has two button type objects: widgets (as used for saved searches
> in notmuch-hello) and buttons as used by parts/citations and id links
> in notmuch-show. These two behave subtly differently when clicked with
> the mouse: widgets select the window clicked before running the
> action, buttons do not.
>
> This patch makes all of these behave the same: clicking always selects
> the clicked window. It does this by defining a notmuch-button-type
> supertype that the other notmuch buttons can inherit from. This
> supertype binds the mouse-action to select the window and then
> activate the button.
> ---
>
> This is a new patch attempting to do the same as
> id:1355958602-16752-1-git-send-email-markwalters1009@gmail.com
>
> This version changes all notmuch buttons to select the appropriate
> window before applying the action. This brings the buttons in line
> with widgets (as used in notmuch hello) and whatever is used for
> http:// links.
>
> I think that buttons should at least run the action in the clicked
> window: whether point should remain there is less clear. This version
> does leave point there as this is what widgets and links do (but this
> would be easy to change).
>
> I don't know whether we want to do this for 0.15: the change for id
> links would be nice, the other cases are less important. If preferred
> I can provide a patch fixing that single instance.
>
> Finally, if anyone who uses the crypto stuff could check that it works
> for crypto buttons that would be nice as I do not have crypto setup.
>
> Best wishes
>
> Mark
>
>
>
>
> emacs/notmuch-crypto.el | 3 ++-
> emacs/notmuch-lib.el | 5 +++++
> emacs/notmuch-show.el | 4 +++-
> emacs/notmuch-wash.el | 3 ++-
> 4 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
> index 83e5d37..173a3e7 100644
> --- a/emacs/notmuch-crypto.el
> +++ b/emacs/notmuch-crypto.el
> @@ -76,7 +76,8 @@ mode."
> (define-button-type 'notmuch-crypto-status-button-type
> 'action (lambda (button) (message (button-get button 'help-echo)))
> 'follow-link t
> - 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")
> + 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."
> + :supertype 'notmuch-button-type)
This should be 'supertype for consistency.
Also, notmuch-crypto.el should have a (require 'notmuch-lib) at the
top now.
>
> (defun notmuch-crypto-insert-sigstatus-button (sigstatus from)
> (let* ((status (plist-get sigstatus :status))
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 0407f8a..1573e32 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -97,6 +97,11 @@ For example, if you wanted to remove an \"inbox\" tag and add an
> :group 'notmuch-search
> :group 'notmuch-show)
>
> +(define-button-type 'notmuch-button-type
> + 'mouse-action (lambda (button)
> + (select-window (posn-window (event-start last-input-event)))
> + (button-activate button)))
> +
I think this deserves a comment explaining how buttons behave by
default and the problems this causes both for code (funny results of
buffer changes and point placement) and user experience (not moving
point to the clicked window).
> (defun notmuch-version ()
> "Return a string with the notmuch version number."
> (let ((long-string
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 5751d98..059194d 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -469,7 +469,8 @@ message at DEPTH in the current thread."
> 'action 'notmuch-show-part-button-default
> 'keymap 'notmuch-show-part-button-map
> 'follow-link t
> - 'face 'message-mml)
> + 'face 'message-mml
> + :supertype 'notmuch-button-type)
'supertype
>
> (defvar notmuch-show-part-button-map
> (let ((map (make-sparse-keymap)))
> @@ -1075,6 +1076,7 @@ buttons for a corresponding notmuch search."
> ;; Remove the overlay created by goto-address-mode
> (remove-overlays (first link) (second link) 'goto-address t)
> (make-text-button (first link) (second link)
> + :type 'notmuch-button-type
'type
> 'action `(lambda (arg)
> (notmuch-show ,(third link)))
> 'follow-link t
> diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
> index d6db4fa..826b6f4 100644
> --- a/emacs/notmuch-wash.el
> +++ b/emacs/notmuch-wash.el
> @@ -115,7 +115,8 @@ lower).")
> (define-button-type 'notmuch-wash-button-invisibility-toggle-type
> 'action 'notmuch-wash-toggle-invisible-action
> 'follow-link t
> - 'face 'font-lock-comment-face)
> + 'face 'font-lock-comment-face
> + :supertype 'notmuch-button-type)
'supertype
>
> (define-button-type 'notmuch-wash-button-citation-toggle-type
> 'help-echo "mouse-1, RET: Show citation"
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] emacs: show: make buttons select window
2013-01-07 20:22 ` Austin Clements
@ 2013-01-07 21:07 ` Mark Walters
2013-01-07 21:20 ` Austin Clements
2013-02-19 1:12 ` David Bremner
0 siblings, 2 replies; 5+ messages in thread
From: Mark Walters @ 2013-01-07 21:07 UTC (permalink / raw)
To: notmuch
Emacs has two button type objects: widgets (as used for saved searches
in notmuch-hello) and buttons as used by parts/citations and id links
in notmuch-show. These two behave subtly differently when clicked with
the mouse: widgets select the window clicked before running the
action, buttons do not.
This patch makes all of these behave the same: clicking always selects
the clicked window. It does this by defining a notmuch-button-type
supertype that the other notmuch buttons can inherit from. This
supertype binds the mouse-action to select the window and then
activate the button.
---
This versions fixes most of the comments raised by Austin's review.
The one change I didn't make was changing :supertype to 'supertype. In
principle I agree with Austin but the : form is used for inheritance
for other notmuch buttons in wash and crypto.
Best wishes
Mark
emacs/notmuch-crypto.el | 5 ++++-
emacs/notmuch-lib.el | 15 +++++++++++++++
emacs/notmuch-show.el | 4 +++-
emacs/notmuch-wash.el | 3 ++-
4 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index 83e5d37..5233824 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -19,6 +19,8 @@
;;
;; Authors: Jameson Rollins <jrollins@finestructure.net>
+(require 'notmuch-lib)
+
(defcustom notmuch-crypto-process-mime nil
"Should cryptographic MIME parts be processed?
@@ -76,7 +78,8 @@ mode."
(define-button-type 'notmuch-crypto-status-button-type
'action (lambda (button) (message (button-get button 'help-echo)))
'follow-link t
- 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")
+ 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."
+ :supertype 'notmuch-button-type)
(defun notmuch-crypto-insert-sigstatus-button (sigstatus from)
(let* ((status (plist-get sigstatus :status))
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 0407f8a..6836192 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -97,6 +97,21 @@ For example, if you wanted to remove an \"inbox\" tag and add an
:group 'notmuch-search
:group 'notmuch-show)
+;; By default clicking on a button does not select the window
+;; containing the button (as opposed to clicking on a widget which
+;; does). This means that the button action is then executed in the
+;; current selected window which can cause problems if the button
+;; changes the buffer (e.g., id: links) or moves point.
+;;
+;; This provides a button type which overrides mouse-action so that
+;; the button's window is selected before the action is run. Other
+;; notmuch buttons can get the same behaviour by inheriting from this
+;; button type.
+(define-button-type 'notmuch-button-type
+ 'mouse-action (lambda (button)
+ (select-window (posn-window (event-start last-input-event)))
+ (button-activate button)))
+
(defun notmuch-version ()
"Return a string with the notmuch version number."
(let ((long-string
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5751d98..059194d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -469,7 +469,8 @@ message at DEPTH in the current thread."
'action 'notmuch-show-part-button-default
'keymap 'notmuch-show-part-button-map
'follow-link t
- 'face 'message-mml)
+ 'face 'message-mml
+ :supertype 'notmuch-button-type)
(defvar notmuch-show-part-button-map
(let ((map (make-sparse-keymap)))
@@ -1075,6 +1076,7 @@ buttons for a corresponding notmuch search."
;; Remove the overlay created by goto-address-mode
(remove-overlays (first link) (second link) 'goto-address t)
(make-text-button (first link) (second link)
+ :type 'notmuch-button-type
'action `(lambda (arg)
(notmuch-show ,(third link)))
'follow-link t
diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index d6db4fa..826b6f4 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -115,7 +115,8 @@ lower).")
(define-button-type 'notmuch-wash-button-invisibility-toggle-type
'action 'notmuch-wash-toggle-invisible-action
'follow-link t
- 'face 'font-lock-comment-face)
+ 'face 'font-lock-comment-face
+ :supertype 'notmuch-button-type)
(define-button-type 'notmuch-wash-button-citation-toggle-type
'help-echo "mouse-1, RET: Show citation"
--
1.7.9.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] emacs: show: make buttons select window
2013-01-07 21:07 ` [PATCH v2] " Mark Walters
@ 2013-01-07 21:20 ` Austin Clements
2013-02-19 1:12 ` David Bremner
1 sibling, 0 replies; 5+ messages in thread
From: Austin Clements @ 2013-01-07 21:20 UTC (permalink / raw)
To: Mark Walters; +Cc: notmuch
LGTM.
Quoth Mark Walters on Jan 07 at 9:07 pm:
> Emacs has two button type objects: widgets (as used for saved searches
> in notmuch-hello) and buttons as used by parts/citations and id links
> in notmuch-show. These two behave subtly differently when clicked with
> the mouse: widgets select the window clicked before running the
> action, buttons do not.
>
> This patch makes all of these behave the same: clicking always selects
> the clicked window. It does this by defining a notmuch-button-type
> supertype that the other notmuch buttons can inherit from. This
> supertype binds the mouse-action to select the window and then
> activate the button.
> ---
>
> This versions fixes most of the comments raised by Austin's review.
> The one change I didn't make was changing :supertype to 'supertype. In
> principle I agree with Austin but the : form is used for inheritance
> for other notmuch buttons in wash and crypto.
>
> Best wishes
>
> Mark
>
>
>
>
> emacs/notmuch-crypto.el | 5 ++++-
> emacs/notmuch-lib.el | 15 +++++++++++++++
> emacs/notmuch-show.el | 4 +++-
> emacs/notmuch-wash.el | 3 ++-
> 4 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
> index 83e5d37..5233824 100644
> --- a/emacs/notmuch-crypto.el
> +++ b/emacs/notmuch-crypto.el
> @@ -19,6 +19,8 @@
> ;;
> ;; Authors: Jameson Rollins <jrollins@finestructure.net>
>
> +(require 'notmuch-lib)
> +
> (defcustom notmuch-crypto-process-mime nil
> "Should cryptographic MIME parts be processed?
>
> @@ -76,7 +78,8 @@ mode."
> (define-button-type 'notmuch-crypto-status-button-type
> 'action (lambda (button) (message (button-get button 'help-echo)))
> 'follow-link t
> - 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts.")
> + 'help-echo "Set notmuch-crypto-process-mime to process cryptographic mime parts."
> + :supertype 'notmuch-button-type)
>
> (defun notmuch-crypto-insert-sigstatus-button (sigstatus from)
> (let* ((status (plist-get sigstatus :status))
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 0407f8a..6836192 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -97,6 +97,21 @@ For example, if you wanted to remove an \"inbox\" tag and add an
> :group 'notmuch-search
> :group 'notmuch-show)
>
> +;; By default clicking on a button does not select the window
> +;; containing the button (as opposed to clicking on a widget which
> +;; does). This means that the button action is then executed in the
> +;; current selected window which can cause problems if the button
> +;; changes the buffer (e.g., id: links) or moves point.
> +;;
> +;; This provides a button type which overrides mouse-action so that
> +;; the button's window is selected before the action is run. Other
> +;; notmuch buttons can get the same behaviour by inheriting from this
> +;; button type.
> +(define-button-type 'notmuch-button-type
> + 'mouse-action (lambda (button)
> + (select-window (posn-window (event-start last-input-event)))
> + (button-activate button)))
> +
> (defun notmuch-version ()
> "Return a string with the notmuch version number."
> (let ((long-string
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 5751d98..059194d 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -469,7 +469,8 @@ message at DEPTH in the current thread."
> 'action 'notmuch-show-part-button-default
> 'keymap 'notmuch-show-part-button-map
> 'follow-link t
> - 'face 'message-mml)
> + 'face 'message-mml
> + :supertype 'notmuch-button-type)
>
> (defvar notmuch-show-part-button-map
> (let ((map (make-sparse-keymap)))
> @@ -1075,6 +1076,7 @@ buttons for a corresponding notmuch search."
> ;; Remove the overlay created by goto-address-mode
> (remove-overlays (first link) (second link) 'goto-address t)
> (make-text-button (first link) (second link)
> + :type 'notmuch-button-type
> 'action `(lambda (arg)
> (notmuch-show ,(third link)))
> 'follow-link t
> diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
> index d6db4fa..826b6f4 100644
> --- a/emacs/notmuch-wash.el
> +++ b/emacs/notmuch-wash.el
> @@ -115,7 +115,8 @@ lower).")
> (define-button-type 'notmuch-wash-button-invisibility-toggle-type
> 'action 'notmuch-wash-toggle-invisible-action
> 'follow-link t
> - 'face 'font-lock-comment-face)
> + 'face 'font-lock-comment-face
> + :supertype 'notmuch-button-type)
>
> (define-button-type 'notmuch-wash-button-citation-toggle-type
> 'help-echo "mouse-1, RET: Show citation"
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] emacs: show: make buttons select window
2013-01-07 21:07 ` [PATCH v2] " Mark Walters
2013-01-07 21:20 ` Austin Clements
@ 2013-02-19 1:12 ` David Bremner
1 sibling, 0 replies; 5+ messages in thread
From: David Bremner @ 2013-02-19 1:12 UTC (permalink / raw)
To: Mark Walters, notmuch
Mark Walters <markwalters1009@gmail.com> writes:
> Emacs has two button type objects: widgets (as used for saved searches
> in notmuch-hello) and buttons as used by parts/citations and id links
> in notmuch-show. These two behave subtly differently when clicked with
> the mouse: widgets select the window clicked before running the
> action, buttons do not.
pushed,
d
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-02-19 1:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-07 18:24 [PATCH] emacs: show: make buttons select window Mark Walters
2013-01-07 20:22 ` Austin Clements
2013-01-07 21:07 ` [PATCH v2] " Mark Walters
2013-01-07 21:20 ` Austin Clements
2013-02-19 1:12 ` David Bremner
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).