[PATCH 0/2] Move pick to use prefix-arg rather than Escape

[PATCH 0/2] Move pick to use prefix-arg rather than Escape

[Mark Walters]

To try and minimise the impact of pick on the code base some
keybindings which would naturally use a prefix argument were done as a
separate keybinding with "M-". (The reason is that a prefix argument
would have meant modifying the original function whereas the M- could
be bound to a new function in pick)

The first patch is actually all inside pick anyway so can be
changed. The second patch removes a different occurence of the M-
keybinding temporarily: we can add back the prefix-arg version once
pick is in mainline.

I also have some patches to add the prefix-argument (going to pick) to
hello buttons and other search entry points.  (I think this is a good
reason to make the change from M- to prefix-arg as it is much easier
to link the prefix-arg into button actions).

Best wishes

Mark

Mark Walters (2):
  contrib: pick: move M-RET to prefix-arg RET
  contrib: pick: remove temporary keybinding

 contrib/notmuch-pick/notmuch-pick.el |   52 +++++++++++++---------------------
 1 files changed, 20 insertions(+), 32 deletions(-)

-- 
1.7.9.1

[PATCH 1/2] contrib: pick: move M-RET to prefix-arg RET

[Mark Walters]

In pick the user has the option of showing the selected message in a
subpane (the message pane) or in the full frame. This is customisable
using the variable notmuch-pick-show-out. At the moment RET is bound
to the default option and M-RET the other option. This is
messy and involves tricks to make sure the keymap is setup at the
right time.

This changes this to prefix-arg RET for the other option which
simplifies the code and makes things cleaner.
---
 contrib/notmuch-pick/notmuch-pick.el |   51 +++++++++++++--------------------
 1 files changed, 20 insertions(+), 31 deletions(-)

diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
index 054762b..3afdea4 100644
--- a/contrib/notmuch-pick/notmuch-pick.el
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -50,17 +50,10 @@
   "Showing message and thread structure."
   :group 'notmuch)
 
-;; This is ugly. We can't run setup-show-out until it has been defined
-;; which needs the keymap to be defined. So we defer setting up to
-;; notmuch-pick-init.
 (defcustom notmuch-pick-show-out nil
   "View selected messages in new window rather than split-pane."
   :type 'boolean
-  :group 'notmuch-pick
-  :set (lambda (symbol value)
-	 (set-default symbol value)
-	 (when (fboundp 'notmuch-pick-setup-show-out)
-	   (notmuch-pick-setup-show-out))))
+  :group 'notmuch-pick)
 
 (defcustom notmuch-pick-result-format
   `(("date" . "%12s  ")
@@ -241,7 +234,6 @@ FUNC."
     ;; Override because we want to close message pane first.
     (define-key map "m" (notmuch-pick-close-message-pane-and #'notmuch-mua-new-mail))
 
-    (define-key map [mouse-1] 'notmuch-pick-show-message)
     ;; these use notmuch-show functions directly
     (define-key map "|" 'notmuch-show-pipe-message)
     (define-key map "w" 'notmuch-show-save-attachments)
@@ -261,6 +253,8 @@ FUNC."
     (define-key map "V" (notmuch-pick-close-message-pane-and #'notmuch-show-view-raw-message))
 
     ;; The main pick bindings
+    (define-key map (kbd "RET") 'notmuch-pick-show-message)
+    (define-key map [mouse-1] 'notmuch-pick-show-message)
     (define-key map "x" 'notmuch-pick-quit)
     (define-key map "A" 'notmuch-pick-archive-thread)
     (define-key map "a" 'notmuch-pick-archive-message-then-next)
@@ -280,21 +274,6 @@ FUNC."
     map))
 (fset 'notmuch-pick-mode-map notmuch-pick-mode-map)
 
-(defun notmuch-pick-setup-show-out ()
-  "Set up the keymap for showing a thread
-
-This uses the value of the defcustom notmuch-pick-show-out to
-decide whether to show a message in the message pane or in the
-whole window."
-  (let ((map notmuch-pick-mode-map))
-    (if notmuch-pick-show-out
-	(progn
-	  (define-key map (kbd "M-RET") 'notmuch-pick-show-message)
-	  (define-key map (kbd "RET") 'notmuch-pick-show-message-out))
-      (progn
-	(define-key map (kbd "RET") 'notmuch-pick-show-message)
-	(define-key map (kbd "M-RET") 'notmuch-pick-show-message-out)))))
-
 (defun notmuch-pick-get-message-properties ()
   "Return the properties of the current message as a plist.
 
@@ -466,7 +445,7 @@ Does NOT change the database."
       (ignore-errors
 	(delete-window notmuch-pick-message-window)))))
 
-(defun notmuch-pick-show-message ()
+(defun notmuch-pick-show-message-in ()
   "Show the current message (in split-pane)."
   (interactive)
   (let ((id (notmuch-pick-get-message-id))
@@ -503,6 +482,17 @@ Does NOT change the database."
       (notmuch-pick-close-message-window)
       (notmuch-show id nil nil nil))))
 
+(defun notmuch-pick-show-message (arg)
+  "Show the current message.
+
+Shows in split pane or whole window according to value of
+`notmuch-pick-show-out'. A prefix argument reverses the choice."
+  (interactive "P")
+  (if (or (and notmuch-pick-show-out  (not arg))
+	  (and (not notmuch-pick-show-out) arg))
+      (notmuch-pick-show-message-out)
+    (notmuch-pick-show-message-in)))
+
 (defun notmuch-pick-scroll-message-window ()
   "Scroll the message window (if it exists)"
   (interactive)
@@ -566,14 +556,14 @@ message will be \"unarchived\", i.e. the tag changes in
   (interactive)
   (forward-line)
   (when (window-live-p notmuch-pick-message-window)
-    (notmuch-pick-show-message)))
+    (notmuch-pick-show-message-in)))
 
 (defun notmuch-pick-prev-message ()
   "Move to previous message."
   (interactive)
   (forward-line -1)
   (when (window-live-p notmuch-pick-message-window)
-    (notmuch-pick-show-message)))
+    (notmuch-pick-show-message-in)))
 
 (defun notmuch-pick-prev-matching-message ()
   "Move to previous matching message."
@@ -582,7 +572,7 @@ message will be \"unarchived\", i.e. the tag changes in
   (while (and (not (bobp)) (not (notmuch-pick-get-match)))
     (forward-line -1))
   (when (window-live-p notmuch-pick-message-window)
-    (notmuch-pick-show-message)))
+    (notmuch-pick-show-message-in)))
 
 (defun notmuch-pick-next-matching-message ()
   "Move to next matching message."
@@ -591,7 +581,7 @@ message will be \"unarchived\", i.e. the tag changes in
   (while (and (not (eobp)) (not (notmuch-pick-get-match)))
     (forward-line))
   (when (window-live-p notmuch-pick-message-window)
-    (notmuch-pick-show-message)))
+    (notmuch-pick-show-message-in)))
 
 (defun notmuch-pick-refresh-view ()
   "Refresh view."
@@ -763,7 +753,7 @@ unchanged ADDRESS if parsing fails."
       (goto-char (point-max))
       (forward-line -1)
       (when notmuch-pick-open-target
-	(notmuch-pick-show-message)))))
+	(notmuch-pick-show-message-in)))))
 
 (defun notmuch-pick-insert-tree (tree depth tree-status first last)
   "Insert the message tree TREE at depth DEPTH in the current thread.
@@ -952,7 +942,6 @@ The arguments are:
 (define-key notmuch-search-mode-map "Z" 'notmuch-pick-from-search-current-query)
 (define-key notmuch-search-mode-map (kbd "M-RET") 'notmuch-pick-from-search-thread)
 (define-key notmuch-show-mode-map "Z" 'notmuch-pick-from-show-current-query)
-(notmuch-pick-setup-show-out)
 (message "Initialised notmuch-pick")
 
 (provide 'notmuch-pick)
-- 
1.7.9.1

[PATCH 2/2] contrib: pick: remove temporary keybinding

[Mark Walters]

We would like to bind prefix-arg RET in search view to "pick show this
thread" (i.e. notmuch-pick-from-search-thread). It is not easy to do
this cleanly from contrib so I have been using M-RET instead.

Temporarily remove this functionality in preparation for entering
mainline and binding to prefix-arg RET.
---
 contrib/notmuch-pick/notmuch-pick.el |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el
index 3afdea4..0aa651e 100644
--- a/contrib/notmuch-pick/notmuch-pick.el
+++ b/contrib/notmuch-pick/notmuch-pick.el
@@ -940,7 +940,6 @@ The arguments are:
 ;; Set up key bindings from the rest of notmuch.
 (define-key notmuch-common-keymap "z" 'notmuch-pick)
 (define-key notmuch-search-mode-map "Z" 'notmuch-pick-from-search-current-query)
-(define-key notmuch-search-mode-map (kbd "M-RET") 'notmuch-pick-from-search-thread)
 (define-key notmuch-show-mode-map "Z" 'notmuch-pick-from-show-current-query)
 (message "Initialised notmuch-pick")
 
-- 
1.7.9.1

Re: [PATCH 0/2] Move pick to use prefix-arg rather than Escape

[Tomi Ollila]

On Mon, Sep 23 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> To try and minimise the impact of pick on the code base some
> keybindings which would naturally use a prefix argument were done as a
> separate keybinding with "M-". (The reason is that a prefix argument
> would have meant modifying the original function whereas the M- could
> be bound to a new function in pick)
>
> The first patch is actually all inside pick anyway so can be
> changed. The second patch removes a different occurence of the M-
> keybinding temporarily: we can add back the prefix-arg version once
> pick is in mainline.
>
> I also have some patches to add the prefix-argument (going to pick) to
> hello buttons and other search entry points.  (I think this is a good
> reason to make the change from M- to prefix-arg as it is much easier
> to link the prefix-arg into button actions).

LGTM.

Tomi

>
> Best wishes
>
> Mark
>
> Mark Walters (2):
>   contrib: pick: move M-RET to prefix-arg RET
>   contrib: pick: remove temporary keybinding
>
>  contrib/notmuch-pick/notmuch-pick.el |   52 +++++++++++++---------------------
>  1 files changed, 20 insertions(+), 32 deletions(-)
>
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 0/2] Move pick to use prefix-arg rather than Escape

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> To try and minimise the impact of pick on the code base some
> keybindings which would naturally use a prefix argument were done as a
> separate keybinding with "M-". (The reason is that a prefix argument
> would have meant modifying the original function whereas the M- could
> be bound to a new function in pick)

pushed.

d