another attempt to add delete functionality in emacs

[Jameson Graef Rollins]

So, after many stabs at adding the ability to "delete" messages in
emacs [0], and the corresponding heated discussions, I'm throwing
another attempt into the fray.

I try to address the concerns that have come up in previous attempts.
In particular, I include a patch that creates a new customization
variable, notmuch-search-exclude-deleted, that will exclude any
messages with the "deleted" tag from searches.  This actually makes
"deleted" messages appear effectively deleted, which is one of the
things cworth wanted to see, and one of the reasons he kept pushing
back on previous attempts at this functionality.

Also, no tags other than "deleted" are modified.  All tags should be
orthogonal, and should be handled so.

Note: this is all about handling the "deleted" tag.  No actual
deletion of message is involved in this functionality at all.  Actual
deletion of messages should always be left entirely up to the user to
handle as they see fit.

jamie.

[0] id:"[hidden email]"
    id:"[hidden email]"
    id:"[hidden email]"
    id:"[hidden email]"

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

The new customization variable, notmuch-search-exclude-deleted, when
set to t, will exclude any messages with the "deleted" tag from
searches.

Additionally, specifying "tag:deleted" in the search directly will
override the exclusion and will included deleted messages in the
search results.
---
 emacs/notmuch.el                                   |    8 ++++
 test/emacs                                         |   42 ++++++++++++++++++++
 .../notmuch-search-tag-inbox-deleted-excluded      |   24 +++++++++++
 3 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs.expected-output/notmuch-search-tag-inbox-deleted-excluded

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index fde2377..c519687 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -905,6 +905,11 @@ PROMPT is the string to prompt with."
       (read-from-minibuffer prompt nil keymap nil
     'notmuch-query-history nil nil))))
 
+(defcustom notmuch-search-exclude-deleted nil
+  "Exclude deleted messages (with \"deleted\" tag) from search results."
+  :group 'notmuch
+  :type 'boolean)
+
 ;;;###autoload
 (defun notmuch-search (query &optional oldest-first target-thread target-line continuation)
   "Run \"notmuch search\" with the given query string and display results.
@@ -927,6 +932,9 @@ The optional parameters are used as follows:
     (set 'notmuch-search-target-thread target-thread)
     (set 'notmuch-search-target-line target-line)
     (set 'notmuch-search-continuation continuation)
+    (when (and notmuch-search-exclude-deleted
+       (not (string-match "tag:deleted[ )]*" query)))
+      (setq query (concat query " and not tag:deleted")))
     (let ((proc (get-buffer-process (current-buffer)))
   (inhibit-read-only t))
       (if proc
diff --git a/test/emacs b/test/emacs
index a06c223..1d78fbe 100755
--- a/test/emacs
+++ b/test/emacs
@@ -35,6 +35,48 @@ test_emacs '(notmuch-search "tag:inbox")
     (test-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-search-tag-inbox
 
+test_begin_subtest "Exclude \"deleted\" messages from search"
+notmuch tag +deleted id:[hidden email]
+# we "delete" a second message that's part of a multi-message thread
+# to make sure the rest of the thread is still returned
+notmuch tag +deleted id:[hidden email]
+test_emacs '(let ((notmuch-search-exclude-deleted t))
+      (notmuch-search "tag:inbox")
+      (notmuch-test-wait)
+      (test-output))'
+notmuch tag -deleted id:[hidden email]
+notmuch tag -deleted id:[hidden email]
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-search-tag-inbox-deleted-excluded
+
+test_begin_subtest "Exclude \"deleted\" messages from search, manual override"
+notmuch tag +deleted id:[hidden email]
+notmuch tag +deleted id:[hidden email]
+cat <<EOF >EXPECTED
+  2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH] Fix linking with gcc to use g++ to link in C++ libs. (deleted inbox unread)
+  2009-11-17 [1/5]   Mikhail Gusarov, Carl Worth, Keith Packard  [notmuch] [PATCH 1/2] Close message file after parsing message headers (deleted inbox unread)
+End of search results.
+EOF
+test_emacs '(let ((notmuch-search-exclude-deleted t))
+      (notmuch-search "tag:inbox and tag:deleted")
+      (notmuch-test-wait)
+      (test-output))'
+notmuch tag -deleted id:[hidden email]
+notmuch tag -deleted id:[hidden email]
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "Exclude \"deleted\" messages from search, but not \"deleted*\""
+notmuch tag +deleted-patch id:[hidden email]
+cat <<EOF >EXPECTED
+  2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH] Fix linking with gcc to use g++ to link in C++ libs. (deleted-patch inbox unread)
+End of search results.
+EOF
+test_emacs '(let ((notmuch-search-exclude-deleted t))
+      (notmuch-search "tag:inbox and tag:deleted-patch")
+      (notmuch-test-wait)
+      (test-output))'
+notmuch tag -deleted-patch id:[hidden email]
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "Navigation of notmuch-hello to search results"
 test_emacs '(notmuch-hello)
     (goto-char (point-min))
diff --git a/test/emacs.expected-output/notmuch-search-tag-inbox-deleted-excluded b/test/emacs.expected-output/notmuch-search-tag-inbox-deleted-excluded
new file mode 100644
index 0000000..39b4c51
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-search-tag-inbox-deleted-excluded
@@ -0,0 +1,24 @@
+  2010-12-29 [1/1]   François Boulogne    [aur-general] Guidelines: cp, mkdir vs install (inbox unread)
+  2010-12-16 [1/1]   Olivier Berger       Essai accentué (inbox unread)
+  2009-11-18 [1/1]   Chris Wilson         [notmuch] [PATCH 1/2] Makefile: evaluate pkg-config once (inbox unread)
+  2009-11-18 [2/2]   Alex Botero-Lowry, Carl Worth  [notmuch] [PATCH] Error out if no query is supplied to search instead of going into an infinite loop (attachment inbox unread)
+  2009-11-18 [2/2]   Ingmar Vanhassel, Carl Worth  [notmuch] [PATCH] Typsos (inbox unread)
+  2009-11-18 [3/3]   Adrian Perez de Castro, Keith Packard, Carl Worth  [notmuch] Introducing myself (inbox signed unread)
+  2009-11-18 [3/3]   Israel Herraiz, Keith Packard, Carl Worth   [notmuch] New to the list (inbox unread)
+  2009-11-18 [3/3]   Jan Janak, Carl Worth        [notmuch] What a great idea! (inbox unread)
+  2009-11-18 [2/2]   Jan Janak, Carl Worth        [notmuch] [PATCH] Older versions of install do not support -C. (inbox unread)
+  2009-11-18 [3/3]   Aron Griffis, Keith Packard, Carl Worth     [notmuch] archive (inbox unread)
+  2009-11-18 [2/2]   Keith Packard, Carl Worth    [notmuch] [PATCH] Make notmuch-show 'X' (and 'x') commands remove inbox (and unread) tags (inbox unread)
+  2009-11-18 [7/7]   Lars Kellogg-Stedman, Mikhail Gusarov, Keith Packard, Carl Worth  [notmuch] Working with Maildir storage? (inbox signed unread)
+  2009-11-18 [4/5]   Mikhail Gusarov, Carl Worth, Keith Packard  [notmuch] [PATCH 1/2] Close message file after parsing message headers (deleted inbox unread)
+  2009-11-18 [2/2]   Keith Packard, Alexander Botero-Lowry    [notmuch] [PATCH] Create a default notmuch-show-hook that highlights URLs and uses word-wrap (inbox unread)
+  2009-11-18 [1/1]   Alexander Botero-Lowry  [notmuch] request for pull (inbox unread)
+  2009-11-18 [4/4]   Jjgod Jiang, Alexander Botero-Lowry      [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)
+  2009-11-18 [1/1]   Rolland Santimano    [notmuch] Link to mailing list archives ? (inbox unread)
+  2009-11-18 [1/1]   Jan Janak            [notmuch] [PATCH] notmuch new: Support for conversion of spool subdirectories into tags (inbox unread)
+  2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH] count_files: sort directory in inode order before statting (inbox unread)
+  2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH 2/2] Read mail directory in inode number order (inbox unread)
+  2009-11-18 [2/2]   Lars Kellogg-Stedman [notmuch] "notmuch help" outputs to stderr? (attachment inbox signed unread)
+  2009-11-17 [1/1]   Mikhail Gusarov      [notmuch] [PATCH] Handle rename of message file (inbox unread)
+  2009-11-17 [2/2]   Alex Botero-Lowry, Carl Worth  [notmuch] preliminary FreeBSD support (attachment inbox unread)
+End of search results.
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

Instead of having a function that is only used for archiving a thread,
we instead make it useful for any tagging operation.  The new
function, notmuch-show-tag-thread-internal, now takes two more
arguments, for the "sign" of the tagging operation ("-" or "+"), and
the tag to be added or removed.  This will allow this function to be
used for any generic thread tagging operation.

The higher level functions that call this function are modified
accordingly.
---
 emacs/notmuch-show.el |   34 ++++++++++++++++++++--------------
 1 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5502efd..1e16f05 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1414,20 +1414,26 @@ argument, hide all of the messages."
   (interactive)
   (backward-button 1))
 
-(defun notmuch-show-archive-thread-internal (show-next)
+(defun notmuch-show-tag-thread-internal (sign tag show-next)
   ;; Remove the tag from the current set of messages.
   (goto-char (point-min))
-  (loop do (notmuch-show-remove-tag "inbox")
- until (not (notmuch-show-goto-message-next)))
-  ;; Move to the next item in the search results, if any.
-  (let ((parent-buffer notmuch-show-parent-buffer))
-    (notmuch-kill-this-buffer)
-    (if parent-buffer
- (progn
-  (switch-to-buffer parent-buffer)
-  (forward-line)
-  (if show-next
-      (notmuch-search-show-thread))))))
+  (let ((tag-function))
+    (cond
+     ((string= sign "-")
+      (setq tag-function 'notmuch-show-remove-tag))
+     ((string= sign "+")
+      (setq tag-function 'notmuch-show-add-tag)))
+    (loop do (funcall tag-function tag)
+  until (not (notmuch-show-goto-message-next)))
+    ;; Move to the next item in the search results, if any.
+    (let ((parent-buffer notmuch-show-parent-buffer))
+      (notmuch-kill-this-buffer)
+      (if parent-buffer
+  (progn
+    (switch-to-buffer parent-buffer)
+    (forward-line)
+    (if show-next
+ (notmuch-search-show-thread)))))))
 
 (defun notmuch-show-archive-thread ()
   "Archive each message in thread, then show next thread from search.
@@ -1441,12 +1447,12 @@ being delivered to the same thread. It does not archive the
 entire thread, but only the messages shown in the current
 buffer."
   (interactive)
-  (notmuch-show-archive-thread-internal t))
+  (notmuch-show-tag-thread-internal "-" "inbox" t))
 
 (defun notmuch-show-archive-thread-then-exit ()
   "Archive each message in thread, then exit back to search results."
   (interactive)
-  (notmuch-show-archive-thread-internal nil))
+  (notmuch-show-tag-thread-internal "-" "inbox" nil))
 
 (defun notmuch-show-stash-cc ()
   "Copy CC field of current message to kill-ring."
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

This completely mimics the behavior of archiving, but instead of
remove the inbox tag it add the "deleted" tag.  The functionality is
bound to the 'd' key in both search and show mode.

Note: this does *not* actually delete any messages.  That is still
left entirely up to the user to figure out how they want to handle
that.  This *just* adds the "deleted" tag to messages, no more.  If
the notmuch-search-exclude-deleted config variable is set, though,
"deleted" messages will be excluded from search results.
---
 emacs/notmuch-show.el |   15 +++++++++++++++
 emacs/notmuch.el      |   12 ++++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 1e16f05..e1d15f4 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -944,6 +944,7 @@ thread id.  If a prefix is given, crypto processing is toggled."
  (define-key map "+" 'notmuch-show-add-tag)
  (define-key map "x" 'notmuch-show-archive-thread-then-exit)
  (define-key map "a" 'notmuch-show-archive-thread)
+ (define-key map "d" 'notmuch-show-delete-thread)
  (define-key map "N" 'notmuch-show-next-message)
  (define-key map "P" 'notmuch-show-previous-message)
  (define-key map "n" 'notmuch-show-next-open-message)
@@ -1454,6 +1455,20 @@ buffer."
   (interactive)
   (notmuch-show-tag-thread-internal "-" "inbox" nil))
 
+(defun notmuch-show-delete-thread ()
+  "Delete each message in thread, then show next thread from search.
+
+Delete each message currently shown by adding the \"deleted\"
+tag to each. Then kill this buffer and show the next thread
+from the search from which this thread was originally shown.
+
+Note: This command is safe from any race condition of new messages
+being delivered to the same thread. It does not archive the
+entire thread, but only the messages shown in the current
+buffer."
+  (interactive)
+  (notmuch-show-tag-thread-internal "+" "deleted" t))
+
 (defun notmuch-show-stash-cc ()
   "Copy CC field of current message to kill-ring."
   (interactive)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index c519687..2b860f4 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -218,6 +218,7 @@ For a mouse binding, return nil."
     (define-key map [mouse-1] 'notmuch-search-show-thread)
     (define-key map "*" 'notmuch-search-operate-all)
     (define-key map "a" 'notmuch-search-archive-thread)
+    (define-key map "d" 'notmuch-search-delete-thread)
     (define-key map "-" 'notmuch-search-remove-tag)
     (define-key map "+" 'notmuch-search-add-tag)
     (define-key map (kbd "RET") 'notmuch-search-show-thread)
@@ -605,6 +606,17 @@ This function advances the next thread when finished."
   (notmuch-search-remove-tag-thread "inbox")
   (forward-line))
 
+(defun notmuch-search-delete-thread ()
+  "Delete the currently selected thread (add the \"deleted\" tag).
+
+This function advances the next thread when finished.
+
+If notmuch-search-exclude-deleted is set, \"deleted\" messages
+will be excluded from searches."
+  (interactive)
+  (notmuch-search-add-tag "deleted")
+  (forward-line))
+
 (defvar notmuch-search-process-filter-data nil
   "Data that has not yet been processed.")
 (make-variable-buffer-local 'notmuch-search-process-filter-data)
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

No functional change here.  The help message previously referred to
the "delete" tag, but "deleted" is now preferred, so hopefully this
will reduce any potential confusion.
---
 emacs/notmuch.el |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 2b860f4..ebdc7d1 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -658,12 +658,12 @@ will be excluded from searches."
 Here is an example of how to color search results based on tags.
  (the following text would be placed in your ~/.emacs file):
 
- (setq notmuch-search-line-faces '((\"delete\" . (:foreground \"red\"
+ (setq notmuch-search-line-faces '((\"deleted\" . (:foreground \"red\"
   :background \"blue\"))
                                    (\"unread\" . (:foreground \"green\"))))
 
 The attributes defined for matching tags are merged, with later
-attributes overriding earlier. A message having both \"delete\"
+attributes overriding earlier. A message having both \"deleted\"
 and \"unread\" tags with the above settings would have a green
 foreground and blue background."
   :type '(alist :key-type (string) :value-type (custom-face-edit))
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

While working on the delete message handling patches, I was reminded
how much I really dislike the default show-mode key bindings.  Why
can't I just archive/delete the current message, without archiving the
entire thread?  It doesn't make any sense.

Here we add two new functions to archive and delete just the single
message, and then move to the next open message.  We also add an
option to the -next-open-message function so that it will pop back out
to the parent search buffer when reaching the end of the thread.  This
should make message processing flow much smoother.

Patches 1,2 and 4 can be applied even if the consensus is to not
change the default key bindings, to make it easier for users to
achieve the desired functionality without having to write their own
functions.

jamie.

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

This adds two new function, notmuch-show-{archive,delete}-message,
that archive/delete the current message, and then move to the next
open one.
---
 emacs/notmuch-show.el |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e1d15f4..8bb052e 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1436,6 +1436,18 @@ argument, hide all of the messages."
     (if show-next
  (notmuch-search-show-thread)))))))
 
+(defun notmuch-show-archive-message ()
+  "Archive the current message and advance.
+
+After the last message is reached, either the buffer will be
+closed and the cursor will move to the search result if
+available, or the cursor will move to the end of the current
+thread.
+"
+    (interactive)
+    (notmuch-show-remove-tag "inbox")
+    (notmuch-show-next-open-message)))
+
 (defun notmuch-show-archive-thread ()
   "Archive each message in thread, then show next thread from search.
 
@@ -1455,6 +1467,18 @@ buffer."
   (interactive)
   (notmuch-show-tag-thread-internal "-" "inbox" nil))
 
+(defun notmuch-show-delete-message ()
+  "Delete the current message and advance.
+
+After the last message is reached, either the buffer will be
+closed and the cursor will move to the search result if
+available, or the cursor will move to the end of the current
+thread.
+"
+    (interactive)
+    (notmuch-show-add-tag "deleted")
+    (notmuch-show-next-open-message)))
+
 (defun notmuch-show-delete-thread ()
   "Delete each message in thread, then show next thread from search.
 
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

This will allow for keybindings that achieve a smoother message
processing flow by reducing the number of key presses needed for most
common operations.
---
 emacs/notmuch-show.el |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 8bb052e..e7bb958 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1264,17 +1264,23 @@ any effects from previous calls to
   (notmuch-show-mark-read)
   (notmuch-show-message-adjust))
 
-(defun notmuch-show-next-open-message ()
+(defun notmuch-show-next-open-message (&optional pop-at-end)
   "Show the next message."
   (interactive)
-  (let (r)
+  (let ((r)
+ (parent-buffer notmuch-show-parent-buffer))
     (while (and (setq r (notmuch-show-goto-message-next))
  (not (notmuch-show-message-visible-p))))
     (if r
  (progn
   (notmuch-show-mark-read)
   (notmuch-show-message-adjust))
-      (goto-char (point-max)))))
+      (if (and parent-buffer pop-at-end)
+  (progn
+    (kill-this-buffer)
+    (switch-to-buffer parent-buffer)
+    (forward-line 1))
+ (goto-char (point-max))))))
 
 (defun notmuch-show-previous-open-message ()
   "Show the previous message."
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

This changes the default key bindings for the 'a' and 'd' keys in
notmuch-show mode.  Instead of archiving/deleting the entire thread,
they now just archive/delete the current message, and then advance to
the next open message.

'A' and 'D' are rebound to the previous archive/delete-thread
functions.
---
 emacs/notmuch-show.el |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e7bb958..4c2b507 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -943,8 +943,10 @@ thread id.  If a prefix is given, crypto processing is toggled."
  (define-key map "-" 'notmuch-show-remove-tag)
  (define-key map "+" 'notmuch-show-add-tag)
  (define-key map "x" 'notmuch-show-archive-thread-then-exit)
- (define-key map "a" 'notmuch-show-archive-thread)
- (define-key map "d" 'notmuch-show-delete-thread)
+ (define-key map "a" 'notmuch-show-archive-message)
+ (define-key map "A" 'notmuch-show-archive-thread)
+ (define-key map "d" 'notmuch-show-delete-message)
+ (define-key map "D" 'notmuch-show-delete-thread)
  (define-key map "N" 'notmuch-show-next-message)
  (define-key map "P" 'notmuch-show-previous-message)
  (define-key map "n" 'notmuch-show-next-open-message)
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

This provides a smoother message processing flow by reducing the
number of key presses needed for these common operations.
---
 emacs/notmuch-show.el |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 4c2b507..7103c23 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1454,7 +1454,7 @@ thread.
 "
     (interactive)
     (notmuch-show-remove-tag "inbox")
-    (notmuch-show-next-open-message)))
+    (notmuch-show-next-open-message t)))
 
 (defun notmuch-show-archive-thread ()
   "Archive each message in thread, then show next thread from search.
@@ -1485,7 +1485,7 @@ thread.
 "
     (interactive)
     (notmuch-show-add-tag "deleted")
-    (notmuch-show-next-open-message)))
+    (notmuch-show-next-open-message t)))
 
 (defun notmuch-show-delete-thread ()
   "Delete each message in thread, then show next thread from search.
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

This provides a smoother message processing flow by reducing the
number of key presses needed for these common operations.
---
Sorry, there were some errant extra parens at the end of these
function definitions.

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

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 4c2b507..a706637 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1454,7 +1454,7 @@ thread.
 "
     (interactive)
     (notmuch-show-remove-tag "inbox")
-    (notmuch-show-next-open-message)))
+    (notmuch-show-next-open-message t))
 
 (defun notmuch-show-archive-thread ()
   "Archive each message in thread, then show next thread from search.
@@ -1485,7 +1485,7 @@ thread.
 "
     (interactive)
     (notmuch-show-add-tag "deleted")
-    (notmuch-show-next-open-message)))
+    (notmuch-show-next-open-message t))
 
 (defun notmuch-show-delete-thread ()
   "Delete each message in thread, then show next thread from search.
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

This adds two new function, notmuch-show-{archive,delete}-message,
that archive/delete the current message, and then move to the next
open one.
---
Sorry, there were some errant extra parens at the end of these
function definitions.

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

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e1d15f4..b2e7829 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1436,6 +1436,18 @@ argument, hide all of the messages."
     (if show-next
  (notmuch-search-show-thread)))))))
 
+(defun notmuch-show-archive-message ()
+  "Archive the current message and advance.
+
+After the last message is reached, either the buffer will be
+closed and the cursor will move to the search result if
+available, or the cursor will move to the end of the current
+thread.
+"
+    (interactive)
+    (notmuch-show-remove-tag "inbox")
+    (notmuch-show-next-open-message))
+
 (defun notmuch-show-archive-thread ()
   "Archive each message in thread, then show next thread from search.
 
@@ -1455,6 +1467,18 @@ buffer."
   (interactive)
   (notmuch-show-tag-thread-internal "-" "inbox" nil))
 
+(defun notmuch-show-delete-message ()
+  "Delete the current message and advance.
+
+After the last message is reached, either the buffer will be
+closed and the cursor will move to the search result if
+available, or the cursor will move to the end of the current
+thread.
+"
+    (interactive)
+    (notmuch-show-add-tag "deleted")
+    (notmuch-show-next-open-message))
+
 (defun notmuch-show-delete-thread ()
   "Delete each message in thread, then show next thread from search.
 
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

This provides a smoother message processing flow by reducing the
number of key presses needed for these common operations.
---
Sorry sorry.  I originally missed the problem in patch 1/4, so I had
to modify this patch again to apply against the new version of the
previous.  So sorry for the noise, and thanks to amdragon for pointing
all of this out.

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

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index b9ec584..a706637 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1454,7 +1454,7 @@ thread.
 "
     (interactive)
     (notmuch-show-remove-tag "inbox")
-    (notmuch-show-next-open-message))
+    (notmuch-show-next-open-message t))
 
 (defun notmuch-show-archive-thread ()
   "Archive each message in thread, then show next thread from search.
@@ -1485,7 +1485,7 @@ thread.
 "
     (interactive)
     (notmuch-show-add-tag "deleted")
-    (notmuch-show-next-open-message))
+    (notmuch-show-next-open-message t))
 
 (defun notmuch-show-delete-thread ()
   "Delete each message in thread, then show next thread from search.
--
1.7.7.3

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Aaron Ecay]

Jameson,

Some comments below:

On Sat,  7 Jan 2012 14:28:12 -0800, Jameson Graef Rollins <[hidden email]> wrote:

> Instead of having a function that is only used for archiving a thread,
> we instead make it useful for any tagging operation.  The new
> function, notmuch-show-tag-thread-internal, now takes two more
> arguments, for the "sign" of the tagging operation ("-" or "+"), and
> the tag to be added or removed.  This will allow this function to be
> used for any generic thread tagging operation.
>
> The higher level functions that call this function are modified
> accordingly.
> ---
>  emacs/notmuch-show.el |   34 ++++++++++++++++++++--------------
>  1 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 5502efd..1e16f05 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1414,20 +1414,26 @@ argument, hide all of the messages."
>    (interactive)
>    (backward-button 1))
>  
> -(defun notmuch-show-archive-thread-internal (show-next)
> +(defun notmuch-show-tag-thread-internal (sign tag show-next)

A couple of comments on the arguments:
- It would be good to make show-next &optional.  This will enable code
  to call the fn with only two arguments, and not showing next will be
  the default behavior.
- A more lispy way of specifying the sign would be to use a
  boolean.  Perhaps you could call this “remove”; a value of ‘t’ would
  remove the tag; ‘nil’ would add it.  Moving this argument after ‘tag’
  and also making it &optional woudl allow this fn to be called with one
  arg to add a tag.  (Maybe this is too minimalist and API, however.)

>    ;; Remove the tag from the current set of messages.
>    (goto-char (point-min))
> -  (loop do (notmuch-show-remove-tag "inbox")
> - until (not (notmuch-show-goto-message-next)))
> -  ;; Move to the next item in the search results, if any.
> -  (let ((parent-buffer notmuch-show-parent-buffer))
> -    (notmuch-kill-this-buffer)
> -    (if parent-buffer
> - (progn
> -  (switch-to-buffer parent-buffer)
> -  (forward-line)
> -  (if show-next
> -      (notmuch-search-show-thread))))))
> +  (let ((tag-function))

No second set of parens is needed around tag-function.

> +    (cond
> +     ((string= sign "-")
> +      (setq tag-function 'notmuch-show-remove-tag))
> +     ((string= sign "+")
> +      (setq tag-function 'notmuch-show-add-tag)))
> +    (loop do (funcall tag-function tag)
> +  until (not (notmuch-show-goto-message-next)))
> +    ;; Move to the next item in the search results, if any.

Does it make sense to separate the tagging and the movement?  It seems
plausible that some code somewhere might want to add/remove a tag from
all messages in the thread w/o changing the display.

> +    (let ((parent-buffer notmuch-show-parent-buffer))
> +      (notmuch-kill-this-buffer)
> +      (if parent-buffer
> +  (progn
> +    (switch-to-buffer parent-buffer)
> +    (forward-line)
> +    (if show-next
> + (notmuch-search-show-thread)))))))

--
Aaron Ecay
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Aaron Ecay]

Jameson,

One small, stylistic/nitpicky comment :)

On Sat,  7 Jan 2012 17:26:53 -0800, Jameson Graef Rollins <[hidden email]> wrote:

> This will allow for keybindings that achieve a smoother message
> processing flow by reducing the number of key presses needed for most
> common operations.
> ---
>  emacs/notmuch-show.el |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 8bb052e..e7bb958 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1264,17 +1264,23 @@ any effects from previous calls to
>    (notmuch-show-mark-read)
>    (notmuch-show-message-adjust))
>  
> -(defun notmuch-show-next-open-message ()
> +(defun notmuch-show-next-open-message (&optional pop-at-end)
>    "Show the next message."
>    (interactive)
> -  (let (r)
> +  (let ((r)
> + (parent-buffer notmuch-show-parent-buffer))

No second set of parentheses is needed around r.  Also, it is more
idiomatic to put the initialized variable (i.e. parent-buffer) before
the uninitialized one (r).

>      (while (and (setq r (notmuch-show-goto-message-next))
>   (not (notmuch-show-message-visible-p))))
>      (if r
>   (progn
>    (notmuch-show-mark-read)
>    (notmuch-show-message-adjust))
> -      (goto-char (point-max)))))
> +      (if (and parent-buffer pop-at-end)
> +  (progn
> +    (kill-this-buffer)
> +    (switch-to-buffer parent-buffer)
> +    (forward-line 1))
> + (goto-char (point-max))))))
>  
>  (defun notmuch-show-previous-open-message ()
>    "Show the previous message."
> --
> 1.7.7.3
>
> _______________________________________________
> notmuch mailing list
> [hidden email]
> http://notmuchmail.org/mailman/listinfo/notmuch

--
Aaron Ecay
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Aaron Ecay]

Jameson,

One comment

On Sat,  7 Jan 2012 14:28:11 -0800, Jameson Graef Rollins <[hidden email]> wrote:

> The new customization variable, notmuch-search-exclude-deleted, when
> set to t, will exclude any messages with the "deleted" tag from
> searches.
>
> Additionally, specifying "tag:deleted" in the search directly will
> override the exclusion and will included deleted messages in the
> search results.
> ---
>  emacs/notmuch.el                                   |    8 ++++
>  test/emacs                                         |   42 ++++++++++++++++++++
>  .../notmuch-search-tag-inbox-deleted-excluded      |   24 +++++++++++
>  3 files changed, 74 insertions(+), 0 deletions(-)
>  create mode 100644 test/emacs.expected-output/notmuch-search-tag-inbox-deleted-excluded
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index fde2377..c519687 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -905,6 +905,11 @@ PROMPT is the string to prompt with."
>        (read-from-minibuffer prompt nil keymap nil
>      'notmuch-query-history nil nil))))
>  
> +(defcustom notmuch-search-exclude-deleted nil
> +  "Exclude deleted messages (with \"deleted\" tag) from search results."
> +  :group 'notmuch
> +  :type 'boolean)
> +
>  ;;;###autoload
>  (defun notmuch-search (query &optional oldest-first target-thread target-line continuation)
>    "Run \"notmuch search\" with the given query string and display results.
> @@ -927,6 +932,9 @@ The optional parameters are used as follows:
>      (set 'notmuch-search-target-thread target-thread)
>      (set 'notmuch-search-target-line target-line)
>      (set 'notmuch-search-continuation continuation)
> +    (when (and notmuch-search-exclude-deleted
> +       (not (string-match "tag:deleted[ )]*" query)))

“is:” is a synonym for “tag:” in searches – so this section of the code
should look for it too.

--
Aaron Ecay
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Austin Clements]

> > @@ -927,6 +932,9 @@ The optional parameters are used as follows:
> >      (set 'notmuch-search-target-thread target-thread)
> >      (set 'notmuch-search-target-line target-line)
> >      (set 'notmuch-search-continuation continuation)
> > +    (when (and notmuch-search-exclude-deleted
> > +       (not (string-match "tag:deleted[ )]*" query)))
>
> “is:” is a synonym for “tag:” in searches – so this section of the code
> should look for it too.

There are several other things that could also trip up this regexp.
xtag:deletedx would be falsely matched, as would a quoted phrase
containing "tag:deleted", while tag:"deleted" and tag:(deleted) would
incorrectly not be matched.  Getting this right is hard, though I'd be
happy with

  "\\<\\(tag\\|is\\):deleted\\>"

or maybe

  "\\<\\(tag\\|is\\):\\(\"?\\)deleted\\>\\2"

Implicit exclusions like this were actually one of my target features
for the custom query parser, but I think hacking around that by
inspecting the query string is a fine interim solution.  (One of these
months I'll dust off the query parser, really!)
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

On Sun, 8 Jan 2012 20:49:38 -0500, Austin Clements <[hidden email]> wrote:

> > > @@ -927,6 +932,9 @@ The optional parameters are used as follows:
> > >      (set 'notmuch-search-target-thread target-thread)
> > >      (set 'notmuch-search-target-line target-line)
> > >      (set 'notmuch-search-continuation continuation)
> > > +    (when (and notmuch-search-exclude-deleted
> > > +       (not (string-match "tag:deleted[ )]*" query)))
> >
> > “is:” is a synonym for “tag:” in searches – so this section of the code
> > should look for it too.
>
> There are several other things that could also trip up this regexp.
> xtag:deletedx would be falsely matched, as would a quoted phrase
> containing "tag:deleted", while tag:"deleted" and tag:(deleted) would
> incorrectly not be matched.

Thanks so much for the review, guys.  I should have mentioned in this
patch that the my regex skills are very weak, and that it was surely
incomplete.  I always forget about the is: prefix as well.

> Getting this right is hard, though I'd be happy with
>
>   "\\<\\(tag\\|is\\):deleted\\>"

Every time I think I start to understand regex I am reminded that it's
black magic and I really know nothing.  For instance, I am not familiar
with "<" or ">", although they appear to be a "word" boundaries
(although I'm not sure how "word" is defined).  Also, why is all the \\
(double?)  escaping needed?  I'll certainly take your word for it,
though.

> or maybe
>
>   "\\<\\(tag\\|is\\):\\(\"?\\)deleted\\>\\2"

After staring at this for 10 minutes I think I'm getting the extra bits
here.  It matches an initial \", and then a second at the end if the
first matched.  That's clever.  Why

  \\>\\2

instead of

 \\2\\>

?

I'm definitely confused by why so much apparent escaping is needed,
though.

> Implicit exclusions like this were actually one of my target features
> for the custom query parser, but I think hacking around that by
> inspecting the query string is a fine interim solution.  (One of these
> months I'll dust off the query parser, really!)

Very much looking forward to it!

jamie.

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Austin Clements]

Quoth Jameson Graef Rollins on Jan 08 at  6:34 pm:

> On Sun, 8 Jan 2012 20:49:38 -0500, Austin Clements <[hidden email]> wrote:
> > > > @@ -927,6 +932,9 @@ The optional parameters are used as follows:
> > > >      (set 'notmuch-search-target-thread target-thread)
> > > >      (set 'notmuch-search-target-line target-line)
> > > >      (set 'notmuch-search-continuation continuation)
> > > > +    (when (and notmuch-search-exclude-deleted
> > > > +       (not (string-match "tag:deleted[ )]*" query)))
> > >
> > > “is:” is a synonym for “tag:” in searches – so this section of the code
> > > should look for it too.
> >
> > There are several other things that could also trip up this regexp.
> > xtag:deletedx would be falsely matched, as would a quoted phrase
> > containing "tag:deleted", while tag:"deleted" and tag:(deleted) would
> > incorrectly not be matched.
>
> Thanks so much for the review, guys.  I should have mentioned in this
> patch that the my regex skills are very weak, and that it was surely
> incomplete.  I always forget about the is: prefix as well.
>
> > Getting this right is hard, though I'd be happy with
> >
> >   "\\<\\(tag\\|is\\):deleted\\>"
>
> Every time I think I start to understand regex I am reminded that it's
> black magic and I really know nothing.  For instance, I am not familiar
> with "<" or ">", although they appear to be a "word" boundaries
> (although I'm not sure how "word" is defined).  Also, why is all the \\
> (double?)  escaping needed?  I'll certainly take your word for it,
> though.

I'm not positive, but I think \> matches on the transition from a
"word-constituent" character to a non-word-constituent character, as
defined by Emacs' active syntax table.

The slashes are all doubled because I was writing it as an Emacs
string for easy pasting (sorry, I should have been explicit about
that).  The regexp itself is

  \<\(tag\|is\):deleted\>

> > or maybe
> >
> >   "\\<\\(tag\\|is\\):\\(\"?\\)deleted\\>\\2"
>
> After staring at this for 10 minutes I think I'm getting the extra bits
> here.  It matches an initial \", and then a second at the end if the
> first matched.  That's clever.  Why

Exactly.

>   \\>\\2
>
> instead of
>
>  \\2\\>
>
> ?

Okay, that can qualify as black magic.  The problem is that a " will
mess up the word-boundary matching because " isn't a word constituent
character.  So, if it is looking for a quote at the end, the \2 in
\2\> would match and consume the ", but then the \> wouldn't match.
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jameson Graef Rollins]

Thanks so much for the review, Aaron.

On Sun, 08 Jan 2012 20:08:59 -0500, Aaron Ecay <[hidden email]> wrote:
> A couple of comments on the arguments:
> - It would be good to make show-next &optional.  This will enable code
>   to call the fn with only two arguments, and not showing next will be
>   the default behavior.

That's a nice idea.  Probably better for a separate patch, though.

> - A more lispy way of specifying the sign would be to use a
>   boolean.  Perhaps you could call this “remove”; a value of ‘t’ would
>   remove the tag; ‘nil’ would add it.  Moving this argument after ‘tag’
>   and also making it &optional woudl allow this fn to be called with one
>   arg to add a tag.  (Maybe this is too minimalist and API, however.)

That might be more lispy, but it seems a lot less clear to me.  It might
save a few keystrokes when coding, but it would definitely make the code
a lot harder to read ("remove" to add a tag?).  I think I would prefer
people to give the sign explicitly.

> No second set of parens is needed around tag-function.

Yeah, I've seen this either way.  I guess it's just a stylistic choice.

I think it might make sense, but again I think that's out of the scope
of this patch series.  The point was to make a minimal set of
modifications here.  If we want to separate out the functionality, we
should do that in a separate patch.

Thanks again for the review.

jamie.

_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch