[PATCH 1/6] emacs: move tag format validation to `notmuch-tag' function

[Dmitry Kurochkin]

Before the change, tag format validation was done in
`notmuch-search-operate-all' function only.  The patch moves it down
to `notmuch-tag', so that all users of that function get input
validation.
---
 emacs/notmuch.el |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 72f78ed..84d7d0a 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -522,6 +522,12 @@ Note: Other code should always use this function alter tags of
 messages instead of running (notmuch-call-notmuch-process \"tag\" ..)
 directly, so that hooks specified in notmuch-before-tag-hook and
 notmuch-after-tag-hook will be run."
+  ;; Perform some validation
+  (when (null tags) (error "No tags given"))
+  (mapc (lambda (tag)
+  (unless (string-match-p "^[-+][-+_.[:word:]]+$" tag)
+    (error "Tag must be of the form `+this_tag' or `-that_tag'")))
+ tags)
   (run-hooks 'notmuch-before-tag-hook)
   (apply 'notmuch-call-notmuch-process
  (append (list "tag") tags (list "--" query)))
@@ -890,12 +896,6 @@ characters as well as `_.+-'.
   (interactive (notmuch-select-tags-with-completion
  "Operations (+add -drop): notmuch tag "
  '("+" "-")))
-  ;; Perform some validation
-  (when (null actions) (error "No operations given"))
-  (mapc (lambda (action)
-  (unless (string-match-p "^[-+][-+_.[:word:]]+$" action)
-    (error "Action must be of the form `+this_tag' or `-that_tag'")))
- actions)
   (apply 'notmuch-tag notmuch-search-query-string actions))
 
 (defun notmuch-search-buffer-title (query)
--
1.7.8.3

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

[Dmitry Kurochkin]

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

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 84d7d0a..ff46617 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -577,7 +577,7 @@ the messages that were tagged"
     (let ((beg (+ (point) 1)))
       (re-search-forward ")")
       (let ((end (- (point) 1)))
- (split-string (buffer-substring beg end))))))
+ (split-string (buffer-substring-no-properties beg end))))))
 
 (defun notmuch-search-get-tags-region (beg end)
   (save-excursion
--
1.7.8.3

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

[Dmitry Kurochkin]

Before the change, "+" and "-" tagging operations in notmuch-search
and notmuch-show views accepted only a single tag.  The patch makes
them use the recently added `notmuch-select-tags-with-completion'
function, which allows to enter multiple tags with "+" and "-"
prefixes.  So after the change, "+" and "-" bindings allow to both add
and remove multiple tags.  The only difference between "+" and "-" is
the minibuffer initial input ("+" and "-" respectively).
---
 emacs/notmuch-show.el |   65 +++++++------------
 emacs/notmuch.el      |  165 +++++++++++++++++++++++++------------------------
 2 files changed, 107 insertions(+), 123 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 84ac624..03eadfb 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -38,8 +38,9 @@
 
 (declare-function notmuch-call-notmuch-process "notmuch" (&rest args))
 (declare-function notmuch-fontify-headers "notmuch" nil)
-(declare-function notmuch-select-tag-with-completion "notmuch" (prompt &rest search-terms))
+(declare-function notmuch-select-tags-with-completion "notmuch" (&optional initial-input &rest search-terms))
 (declare-function notmuch-search-show-thread "notmuch" nil)
+(declare-function notmuch-update-tags "notmuch" (current-tags changed-tags))
 
 (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
   "Headers that should be shown in a message, in this order.
@@ -1267,7 +1268,7 @@ Some useful entries are:
 
 (defun notmuch-show-mark-read ()
   "Mark the current message as read."
-  (notmuch-show-remove-tag "unread"))
+  (notmuch-show-tag-message "-unread"))
 
 ;; Functions for getting attributes of several messages in the current
 ;; thread.
@@ -1470,51 +1471,33 @@ than only the current message."
     (message (format "Command '%s' exited abnormally with code %d"
      shell-command exit-code))))))))
 
-(defun notmuch-show-add-tags-worker (current-tags add-tags)
-  "Add to `current-tags' with any tags from `add-tags' not
-currently present and return the result."
-  (let ((result-tags (copy-sequence current-tags)))
-    (mapc (lambda (add-tag)
-    (unless (member add-tag current-tags)
-      (setq result-tags (push add-tag result-tags))))
-    add-tags)
-    (sort result-tags 'string<)))
-
-(defun notmuch-show-del-tags-worker (current-tags del-tags)
-  "Remove any tags in `del-tags' from `current-tags' and return
-the result."
-  (let ((result-tags (copy-sequence current-tags)))
-    (mapc (lambda (del-tag)
-    (setq result-tags (delete del-tag result-tags)))
-  del-tags)
-    result-tags))
-
-(defun notmuch-show-add-tag (&rest toadd)
-  "Add a tag to the current message."
-  (interactive
-   (list (notmuch-select-tag-with-completion "Tag to add: ")))
+(defun notmuch-show-tag-message (&rest changed-tags)
+  "Change tags for the current message.
 
+`Changed-tags' is a list of tag operations for \"notmuch tag\",
+i.e. a list of tags to change with '+' and '-' prefixes."
   (let* ((current-tags (notmuch-show-get-tags))
- (new-tags (notmuch-show-add-tags-worker current-tags toadd)))
-
+ (new-tags (notmuch-update-tags current-tags changed-tags)))
     (unless (equal current-tags new-tags)
-      (apply 'notmuch-tag (notmuch-show-get-message-id)
-     (mapcar (lambda (s) (concat "+" s)) toadd))
+      (apply 'notmuch-tag (notmuch-show-get-message-id) changed-tags)
       (notmuch-show-set-tags new-tags))))
 
-(defun notmuch-show-remove-tag (&rest toremove)
-  "Remove a tag from the current message."
-  (interactive
-   (list (notmuch-select-tag-with-completion
-  "Tag to remove: " (notmuch-show-get-message-id))))
+(defun notmuch-show-tag (&optional initial-input)
+  "Change tags for the current message, read input from the minibuffer."
+  (interactive)
+  (let ((changed-tags (notmuch-select-tags-with-completion
+       initial-input (notmuch-show-get-message-id))))
+    (apply 'notmuch-show-tag-message changed-tags)))
 
-  (let* ((current-tags (notmuch-show-get-tags))
- (new-tags (notmuch-show-del-tags-worker current-tags toremove)))
+(defun notmuch-show-add-tag ()
+  "Same as `notmuch-show-tag' but sets initial input to '+'."
+  (interactive)
+  (notmuch-show-tag "+"))
 
-    (unless (equal current-tags new-tags)
-      (apply 'notmuch-tag (notmuch-show-get-message-id)
-     (mapcar (lambda (s) (concat "-" s)) toremove))
-      (notmuch-show-set-tags new-tags))))
+(defun notmuch-show-remove-tag ()
+  "Same as `notmuch-show-tag' but sets initial input to '-'."
+  (interactive)
+  (notmuch-show-tag "-"))
 
 (defun notmuch-show-toggle-headers ()
   "Toggle the visibility of the current message headers."
@@ -1559,7 +1542,7 @@ argument, hide all of the messages."
 (defun notmuch-show-archive-thread-internal (show-next)
   ;; Remove the tag from the current set of messages.
   (goto-char (point-min))
-  (loop do (notmuch-show-remove-tag "inbox")
+  (loop do (notmuch-show-tag-message "-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))
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index ff46617..24b0ea3 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -76,38 +76,56 @@ For example:
 (defvar notmuch-query-history nil
   "Variable to store minibuffer history for notmuch queries")
 
-(defun notmuch-tag-completions (&optional prefixes search-terms)
-  (let ((tag-list
- (split-string
-  (with-output-to-string
-    (with-current-buffer standard-output
-      (apply 'call-process notmuch-command nil t
-     nil "search-tags" search-terms)))
-  "\n+" t)))
-    (if (null prefixes)
- tag-list
-      (apply #'append
-     (mapcar (lambda (tag)
-       (mapcar (lambda (prefix)
- (concat prefix tag)) prefixes))
-     tag-list)))))
+(defun notmuch-tag-completions (&optional search-terms)
+  (split-string
+   (with-output-to-string
+     (with-current-buffer standard-output
+       (apply 'call-process notmuch-command nil t
+      nil "search-tags" search-terms)))
+   "\n+" t))
 
 (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
-  (let ((tag-list (notmuch-tag-completions nil search-terms)))
+  (let ((tag-list (notmuch-tag-completions search-terms)))
     (completing-read prompt tag-list)))
 
-(defun notmuch-select-tags-with-completion (prompt &optional prefixes &rest search-terms)
-  (let ((tag-list (notmuch-tag-completions prefixes search-terms))
- (crm-separator " ")
- ;; By default, space is bound to "complete word" function.
- ;; Re-bind it to insert a space instead.  Note that <tab>
- ;; still does the completion.
- (crm-local-completion-map
- (let ((map (make-sparse-keymap)))
-   (set-keymap-parent map crm-local-completion-map)
-   (define-key map " " 'self-insert-command)
-   map)))
-    (delete "" (completing-read-multiple prompt tag-list))))
+(defun notmuch-select-tags-with-completion (&optional initial-input &rest search-terms)
+  (let* ((add-tag-list (mapcar (apply-partially 'concat "+")
+       (notmuch-tag-completions)))
+ (remove-tag-list (mapcar (apply-partially 'concat "-")
+  (notmuch-tag-completions search-terms)))
+ (tag-list (append add-tag-list remove-tag-list))
+ (crm-separator " ")
+ ;; By default, space is bound to "complete word" function.
+ ;; Re-bind it to insert a space instead.  Note that <tab>
+ ;; still does the completion.
+ (crm-local-completion-map
+  (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map crm-local-completion-map)
+    (define-key map " " 'self-insert-command)
+    map)))
+    (delete "" (completing-read-multiple
+ "Operations (+add -drop): notmuch tag " tag-list nil
+ nil initial-input))))
+
+(defun notmuch-update-tags (current-tags changed-tags)
+  "Update `current-tags' with `changed-tags' and return the result.
+
+`Changed-tags' is a list of tag operations given to \"notmuch
+tag\", i.e. a list of changed tags with '+' and '-' prefixes."
+  (let ((result-tags (copy-sequence current-tags)))
+    (mapc (lambda (changed-tag)
+    (unless (string= changed-tag "")
+      (let ((op (substring changed-tag 0 1))
+    (tag (substring changed-tag 1)))
+ (cond ((string= op "+")
+       (unless (member tag result-tags)
+ (push tag result-tags)))
+      ((string= op "-")
+       (setq result-tags (delete tag result-tags)))
+      (t
+       (error "Changed tag must be of the form `+this_tag' or `-that_tag'"))))))
+       changed-tags)
+    (sort result-tags 'string<)))
 
 (defun notmuch-foreach-mime-part (function mm-handle)
   (cond ((stringp (car mm-handle))
@@ -447,6 +465,10 @@ Complete list of currently available key bindings:
   "Return a list of threads for the current region"
   (notmuch-search-properties-in-region 'notmuch-search-thread-id beg end))
 
+(defun notmuch-search-find-thread-id-region-search (beg end)
+  "Return a search string for threads for the current region"
+  (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or "))
+
 (defun notmuch-search-find-authors ()
   "Return the authors for the current thread"
   (get-text-property (point) 'notmuch-search-authors))
@@ -590,74 +612,55 @@ the messages that were tagged"
  (forward-line 1))
       output)))
 
-(defun notmuch-search-add-tag-thread (tag)
-  (notmuch-search-add-tag-region tag (point) (point)))
+(defun notmuch-search-tag-thread (&rest tags)
+  "Change tags for the currently selected thread.
 
-(defun notmuch-search-add-tag-region (tag beg end)
-  (let ((search-id-string (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or ")))
-    (notmuch-tag search-id-string (concat "+" tag))
-    (save-excursion
-      (let ((last-line (line-number-at-pos end))
-    (max-line (- (line-number-at-pos (point-max)) 2)))
- (goto-char beg)
- (while (<= (line-number-at-pos) (min last-line max-line))
-  (notmuch-search-set-tags (delete-dups (sort (cons tag (notmuch-search-get-tags)) 'string<)))
-  (forward-line))))))
+See `notmuch-search-tag-region' for details."
+  (apply 'notmuch-search-tag-region (point) (point) tags))
 
-(defun notmuch-search-remove-tag-thread (tag)
-  (notmuch-search-remove-tag-region tag (point) (point)))
+(defun notmuch-search-tag-region (beg end &rest tags)
+  "Change tags for threads in the given region.
 
-(defun notmuch-search-remove-tag-region (tag beg end)
-  (let ((search-id-string (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or ")))
-    (notmuch-tag search-id-string (concat "-" tag))
+`Tags' is a list of tag operations for \"notmuch tag\", i.e. a
+list of tags to change with '+' and '-' prefixes.  The tags are
+added or removed for all threads in the region from `beg' to
+`end'."
+  (let ((search-string (notmuch-search-find-thread-id-region-search beg end)))
+    (apply 'notmuch-tag search-string tags)
     (save-excursion
       (let ((last-line (line-number-at-pos end))
     (max-line (- (line-number-at-pos (point-max)) 2)))
  (goto-char beg)
  (while (<= (line-number-at-pos) (min last-line max-line))
-  (notmuch-search-set-tags (delete tag (notmuch-search-get-tags)))
+  (notmuch-search-set-tags
+   (notmuch-update-tags (notmuch-search-get-tags) tags))
   (forward-line))))))
 
-(defun notmuch-search-add-tag (tag)
-  "Add a tag to the currently selected thread or region.
-
-The tag is added to all messages in the currently selected thread
-or threads in the current region."
-  (interactive
-   (list (notmuch-select-tag-with-completion "Tag to add: ")))
-  (save-excursion
-    (if (region-active-p)
- (let* ((beg (region-beginning))
-       (end (region-end)))
-  (notmuch-search-add-tag-region tag beg end))
-      (notmuch-search-add-tag-thread tag))))
-
-(defun notmuch-search-remove-tag (tag)
-  "Remove a tag from the currently selected thread or region.
+(defun notmuch-search-tag (&optional initial-input)
+  "Change tags for the currently selected thread or region."
+  (interactive)
+  (let* ((beg (if (region-active-p) (region-beginning) (point)))
+ (end (if (region-active-p) (region-end) (point)))
+ (search-string (notmuch-search-find-thread-id-region-search beg end))
+ (tags (notmuch-select-tags-with-completion initial-input search-string)))
+    (apply 'notmuch-search-tag-region beg end tags)))
+
+(defun notmuch-search-add-tag ()
+  "Same as `notmuch-search-tag' but sets initial input to '+'."
+  (interactive)
+  (notmuch-search-tag "+"))
 
-The tag is removed from all messages in the currently selected
-thread or threads in the current region."
-  (interactive
-   (list (notmuch-select-tag-with-completion
-  "Tag to remove: "
-  (if (region-active-p)
-      (mapconcat 'identity
- (notmuch-search-find-thread-id-region (region-beginning) (region-end))
- " ")
-    (notmuch-search-find-thread-id)))))
-  (save-excursion
-    (if (region-active-p)
- (let* ((beg (region-beginning))
-       (end (region-end)))
-  (notmuch-search-remove-tag-region tag beg end))
-      (notmuch-search-remove-tag-thread tag))))
+(defun notmuch-search-remove-tag ()
+  "Same as `notmuch-search-tag' but sets initial input to '-'."
+  (interactive)
+  (notmuch-search-tag "-"))
 
 (defun notmuch-search-archive-thread ()
   "Archive the currently selected thread (remove its \"inbox\" tag).
 
 This function advances the next thread when finished."
   (interactive)
-  (notmuch-search-remove-tag-thread "inbox")
+  (notmuch-search-tag-thread "-inbox")
   (notmuch-search-next-thread))
 
 (defvar notmuch-search-process-filter-data nil
@@ -893,9 +896,7 @@ will prompt for tags to be added or removed. Tags prefixed with
 Each character of the tag name may consist of alphanumeric
 characters as well as `_.+-'.
 "
-  (interactive (notmuch-select-tags-with-completion
- "Operations (+add -drop): notmuch tag "
- '("+" "-")))
+  (interactive (notmuch-select-tags-with-completion))
   (apply 'notmuch-tag notmuch-search-query-string actions))
 
 (defun notmuch-search-buffer-title (query)
--
1.7.8.3

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

[Dmitry Kurochkin]

After the recent tagging operations changes, functions bound to "+"
and "-" in notmuch-search and notmuch-show views always read input
from the minibuffer.  Use kbd macros instead of calling them directly.
---
 test/emacs |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/test/emacs b/test/emacs
index 8ca4c8a..b9c0e02 100755
--- a/test/emacs
+++ b/test/emacs
@@ -101,26 +101,26 @@ test_begin_subtest "Add tag from search view"
 os_x_darwin_thread=$(notmuch search --output=threads id:[hidden email])
 test_emacs "(notmuch-search \"$os_x_darwin_thread\")
     (notmuch-test-wait)
-    (notmuch-search-add-tag \"tag-from-search-view\")"
+    (execute-kbd-macro \"+tag-from-search-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-search-view unread)"
 
 test_begin_subtest "Remove tag from search view"
 test_emacs "(notmuch-search \"$os_x_darwin_thread\")
     (notmuch-test-wait)
-    (notmuch-search-remove-tag \"tag-from-search-view\")"
+    (execute-kbd-macro \"-tag-from-search-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
 test_begin_subtest "Add tag from notmuch-show view"
 test_emacs "(notmuch-show \"$os_x_darwin_thread\")
-    (notmuch-show-add-tag \"tag-from-show-view\")"
+    (execute-kbd-macro \"+tag-from-show-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-show-view unread)"
 
 test_begin_subtest "Remove tag from notmuch-show view"
 test_emacs "(notmuch-show \"$os_x_darwin_thread\")
-    (notmuch-show-remove-tag \"tag-from-show-view\")"
+    (execute-kbd-macro \"-tag-from-show-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
@@ -128,14 +128,14 @@ test_begin_subtest "Message with .. in Message-Id:"
 add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
 test_emacs '(notmuch-search "id:\"123..456@example\"")
     (notmuch-test-wait)
-    (notmuch-search-add-tag "search-add")
-    (notmuch-search-add-tag "search-remove")
-    (notmuch-search-remove-tag "search-remove")
+    (execute-kbd-macro "+search-add")
+    (execute-kbd-macro "+search-remove")
+    (execute-kbd-macro "-search-remove")
     (notmuch-show "id:\"123..456@example\"")
     (notmuch-test-wait)
-    (notmuch-show-add-tag "show-add")
-    (notmuch-show-add-tag "show-remove")
-    (notmuch-show-remove-tag "show-remove")'
+    (execute-kbd-macro "+show-add")
+    (execute-kbd-macro "+show-remove")
+    (execute-kbd-macro "-show-remove")'
 output=$(notmuch search 'id:"123..456@example"' | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Message with .. in Message-Id (inbox search-add show-add)"
 
--
1.7.8.3

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

[Dmitry Kurochkin]

The patch adds `notmuch-show-operate-all' function bound to "*" in
notmuch-show view.  The function is similar to the
`notmuch-search-operate-all' function for the notmuch-search view: it
changes tags for all messages in the current thread.
---
 emacs/notmuch-show.el |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 03eadfb..2ca4d92 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1073,6 +1073,7 @@ thread id.  If a prefix is given, crypto processing is toggled."
  (define-key map "c" 'notmuch-show-stash-map)
  (define-key map "=" 'notmuch-show-refresh-view)
  (define-key map "h" 'notmuch-show-toggle-headers)
+ (define-key map "*" 'notmuch-show-operate-all)
  (define-key map "-" 'notmuch-show-remove-tag)
  (define-key map "+" 'notmuch-show-add-tag)
  (define-key map "x" 'notmuch-show-archive-thread-then-exit)
@@ -1489,6 +1490,21 @@ i.e. a list of tags to change with '+' and '-' prefixes."
        initial-input (notmuch-show-get-message-id))))
     (apply 'notmuch-show-tag-message changed-tags)))
 
+(defun notmuch-show-operate-all (&rest changed-tags)
+  "Change tags for all messages in the current thread.
+
+`Changed-tags' is a list of tag operations for \"notmuch tag\",
+i.e. a list of tags to change with '+' and '-' prefixes."
+  (interactive (notmuch-select-tags-with-completion nil notmuch-show-thread-id))
+  (apply 'notmuch-tag notmuch-show-thread-id changed-tags)
+  (save-excursion
+    (goto-char (point-min))
+    (loop do (let* ((current-tags (notmuch-show-get-tags))
+    (new-tags (notmuch-update-tags current-tags changed-tags)))
+       (unless (equal current-tags new-tags)
+ (notmuch-show-set-tags new-tags)))
+  while (notmuch-show-goto-message-next))))
+
 (defun notmuch-show-add-tag ()
   "Same as `notmuch-show-tag' but sets initial input to '+'."
   (interactive)
--
1.7.8.3

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

[Dmitry Kurochkin]

Some tag-related operations accept a single tag without prefix
(`notmuch-select-tag-with-completion'), others accept multiple tags
prefixed with '+' or '-' (`notmuch-select-tags-with-completion').
Before the change, both functions used a single default minibuffer
history.  This is inconvenient because you have to skip options with
incompatible format when going through the history.  The patch adds
separate history lists for the two functions.  Note that functions
that accept the same input format (e.g. "+", "-", "*") share the
history list as before.
---
 emacs/notmuch.el |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 24b0ea3..9813e0a 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -76,6 +76,14 @@ For example:
 (defvar notmuch-query-history nil
   "Variable to store minibuffer history for notmuch queries")
 
+(defvar notmuch-select-tag-history nil
+  "Variable to store notmuch tag history for
+  `notmuch-select-tag-with-completion'.")
+
+(defvar notmuch-select-tags-history nil
+  "Variable to store notmuch tags history for
+  `notmuch-select-tags-with-completion'.")
+
 (defun notmuch-tag-completions (&optional search-terms)
   (split-string
    (with-output-to-string
@@ -86,7 +94,7 @@ For example:
 
 (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
   (let ((tag-list (notmuch-tag-completions search-terms)))
-    (completing-read prompt tag-list)))
+    (completing-read prompt tag-list nil nil nil 'notmuch-select-tag-history)))
 
 (defun notmuch-select-tags-with-completion (&optional initial-input &rest search-terms)
   (let* ((add-tag-list (mapcar (apply-partially 'concat "+")
@@ -105,7 +113,7 @@ For example:
     map)))
     (delete "" (completing-read-multiple
  "Operations (+add -drop): notmuch tag " tag-list nil
- nil initial-input))))
+ nil initial-input 'notmuch-select-tags-history))))
 
 (defun notmuch-update-tags (current-tags changed-tags)
   "Update `current-tags' with `changed-tags' and return the result.
--
1.7.8.3

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

[Dmitry Kurochkin]

The tag syntax check in `notmuch-tag' function was too strict and did
not allow nmbug tags with "::".  Since the check is done for all
tagging operations in Emacs UI, this basically means that no nmbug
tags can be changed.  The patch relaxes the tag syntax check to allow
any tag names that do not include whitespace characters.
---
 emacs/notmuch.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 9813e0a..0de6123 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -555,7 +555,7 @@ notmuch-after-tag-hook will be run."
   ;; Perform some validation
   (when (null tags) (error "No tags given"))
   (mapc (lambda (tag)
-  (unless (string-match-p "^[-+][-+_.[:word:]]+$" tag)
+  (unless (string-match-p "^[-+]\\S-+$" tag)
     (error "Tag must be of the form `+this_tag' or `-that_tag'")))
  tags)
   (run-hooks 'notmuch-before-tag-hook)
--
1.7.8.3

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

[Dmitry Kurochkin]

Before the change, `notmuch-show-operate-all' used thread id for
"notmuch tag" search.  This could result in tagging unexpected
messages that were added to the thread after the notmuch-show buffer
was created.  The patch changes `notmuch-show-operate-all' to use ids
of shown messages to fix this.
---
 emacs/notmuch-show.el |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 2ca4d92..e606224 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1170,6 +1170,15 @@ All currently available key bindings:
     (notmuch-show-move-to-message-top)
     t))
 
+(defun notmuch-show-mapc (function)
+  "Iterate through all messages with
+`notmuch-show-goto-message-next' and call `function' for side
+effects."
+  (save-excursion
+    (goto-char (point-min))
+    (loop do (funcall function)
+  while (notmuch-show-goto-message-next))))
+
 ;; Functions relating to the visibility of messages and their
 ;; components.
 
@@ -1222,6 +1231,18 @@ Some useful entries are:
   "Return the message id of the current message."
   (concat "id:\"" (notmuch-show-get-prop :id) "\""))
 
+(defun notmuch-show-get-messages-ids ()
+  "Return all message ids of currently shown messages."
+  (let ((message-ids))
+    (notmuch-show-mapc
+     (lambda () (push (notmuch-show-get-message-id) message-ids)))
+    message-ids))
+
+(defun notmuch-show-get-messages-ids-search ()
+  "Return a search string for all message ids of currently shown
+messages."
+  (mapconcat 'identity (notmuch-show-get-messages-ids) " or "))
+
 ;; dme: Would it make sense to use a macro for many of these?
 
 (defun notmuch-show-get-filename ()
@@ -1496,7 +1517,7 @@ i.e. a list of tags to change with '+' and '-' prefixes."
 `Changed-tags' is a list of tag operations for \"notmuch tag\",
 i.e. a list of tags to change with '+' and '-' prefixes."
   (interactive (notmuch-select-tags-with-completion nil notmuch-show-thread-id))
-  (apply 'notmuch-tag notmuch-show-thread-id changed-tags)
+  (apply 'notmuch-tag (notmuch-show-get-messages-ids-search) changed-tags)
   (save-excursion
     (goto-char (point-min))
     (loop do (let* ((current-tags (notmuch-show-get-tags))
--
1.7.8.3

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

[Dmitry Kurochkin]

Use `notmuch-show-mapc' function instead of a custom `loop'.
---
 emacs/notmuch-show.el |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e606224..4ec3fce 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1518,13 +1518,12 @@ i.e. a list of tags to change with '+' and '-' prefixes."
 i.e. a list of tags to change with '+' and '-' prefixes."
   (interactive (notmuch-select-tags-with-completion nil notmuch-show-thread-id))
   (apply 'notmuch-tag (notmuch-show-get-messages-ids-search) changed-tags)
-  (save-excursion
-    (goto-char (point-min))
-    (loop do (let* ((current-tags (notmuch-show-get-tags))
-    (new-tags (notmuch-update-tags current-tags changed-tags)))
-       (unless (equal current-tags new-tags)
- (notmuch-show-set-tags new-tags)))
-  while (notmuch-show-goto-message-next))))
+  (notmuch-show-mapc
+   (lambda ()
+     (let* ((current-tags (notmuch-show-get-tags))
+    (new-tags (notmuch-update-tags current-tags changed-tags)))
+       (unless (equal current-tags new-tags)
+ (notmuch-show-set-tags new-tags))))))
 
 (defun notmuch-show-add-tag ()
   "Same as `notmuch-show-tag' but sets initial input to '+'."
--
1.7.8.3

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

[Jani Nikula]

On Jan 28, 2012 7:06 AM, "Dmitry Kurochkin" <[hidden email]> wrote:
>
> The tag syntax check in `notmuch-tag' function was too strict and did
> not allow nmbug tags with "::".  Since the check is done for all
> tagging operations in Emacs UI, this basically means that no nmbug
> tags can be changed.  The patch relaxes the tag syntax check to allow
> any tag names that do not include whitespace characters.

Imho the syntax check should be in cli, or lib even. I posted a patch to cli some time ago when I realized it's possible to add tag "-" but you can't remove it with the current cli. (On the road, can't find the message id now.)

> ---
>  emacs/notmuch.el |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 9813e0a..0de6123 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -555,7 +555,7 @@ notmuch-after-tag-hook will be run."
>   ;; Perform some validation
>   (when (null tags) (error "No tags given"))
>   (mapc (lambda (tag)
> -         (unless (string-match-p "^[-+][-+_.[:word:]]+$" tag)
> +         (unless (string-match-p "^[-+]\\S-+$" tag)
>            (error "Tag must be of the form `+this_tag' or `-that_tag'")))
>        tags)
>   (run-hooks 'notmuch-before-tag-hook)
> --
> 1.7.8.3
>
> _______________________________________________
> notmuch mailing list
> [hidden email]
> http://notmuchmail.org/mailman/listinfo/notmuch

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

[Jani Nikula]

I guess this now includes the optimization of doing the tagging in a single call to notmuch tag. (As opposed to calling it once per msg like it used to be a while back.) There was some discussion about the cmdline length for large threads potentially growing too big when I sent such an optimization patch, shall we just ignore that and hope for the best? I guess an idea was to limit to, say, a few hundred msg ids per command. (Again, sorry I can't look up the earlier thread now.)

On Jan 28, 2012 8:00 AM, "Dmitry Kurochkin" <[hidden email]> wrote:
>
> Before the change, `notmuch-show-operate-all' used thread id for
> "notmuch tag" search.  This could result in tagging unexpected
> messages that were added to the thread after the notmuch-show buffer
> was created.  The patch changes `notmuch-show-operate-all' to use ids
> of shown messages to fix this.
> ---
>  emacs/notmuch-show.el |   23 ++++++++++++++++++++++-
>  1 files changed, 22 insertions(+), 1 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 2ca4d92..e606224 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1170,6 +1170,15 @@ All currently available key bindings:
>     (notmuch-show-move-to-message-top)
>     t))
>
> +(defun notmuch-show-mapc (function)
> +  "Iterate through all messages with
> +`notmuch-show-goto-message-next' and call `function' for side
> +effects."
> +  (save-excursion
> +    (goto-char (point-min))
> +    (loop do (funcall function)
> +         while (notmuch-show-goto-message-next))))
> +
>  ;; Functions relating to the visibility of messages and their
>  ;; components.
>
> @@ -1222,6 +1231,18 @@ Some useful entries are:
>   "Return the message id of the current message."
>   (concat "id:\"" (notmuch-show-get-prop :id) "\""))
>
> +(defun notmuch-show-get-messages-ids ()
> +  "Return all message ids of currently shown messages."
> +  (let ((message-ids))
> +    (notmuch-show-mapc
> +     (lambda () (push (notmuch-show-get-message-id) message-ids)))
> +    message-ids))
> +
> +(defun notmuch-show-get-messages-ids-search ()
> +  "Return a search string for all message ids of currently shown
> +messages."
> +  (mapconcat 'identity (notmuch-show-get-messages-ids) " or "))
> +
>  ;; dme: Would it make sense to use a macro for many of these?
>
>  (defun notmuch-show-get-filename ()
> @@ -1496,7 +1517,7 @@ i.e. a list of tags to change with '+' and '-' prefixes."
>  `Changed-tags' is a list of tag operations for \"notmuch tag\",
>  i.e. a list of tags to change with '+' and '-' prefixes."
>   (interactive (notmuch-select-tags-with-completion nil notmuch-show-thread-id))
> -  (apply 'notmuch-tag notmuch-show-thread-id changed-tags)
> +  (apply 'notmuch-tag (notmuch-show-get-messages-ids-search) changed-tags)
>   (save-excursion
>     (goto-char (point-min))
>     (loop do (let* ((current-tags (notmuch-show-get-tags))
> --
> 1.7.8.3
>
> _______________________________________________
> notmuch mailing list
> [hidden email]
> http://notmuchmail.org/mailman/listinfo/notmuch

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

[Dmitry Kurochkin]

Hi Jani.

On Sat, 28 Jan 2012 11:09:45 +0200, Jani Nikula <[hidden email]> wrote:
> I guess this now includes the optimization of doing the tagging in a single
> call to notmuch tag. (As opposed to calling it once per msg like it used to
> be a while back.)

This patch changes the code which was added few patches earlier in the
series.  And rational for it is not an optimization but a bug fix as
described in the preamble.

> There was some discussion about the cmdline length for
> large threads potentially growing too big when I sent such an optimization
> patch, shall we just ignore that and hope for the best?

Sorry, I think I did not read that discussion in details.  I have never
hit this issue.  So, for now, I do not care about it.

Notmuch-search range tagging works in a similar way, constructing search
string of OR'ed thread ids.  So the patch is consistent with the
existing code.

> I guess an idea was
> to limit to, say, a few hundred msg ids per command. (Again, sorry I can't
> look up the earlier thread now.)
>

That may be a solution.  Though, I think it should rely on system's
command line limit instead of magic constants.

Anyway, this problem is out of scope of this patch series.  Currently,
neither notmuch-search region tagging code nor the proposed patch try to
solve the issue.  If and when a proper solution is found, we should
implement it for both (or probably more) cases (provided this patch gets
accepted).

Regards,
  Dmitry

> On Jan 28, 2012 8:00 AM, "Dmitry Kurochkin" <[hidden email]>
> wrote:
> >
> > Before the change, `notmuch-show-operate-all' used thread id for
> > "notmuch tag" search.  This could result in tagging unexpected
> > messages that were added to the thread after the notmuch-show buffer
> > was created.  The patch changes `notmuch-show-operate-all' to use ids
> > of shown messages to fix this.
> > ---
> >  emacs/notmuch-show.el |   23 ++++++++++++++++++++++-
> >  1 files changed, 22 insertions(+), 1 deletions(-)
> >
> > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> > index 2ca4d92..e606224 100644
> > --- a/emacs/notmuch-show.el
> > +++ b/emacs/notmuch-show.el
> > @@ -1170,6 +1170,15 @@ All currently available key bindings:
> >     (notmuch-show-move-to-message-top)
> >     t))
> >
> > +(defun notmuch-show-mapc (function)
> > +  "Iterate through all messages with
> > +`notmuch-show-goto-message-next' and call `function' for side
> > +effects."
> > +  (save-excursion
> > +    (goto-char (point-min))
> > +    (loop do (funcall function)
> > +         while (notmuch-show-goto-message-next))))
> > +
> >  ;; Functions relating to the visibility of messages and their
> >  ;; components.
> >
> > @@ -1222,6 +1231,18 @@ Some useful entries are:
> >   "Return the message id of the current message."
> >   (concat "id:\"" (notmuch-show-get-prop :id) "\""))
> >
> > +(defun notmuch-show-get-messages-ids ()
> > +  "Return all message ids of currently shown messages."
> > +  (let ((message-ids))
> > +    (notmuch-show-mapc
> > +     (lambda () (push (notmuch-show-get-message-id) message-ids)))
> > +    message-ids))
> > +
> > +(defun notmuch-show-get-messages-ids-search ()
> > +  "Return a search string for all message ids of currently shown
> > +messages."
> > +  (mapconcat 'identity (notmuch-show-get-messages-ids) " or "))
> > +
> >  ;; dme: Would it make sense to use a macro for many of these?
> >
> >  (defun notmuch-show-get-filename ()
> > @@ -1496,7 +1517,7 @@ i.e. a list of tags to change with '+' and '-'
> prefixes."
> >  `Changed-tags' is a list of tag operations for \"notmuch tag\",
> >  i.e. a list of tags to change with '+' and '-' prefixes."
> >   (interactive (notmuch-select-tags-with-completion nil
> notmuch-show-thread-id))
> > -  (apply 'notmuch-tag notmuch-show-thread-id changed-tags)
> > +  (apply 'notmuch-tag (notmuch-show-get-messages-ids-search)
> changed-tags)
> >   (save-excursion
> >     (goto-char (point-min))
> >     (loop do (let* ((current-tags (notmuch-show-get-tags))
> > --
> > 1.7.8.3
> >
> > _______________________________________________
> > notmuch mailing list
> > [hidden email]
> > http://notmuchmail.org/mailman/listinfo/notmuch

Non-text part: text/html
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Dmitry Kurochkin]

On Sat, 28 Jan 2012 10:56:21 +0200, Jani Nikula <[hidden email]> wrote:

> On Jan 28, 2012 7:06 AM, "Dmitry Kurochkin" <[hidden email]>
> wrote:
> >
> > The tag syntax check in `notmuch-tag' function was too strict and did
> > not allow nmbug tags with "::".  Since the check is done for all
> > tagging operations in Emacs UI, this basically means that no nmbug
> > tags can be changed.  The patch relaxes the tag syntax check to allow
> > any tag names that do not include whitespace characters.
>
> Imho the syntax check should be in cli, or lib even. I posted a patch to
> cli some time ago when I realized it's possible to add tag "-" but you
> can't remove it with the current cli. (On the road, can't find the message
> id now.)
>

I agree that this is an issue (a general issue for notmuch, not just
Emacs UI).  But it is outside of scope of this patch.  This patch
purpose is just to solve an immediate issue with a broken regexp.

Regards,
  Dmitry

> > ---
> >  emacs/notmuch.el |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index 9813e0a..0de6123 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -555,7 +555,7 @@ notmuch-after-tag-hook will be run."
> >   ;; Perform some validation
> >   (when (null tags) (error "No tags given"))
> >   (mapc (lambda (tag)
> > -         (unless (string-match-p "^[-+][-+_.[:word:]]+$" tag)
> > +         (unless (string-match-p "^[-+]\\S-+$" tag)
> >            (error "Tag must be of the form `+this_tag' or `-that_tag'")))
> >        tags)
> >   (run-hooks 'notmuch-before-tag-hook)
> > --
> > 1.7.8.3
> >
> > _______________________________________________
> > notmuch mailing list
> > [hidden email]
> > http://notmuchmail.org/mailman/listinfo/notmuch

Non-text part: text/html
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Jeremy Nickurak]

Is it safe to assume that any reasonable seperator (comma, space,
semicolon, plus or minus sign, anything) won't show up in a tag name?

On Fri, Jan 27, 2012 at 21:41, Dmitry Kurochkin
<[hidden email]> wrote:

> Before the change, "+" and "-" tagging operations in notmuch-search
> and notmuch-show views accepted only a single tag.  The patch makes
> them use the recently added `notmuch-select-tags-with-completion'
> function, which allows to enter multiple tags with "+" and "-"
> prefixes.  So after the change, "+" and "-" bindings allow to both add
> and remove multiple tags.  The only difference between "+" and "-" is
> the minibuffer initial input ("+" and "-" respectively).
> ---
>  emacs/notmuch-show.el |   65 +++++++------------
>  emacs/notmuch.el      |  165 +++++++++++++++++++++++++------------------------
>  2 files changed, 107 insertions(+), 123 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 84ac624..03eadfb 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -38,8 +38,9 @@
>
>  (declare-function notmuch-call-notmuch-process "notmuch" (&rest args))
>  (declare-function notmuch-fontify-headers "notmuch" nil)
> -(declare-function notmuch-select-tag-with-completion "notmuch" (prompt &rest search-terms))
> +(declare-function notmuch-select-tags-with-completion "notmuch" (&optional initial-input &rest search-terms))
>  (declare-function notmuch-search-show-thread "notmuch" nil)
> +(declare-function notmuch-update-tags "notmuch" (current-tags changed-tags))
>
>  (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
>   "Headers that should be shown in a message, in this order.
> @@ -1267,7 +1268,7 @@ Some useful entries are:
>
>  (defun notmuch-show-mark-read ()
>   "Mark the current message as read."
> -  (notmuch-show-remove-tag "unread"))
> +  (notmuch-show-tag-message "-unread"))
>
>  ;; Functions for getting attributes of several messages in the current
>  ;; thread.
> @@ -1470,51 +1471,33 @@ than only the current message."
>            (message (format "Command '%s' exited abnormally with code %d"
>                             shell-command exit-code))))))))
>
> -(defun notmuch-show-add-tags-worker (current-tags add-tags)
> -  "Add to `current-tags' with any tags from `add-tags' not
> -currently present and return the result."
> -  (let ((result-tags (copy-sequence current-tags)))
> -    (mapc (lambda (add-tag)
> -           (unless (member add-tag current-tags)
> -             (setq result-tags (push add-tag result-tags))))
> -           add-tags)
> -    (sort result-tags 'string<)))
> -
> -(defun notmuch-show-del-tags-worker (current-tags del-tags)
> -  "Remove any tags in `del-tags' from `current-tags' and return
> -the result."
> -  (let ((result-tags (copy-sequence current-tags)))
> -    (mapc (lambda (del-tag)
> -           (setq result-tags (delete del-tag result-tags)))
> -         del-tags)
> -    result-tags))
> -
> -(defun notmuch-show-add-tag (&rest toadd)
> -  "Add a tag to the current message."
> -  (interactive
> -   (list (notmuch-select-tag-with-completion "Tag to add: ")))
> +(defun notmuch-show-tag-message (&rest changed-tags)
> +  "Change tags for the current message.
>
> +`Changed-tags' is a list of tag operations for \"notmuch tag\",
> +i.e. a list of tags to change with '+' and '-' prefixes."
>   (let* ((current-tags (notmuch-show-get-tags))
> -        (new-tags (notmuch-show-add-tags-worker current-tags toadd)))
> -
> +        (new-tags (notmuch-update-tags current-tags changed-tags)))
>     (unless (equal current-tags new-tags)
> -      (apply 'notmuch-tag (notmuch-show-get-message-id)
> -            (mapcar (lambda (s) (concat "+" s)) toadd))
> +      (apply 'notmuch-tag (notmuch-show-get-message-id) changed-tags)
>       (notmuch-show-set-tags new-tags))))
>
> -(defun notmuch-show-remove-tag (&rest toremove)
> -  "Remove a tag from the current message."
> -  (interactive
> -   (list (notmuch-select-tag-with-completion
> -         "Tag to remove: " (notmuch-show-get-message-id))))
> +(defun notmuch-show-tag (&optional initial-input)
> +  "Change tags for the current message, read input from the minibuffer."
> +  (interactive)
> +  (let ((changed-tags (notmuch-select-tags-with-completion
> +                      initial-input (notmuch-show-get-message-id))))
> +    (apply 'notmuch-show-tag-message changed-tags)))
>
> -  (let* ((current-tags (notmuch-show-get-tags))
> -        (new-tags (notmuch-show-del-tags-worker current-tags toremove)))
> +(defun notmuch-show-add-tag ()
> +  "Same as `notmuch-show-tag' but sets initial input to '+'."
> +  (interactive)
> +  (notmuch-show-tag "+"))
>
> -    (unless (equal current-tags new-tags)
> -      (apply 'notmuch-tag (notmuch-show-get-message-id)
> -            (mapcar (lambda (s) (concat "-" s)) toremove))
> -      (notmuch-show-set-tags new-tags))))
> +(defun notmuch-show-remove-tag ()
> +  "Same as `notmuch-show-tag' but sets initial input to '-'."
> +  (interactive)
> +  (notmuch-show-tag "-"))
>
>  (defun notmuch-show-toggle-headers ()
>   "Toggle the visibility of the current message headers."
> @@ -1559,7 +1542,7 @@ argument, hide all of the messages."
>  (defun notmuch-show-archive-thread-internal (show-next)
>   ;; Remove the tag from the current set of messages.
>   (goto-char (point-min))
> -  (loop do (notmuch-show-remove-tag "inbox")
> +  (loop do (notmuch-show-tag-message "-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))
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index ff46617..24b0ea3 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -76,38 +76,56 @@ For example:
>  (defvar notmuch-query-history nil
>   "Variable to store minibuffer history for notmuch queries")
>
> -(defun notmuch-tag-completions (&optional prefixes search-terms)
> -  (let ((tag-list
> -        (split-string
> -         (with-output-to-string
> -           (with-current-buffer standard-output
> -             (apply 'call-process notmuch-command nil t
> -                    nil "search-tags" search-terms)))
> -         "\n+" t)))
> -    (if (null prefixes)
> -       tag-list
> -      (apply #'append
> -            (mapcar (lambda (tag)
> -                      (mapcar (lambda (prefix)
> -                                (concat prefix tag)) prefixes))
> -                    tag-list)))))
> +(defun notmuch-tag-completions (&optional search-terms)
> +  (split-string
> +   (with-output-to-string
> +     (with-current-buffer standard-output
> +       (apply 'call-process notmuch-command nil t
> +             nil "search-tags" search-terms)))
> +   "\n+" t))
>
>  (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
> -  (let ((tag-list (notmuch-tag-completions nil search-terms)))
> +  (let ((tag-list (notmuch-tag-completions search-terms)))
>     (completing-read prompt tag-list)))
>
> -(defun notmuch-select-tags-with-completion (prompt &optional prefixes &rest search-terms)
> -  (let ((tag-list (notmuch-tag-completions prefixes search-terms))
> -       (crm-separator " ")
> -       ;; By default, space is bound to "complete word" function.
> -       ;; Re-bind it to insert a space instead.  Note that <tab>
> -       ;; still does the completion.
> -       (crm-local-completion-map
> -        (let ((map (make-sparse-keymap)))
> -          (set-keymap-parent map crm-local-completion-map)
> -          (define-key map " " 'self-insert-command)
> -          map)))
> -    (delete "" (completing-read-multiple prompt tag-list))))
> +(defun notmuch-select-tags-with-completion (&optional initial-input &rest search-terms)
> +  (let* ((add-tag-list (mapcar (apply-partially 'concat "+")
> +                              (notmuch-tag-completions)))
> +        (remove-tag-list (mapcar (apply-partially 'concat "-")
> +                                 (notmuch-tag-completions search-terms)))
> +        (tag-list (append add-tag-list remove-tag-list))
> +        (crm-separator " ")
> +        ;; By default, space is bound to "complete word" function.
> +        ;; Re-bind it to insert a space instead.  Note that <tab>
> +        ;; still does the completion.
> +        (crm-local-completion-map
> +         (let ((map (make-sparse-keymap)))
> +           (set-keymap-parent map crm-local-completion-map)
> +           (define-key map " " 'self-insert-command)
> +           map)))
> +    (delete "" (completing-read-multiple
> +               "Operations (+add -drop): notmuch tag " tag-list nil
> +               nil initial-input))))
> +
> +(defun notmuch-update-tags (current-tags changed-tags)
> +  "Update `current-tags' with `changed-tags' and return the result.
> +
> +`Changed-tags' is a list of tag operations given to \"notmuch
> +tag\", i.e. a list of changed tags with '+' and '-' prefixes."
> +  (let ((result-tags (copy-sequence current-tags)))
> +    (mapc (lambda (changed-tag)
> +           (unless (string= changed-tag "")
> +             (let ((op (substring changed-tag 0 1))
> +                   (tag (substring changed-tag 1)))
> +               (cond ((string= op "+")
> +                      (unless (member tag result-tags)
> +                        (push tag result-tags)))
> +                     ((string= op "-")
> +                      (setq result-tags (delete tag result-tags)))
> +                     (t
> +                      (error "Changed tag must be of the form `+this_tag' or `-that_tag'"))))))
> +       changed-tags)
> +    (sort result-tags 'string<)))
>
>  (defun notmuch-foreach-mime-part (function mm-handle)
>   (cond ((stringp (car mm-handle))
> @@ -447,6 +465,10 @@ Complete list of currently available key bindings:
>   "Return a list of threads for the current region"
>   (notmuch-search-properties-in-region 'notmuch-search-thread-id beg end))
>
> +(defun notmuch-search-find-thread-id-region-search (beg end)
> +  "Return a search string for threads for the current region"
> +  (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or "))
> +
>  (defun notmuch-search-find-authors ()
>   "Return the authors for the current thread"
>   (get-text-property (point) 'notmuch-search-authors))
> @@ -590,74 +612,55 @@ the messages that were tagged"
>        (forward-line 1))
>       output)))
>
> -(defun notmuch-search-add-tag-thread (tag)
> -  (notmuch-search-add-tag-region tag (point) (point)))
> +(defun notmuch-search-tag-thread (&rest tags)
> +  "Change tags for the currently selected thread.
>
> -(defun notmuch-search-add-tag-region (tag beg end)
> -  (let ((search-id-string (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or ")))
> -    (notmuch-tag search-id-string (concat "+" tag))
> -    (save-excursion
> -      (let ((last-line (line-number-at-pos end))
> -           (max-line (- (line-number-at-pos (point-max)) 2)))
> -       (goto-char beg)
> -       (while (<= (line-number-at-pos) (min last-line max-line))
> -         (notmuch-search-set-tags (delete-dups (sort (cons tag (notmuch-search-get-tags)) 'string<)))
> -         (forward-line))))))
> +See `notmuch-search-tag-region' for details."
> +  (apply 'notmuch-search-tag-region (point) (point) tags))
>
> -(defun notmuch-search-remove-tag-thread (tag)
> -  (notmuch-search-remove-tag-region tag (point) (point)))
> +(defun notmuch-search-tag-region (beg end &rest tags)
> +  "Change tags for threads in the given region.
>
> -(defun notmuch-search-remove-tag-region (tag beg end)
> -  (let ((search-id-string (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or ")))
> -    (notmuch-tag search-id-string (concat "-" tag))
> +`Tags' is a list of tag operations for \"notmuch tag\", i.e. a
> +list of tags to change with '+' and '-' prefixes.  The tags are
> +added or removed for all threads in the region from `beg' to
> +`end'."
> +  (let ((search-string (notmuch-search-find-thread-id-region-search beg end)))
> +    (apply 'notmuch-tag search-string tags)
>     (save-excursion
>       (let ((last-line (line-number-at-pos end))
>            (max-line (- (line-number-at-pos (point-max)) 2)))
>        (goto-char beg)
>        (while (<= (line-number-at-pos) (min last-line max-line))
> -         (notmuch-search-set-tags (delete tag (notmuch-search-get-tags)))
> +         (notmuch-search-set-tags
> +          (notmuch-update-tags (notmuch-search-get-tags) tags))
>          (forward-line))))))
>
> -(defun notmuch-search-add-tag (tag)
> -  "Add a tag to the currently selected thread or region.
> -
> -The tag is added to all messages in the currently selected thread
> -or threads in the current region."
> -  (interactive
> -   (list (notmuch-select-tag-with-completion "Tag to add: ")))
> -  (save-excursion
> -    (if (region-active-p)
> -       (let* ((beg (region-beginning))
> -              (end (region-end)))
> -         (notmuch-search-add-tag-region tag beg end))
> -      (notmuch-search-add-tag-thread tag))))
> -
> -(defun notmuch-search-remove-tag (tag)
> -  "Remove a tag from the currently selected thread or region.
> +(defun notmuch-search-tag (&optional initial-input)
> +  "Change tags for the currently selected thread or region."
> +  (interactive)
> +  (let* ((beg (if (region-active-p) (region-beginning) (point)))
> +        (end (if (region-active-p) (region-end) (point)))
> +        (search-string (notmuch-search-find-thread-id-region-search beg end))
> +        (tags (notmuch-select-tags-with-completion initial-input search-string)))
> +    (apply 'notmuch-search-tag-region beg end tags)))
> +
> +(defun notmuch-search-add-tag ()
> +  "Same as `notmuch-search-tag' but sets initial input to '+'."
> +  (interactive)
> +  (notmuch-search-tag "+"))
>
> -The tag is removed from all messages in the currently selected
> -thread or threads in the current region."
> -  (interactive
> -   (list (notmuch-select-tag-with-completion
> -         "Tag to remove: "
> -         (if (region-active-p)
> -             (mapconcat 'identity
> -                        (notmuch-search-find-thread-id-region (region-beginning) (region-end))
> -                        " ")
> -           (notmuch-search-find-thread-id)))))
> -  (save-excursion
> -    (if (region-active-p)
> -       (let* ((beg (region-beginning))
> -              (end (region-end)))
> -         (notmuch-search-remove-tag-region tag beg end))
> -      (notmuch-search-remove-tag-thread tag))))
> +(defun notmuch-search-remove-tag ()
> +  "Same as `notmuch-search-tag' but sets initial input to '-'."
> +  (interactive)
> +  (notmuch-search-tag "-"))
>
>  (defun notmuch-search-archive-thread ()
>   "Archive the currently selected thread (remove its \"inbox\" tag).
>
>  This function advances the next thread when finished."
>   (interactive)
> -  (notmuch-search-remove-tag-thread "inbox")
> +  (notmuch-search-tag-thread "-inbox")
>   (notmuch-search-next-thread))
>
>  (defvar notmuch-search-process-filter-data nil
> @@ -893,9 +896,7 @@ will prompt for tags to be added or removed. Tags prefixed with
>  Each character of the tag name may consist of alphanumeric
>  characters as well as `_.+-'.
>  "
> -  (interactive (notmuch-select-tags-with-completion
> -               "Operations (+add -drop): notmuch tag "
> -               '("+" "-")))
> +  (interactive (notmuch-select-tags-with-completion))
>   (apply 'notmuch-tag notmuch-search-query-string actions))
>
>  (defun notmuch-search-buffer-title (query)
> --
> 1.7.8.3
>
> _______________________________________________
> notmuch mailing list
> [hidden email]
> http://notmuchmail.org/mailman/listinfo/notmuch

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

[Dmitry Kurochkin]

On Sat, 28 Jan 2012 09:49:33 -0700, Jeremy Nickurak <[hidden email]> wrote:
> Is it safe to assume that any reasonable seperator (comma, space,
> semicolon, plus or minus sign, anything) won't show up in a tag name?
>

No.  Threre are existing issues with tag names that contain "unexpected"
characters.  This series does not aim to resolve them and not make it
worse.  Also see Jani's reply to another patch in the series.

Regards,
  Dmitry

[1] id:"[hidden email]"

> On Fri, Jan 27, 2012 at 21:41, Dmitry Kurochkin
> <[hidden email]> wrote:
> > Before the change, "+" and "-" tagging operations in notmuch-search
> > and notmuch-show views accepted only a single tag.  The patch makes
> > them use the recently added `notmuch-select-tags-with-completion'
> > function, which allows to enter multiple tags with "+" and "-"
> > prefixes.  So after the change, "+" and "-" bindings allow to both add
> > and remove multiple tags.  The only difference between "+" and "-" is
> > the minibuffer initial input ("+" and "-" respectively).
> > ---
> >  emacs/notmuch-show.el |   65 +++++++------------
> >  emacs/notmuch.el      |  165 +++++++++++++++++++++++++------------------------
> >  2 files changed, 107 insertions(+), 123 deletions(-)
> >
> > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> > index 84ac624..03eadfb 100644
> > --- a/emacs/notmuch-show.el
> > +++ b/emacs/notmuch-show.el
> > @@ -38,8 +38,9 @@
> >
> >  (declare-function notmuch-call-notmuch-process "notmuch" (&rest args))
> >  (declare-function notmuch-fontify-headers "notmuch" nil)
> > -(declare-function notmuch-select-tag-with-completion "notmuch" (prompt &rest search-terms))
> > +(declare-function notmuch-select-tags-with-completion "notmuch" (&optional initial-input &rest search-terms))
> >  (declare-function notmuch-search-show-thread "notmuch" nil)
> > +(declare-function notmuch-update-tags "notmuch" (current-tags changed-tags))
> >
> >  (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
> >   "Headers that should be shown in a message, in this order.
> > @@ -1267,7 +1268,7 @@ Some useful entries are:
> >
> >  (defun notmuch-show-mark-read ()
> >   "Mark the current message as read."
> > -  (notmuch-show-remove-tag "unread"))
> > +  (notmuch-show-tag-message "-unread"))
> >
> >  ;; Functions for getting attributes of several messages in the current
> >  ;; thread.
> > @@ -1470,51 +1471,33 @@ than only the current message."
> >            (message (format "Command '%s' exited abnormally with code %d"
> >                             shell-command exit-code))))))))
> >
> > -(defun notmuch-show-add-tags-worker (current-tags add-tags)
> > -  "Add to `current-tags' with any tags from `add-tags' not
> > -currently present and return the result."
> > -  (let ((result-tags (copy-sequence current-tags)))
> > -    (mapc (lambda (add-tag)
> > -           (unless (member add-tag current-tags)
> > -             (setq result-tags (push add-tag result-tags))))
> > -           add-tags)
> > -    (sort result-tags 'string<)))
> > -
> > -(defun notmuch-show-del-tags-worker (current-tags del-tags)
> > -  "Remove any tags in `del-tags' from `current-tags' and return
> > -the result."
> > -  (let ((result-tags (copy-sequence current-tags)))
> > -    (mapc (lambda (del-tag)
> > -           (setq result-tags (delete del-tag result-tags)))
> > -         del-tags)
> > -    result-tags))
> > -
> > -(defun notmuch-show-add-tag (&rest toadd)
> > -  "Add a tag to the current message."
> > -  (interactive
> > -   (list (notmuch-select-tag-with-completion "Tag to add: ")))
> > +(defun notmuch-show-tag-message (&rest changed-tags)
> > +  "Change tags for the current message.
> >
> > +`Changed-tags' is a list of tag operations for \"notmuch tag\",
> > +i.e. a list of tags to change with '+' and '-' prefixes."
> >   (let* ((current-tags (notmuch-show-get-tags))
> > -        (new-tags (notmuch-show-add-tags-worker current-tags toadd)))
> > -
> > +        (new-tags (notmuch-update-tags current-tags changed-tags)))
> >     (unless (equal current-tags new-tags)
> > -      (apply 'notmuch-tag (notmuch-show-get-message-id)
> > -            (mapcar (lambda (s) (concat "+" s)) toadd))
> > +      (apply 'notmuch-tag (notmuch-show-get-message-id) changed-tags)
> >       (notmuch-show-set-tags new-tags))))
> >
> > -(defun notmuch-show-remove-tag (&rest toremove)
> > -  "Remove a tag from the current message."
> > -  (interactive
> > -   (list (notmuch-select-tag-with-completion
> > -         "Tag to remove: " (notmuch-show-get-message-id))))
> > +(defun notmuch-show-tag (&optional initial-input)
> > +  "Change tags for the current message, read input from the minibuffer."
> > +  (interactive)
> > +  (let ((changed-tags (notmuch-select-tags-with-completion
> > +                      initial-input (notmuch-show-get-message-id))))
> > +    (apply 'notmuch-show-tag-message changed-tags)))
> >
> > -  (let* ((current-tags (notmuch-show-get-tags))
> > -        (new-tags (notmuch-show-del-tags-worker current-tags toremove)))
> > +(defun notmuch-show-add-tag ()
> > +  "Same as `notmuch-show-tag' but sets initial input to '+'."
> > +  (interactive)
> > +  (notmuch-show-tag "+"))
> >
> > -    (unless (equal current-tags new-tags)
> > -      (apply 'notmuch-tag (notmuch-show-get-message-id)
> > -            (mapcar (lambda (s) (concat "-" s)) toremove))
> > -      (notmuch-show-set-tags new-tags))))
> > +(defun notmuch-show-remove-tag ()
> > +  "Same as `notmuch-show-tag' but sets initial input to '-'."
> > +  (interactive)
> > +  (notmuch-show-tag "-"))
> >
> >  (defun notmuch-show-toggle-headers ()
> >   "Toggle the visibility of the current message headers."
> > @@ -1559,7 +1542,7 @@ argument, hide all of the messages."
> >  (defun notmuch-show-archive-thread-internal (show-next)
> >   ;; Remove the tag from the current set of messages.
> >   (goto-char (point-min))
> > -  (loop do (notmuch-show-remove-tag "inbox")
> > +  (loop do (notmuch-show-tag-message "-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))
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index ff46617..24b0ea3 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -76,38 +76,56 @@ For example:
> >  (defvar notmuch-query-history nil
> >   "Variable to store minibuffer history for notmuch queries")
> >
> > -(defun notmuch-tag-completions (&optional prefixes search-terms)
> > -  (let ((tag-list
> > -        (split-string
> > -         (with-output-to-string
> > -           (with-current-buffer standard-output
> > -             (apply 'call-process notmuch-command nil t
> > -                    nil "search-tags" search-terms)))
> > -         "\n+" t)))
> > -    (if (null prefixes)
> > -       tag-list
> > -      (apply #'append
> > -            (mapcar (lambda (tag)
> > -                      (mapcar (lambda (prefix)
> > -                                (concat prefix tag)) prefixes))
> > -                    tag-list)))))
> > +(defun notmuch-tag-completions (&optional search-terms)
> > +  (split-string
> > +   (with-output-to-string
> > +     (with-current-buffer standard-output
> > +       (apply 'call-process notmuch-command nil t
> > +             nil "search-tags" search-terms)))
> > +   "\n+" t))
> >
> >  (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
> > -  (let ((tag-list (notmuch-tag-completions nil search-terms)))
> > +  (let ((tag-list (notmuch-tag-completions search-terms)))
> >     (completing-read prompt tag-list)))
> >
> > -(defun notmuch-select-tags-with-completion (prompt &optional prefixes &rest search-terms)
> > -  (let ((tag-list (notmuch-tag-completions prefixes search-terms))
> > -       (crm-separator " ")
> > -       ;; By default, space is bound to "complete word" function.
> > -       ;; Re-bind it to insert a space instead.  Note that <tab>
> > -       ;; still does the completion.
> > -       (crm-local-completion-map
> > -        (let ((map (make-sparse-keymap)))
> > -          (set-keymap-parent map crm-local-completion-map)
> > -          (define-key map " " 'self-insert-command)
> > -          map)))
> > -    (delete "" (completing-read-multiple prompt tag-list))))
> > +(defun notmuch-select-tags-with-completion (&optional initial-input &rest search-terms)
> > +  (let* ((add-tag-list (mapcar (apply-partially 'concat "+")
> > +                              (notmuch-tag-completions)))
> > +        (remove-tag-list (mapcar (apply-partially 'concat "-")
> > +                                 (notmuch-tag-completions search-terms)))
> > +        (tag-list (append add-tag-list remove-tag-list))
> > +        (crm-separator " ")
> > +        ;; By default, space is bound to "complete word" function.
> > +        ;; Re-bind it to insert a space instead.  Note that <tab>
> > +        ;; still does the completion.
> > +        (crm-local-completion-map
> > +         (let ((map (make-sparse-keymap)))
> > +           (set-keymap-parent map crm-local-completion-map)
> > +           (define-key map " " 'self-insert-command)
> > +           map)))
> > +    (delete "" (completing-read-multiple
> > +               "Operations (+add -drop): notmuch tag " tag-list nil
> > +               nil initial-input))))
> > +
> > +(defun notmuch-update-tags (current-tags changed-tags)
> > +  "Update `current-tags' with `changed-tags' and return the result.
> > +
> > +`Changed-tags' is a list of tag operations given to \"notmuch
> > +tag\", i.e. a list of changed tags with '+' and '-' prefixes."
> > +  (let ((result-tags (copy-sequence current-tags)))
> > +    (mapc (lambda (changed-tag)
> > +           (unless (string= changed-tag "")
> > +             (let ((op (substring changed-tag 0 1))
> > +                   (tag (substring changed-tag 1)))
> > +               (cond ((string= op "+")
> > +                      (unless (member tag result-tags)
> > +                        (push tag result-tags)))
> > +                     ((string= op "-")
> > +                      (setq result-tags (delete tag result-tags)))
> > +                     (t
> > +                      (error "Changed tag must be of the form `+this_tag' or `-that_tag'"))))))
> > +       changed-tags)
> > +    (sort result-tags 'string<)))
> >
> >  (defun notmuch-foreach-mime-part (function mm-handle)
> >   (cond ((stringp (car mm-handle))
> > @@ -447,6 +465,10 @@ Complete list of currently available key bindings:
> >   "Return a list of threads for the current region"
> >   (notmuch-search-properties-in-region 'notmuch-search-thread-id beg end))
> >
> > +(defun notmuch-search-find-thread-id-region-search (beg end)
> > +  "Return a search string for threads for the current region"
> > +  (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or "))
> > +
> >  (defun notmuch-search-find-authors ()
> >   "Return the authors for the current thread"
> >   (get-text-property (point) 'notmuch-search-authors))
> > @@ -590,74 +612,55 @@ the messages that were tagged"
> >        (forward-line 1))
> >       output)))
> >
> > -(defun notmuch-search-add-tag-thread (tag)
> > -  (notmuch-search-add-tag-region tag (point) (point)))
> > +(defun notmuch-search-tag-thread (&rest tags)
> > +  "Change tags for the currently selected thread.
> >
> > -(defun notmuch-search-add-tag-region (tag beg end)
> > -  (let ((search-id-string (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or ")))
> > -    (notmuch-tag search-id-string (concat "+" tag))
> > -    (save-excursion
> > -      (let ((last-line (line-number-at-pos end))
> > -           (max-line (- (line-number-at-pos (point-max)) 2)))
> > -       (goto-char beg)
> > -       (while (<= (line-number-at-pos) (min last-line max-line))
> > -         (notmuch-search-set-tags (delete-dups (sort (cons tag (notmuch-search-get-tags)) 'string<)))
> > -         (forward-line))))))
> > +See `notmuch-search-tag-region' for details."
> > +  (apply 'notmuch-search-tag-region (point) (point) tags))
> >
> > -(defun notmuch-search-remove-tag-thread (tag)
> > -  (notmuch-search-remove-tag-region tag (point) (point)))
> > +(defun notmuch-search-tag-region (beg end &rest tags)
> > +  "Change tags for threads in the given region.
> >
> > -(defun notmuch-search-remove-tag-region (tag beg end)
> > -  (let ((search-id-string (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or ")))
> > -    (notmuch-tag search-id-string (concat "-" tag))
> > +`Tags' is a list of tag operations for \"notmuch tag\", i.e. a
> > +list of tags to change with '+' and '-' prefixes.  The tags are
> > +added or removed for all threads in the region from `beg' to
> > +`end'."
> > +  (let ((search-string (notmuch-search-find-thread-id-region-search beg end)))
> > +    (apply 'notmuch-tag search-string tags)
> >     (save-excursion
> >       (let ((last-line (line-number-at-pos end))
> >            (max-line (- (line-number-at-pos (point-max)) 2)))
> >        (goto-char beg)
> >        (while (<= (line-number-at-pos) (min last-line max-line))
> > -         (notmuch-search-set-tags (delete tag (notmuch-search-get-tags)))
> > +         (notmuch-search-set-tags
> > +          (notmuch-update-tags (notmuch-search-get-tags) tags))
> >          (forward-line))))))
> >
> > -(defun notmuch-search-add-tag (tag)
> > -  "Add a tag to the currently selected thread or region.
> > -
> > -The tag is added to all messages in the currently selected thread
> > -or threads in the current region."
> > -  (interactive
> > -   (list (notmuch-select-tag-with-completion "Tag to add: ")))
> > -  (save-excursion
> > -    (if (region-active-p)
> > -       (let* ((beg (region-beginning))
> > -              (end (region-end)))
> > -         (notmuch-search-add-tag-region tag beg end))
> > -      (notmuch-search-add-tag-thread tag))))
> > -
> > -(defun notmuch-search-remove-tag (tag)
> > -  "Remove a tag from the currently selected thread or region.
> > +(defun notmuch-search-tag (&optional initial-input)
> > +  "Change tags for the currently selected thread or region."
> > +  (interactive)
> > +  (let* ((beg (if (region-active-p) (region-beginning) (point)))
> > +        (end (if (region-active-p) (region-end) (point)))
> > +        (search-string (notmuch-search-find-thread-id-region-search beg end))
> > +        (tags (notmuch-select-tags-with-completion initial-input search-string)))
> > +    (apply 'notmuch-search-tag-region beg end tags)))
> > +
> > +(defun notmuch-search-add-tag ()
> > +  "Same as `notmuch-search-tag' but sets initial input to '+'."
> > +  (interactive)
> > +  (notmuch-search-tag "+"))
> >
> > -The tag is removed from all messages in the currently selected
> > -thread or threads in the current region."
> > -  (interactive
> > -   (list (notmuch-select-tag-with-completion
> > -         "Tag to remove: "
> > -         (if (region-active-p)
> > -             (mapconcat 'identity
> > -                        (notmuch-search-find-thread-id-region (region-beginning) (region-end))
> > -                        " ")
> > -           (notmuch-search-find-thread-id)))))
> > -  (save-excursion
> > -    (if (region-active-p)
> > -       (let* ((beg (region-beginning))
> > -              (end (region-end)))
> > -         (notmuch-search-remove-tag-region tag beg end))
> > -      (notmuch-search-remove-tag-thread tag))))
> > +(defun notmuch-search-remove-tag ()
> > +  "Same as `notmuch-search-tag' but sets initial input to '-'."
> > +  (interactive)
> > +  (notmuch-search-tag "-"))
> >
> >  (defun notmuch-search-archive-thread ()
> >   "Archive the currently selected thread (remove its \"inbox\" tag).
> >
> >  This function advances the next thread when finished."
> >   (interactive)
> > -  (notmuch-search-remove-tag-thread "inbox")
> > +  (notmuch-search-tag-thread "-inbox")
> >   (notmuch-search-next-thread))
> >
> >  (defvar notmuch-search-process-filter-data nil
> > @@ -893,9 +896,7 @@ will prompt for tags to be added or removed. Tags prefixed with
> >  Each character of the tag name may consist of alphanumeric
> >  characters as well as `_.+-'.
> >  "
> > -  (interactive (notmuch-select-tags-with-completion
> > -               "Operations (+add -drop): notmuch tag "
> > -               '("+" "-")))
> > +  (interactive (notmuch-select-tags-with-completion))
> >   (apply 'notmuch-tag notmuch-search-query-string actions))
> >
> >  (defun notmuch-search-buffer-title (query)
> > --
> > 1.7.8.3
> >
> > _______________________________________________
> > notmuch mailing list
> > [hidden email]
> > http://notmuchmail.org/mailman/listinfo/notmuch

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

[Austin Clements]

One philosophical nit below, but not enough to hold this up.

Quoth Dmitry Kurochkin on Jan 28 at  8:41 am:

> Before the change, tag format validation was done in
> `notmuch-search-operate-all' function only.  The patch moves it down
> to `notmuch-tag', so that all users of that function get input
> validation.
> ---
>  emacs/notmuch.el |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 72f78ed..84d7d0a 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -522,6 +522,12 @@ Note: Other code should always use this function alter tags of
>  messages instead of running (notmuch-call-notmuch-process \"tag\" ..)
>  directly, so that hooks specified in notmuch-before-tag-hook and
>  notmuch-after-tag-hook will be run."
> +  ;; Perform some validation
> +  (when (null tags) (error "No tags given"))

Since this is a non-interactive function and hence is meant to be
invoked programmatically, I would expect it to accept zero tags.
Unlike the following check, this is a UI-level check and thus, I
believe, belongs in interactive functions (even if that requires a
little duplication).

> +  (mapc (lambda (tag)
> +  (unless (string-match-p "^[-+][-+_.[:word:]]+$" tag)
> +    (error "Tag must be of the form `+this_tag' or `-that_tag'")))
> + tags)
>    (run-hooks 'notmuch-before-tag-hook)
>    (apply 'notmuch-call-notmuch-process
>   (append (list "tag") tags (list "--" query)))
> @@ -890,12 +896,6 @@ characters as well as `_.+-'.
>    (interactive (notmuch-select-tags-with-completion
>   "Operations (+add -drop): notmuch tag "
>   '("+" "-")))
> -  ;; Perform some validation
> -  (when (null actions) (error "No operations given"))
> -  (mapc (lambda (action)
> -  (unless (string-match-p "^[-+][-+_.[:word:]]+$" action)
> -    (error "Action must be of the form `+this_tag' or `-that_tag'")))
> - actions)
>    (apply 'notmuch-tag notmuch-search-query-string actions))
>  
>  (defun notmuch-search-buffer-title (query)

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

[Dmitry Kurochkin]

Hi Austin.

On Sun, 29 Jan 2012 16:34:27 -0500, Austin Clements <[hidden email]> wrote:

> One philosophical nit below, but not enough to hold this up.
>
> Quoth Dmitry Kurochkin on Jan 28 at  8:41 am:
> > Before the change, tag format validation was done in
> > `notmuch-search-operate-all' function only.  The patch moves it down
> > to `notmuch-tag', so that all users of that function get input
> > validation.
> > ---
> >  emacs/notmuch.el |   12 ++++++------
> >  1 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index 72f78ed..84d7d0a 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -522,6 +522,12 @@ Note: Other code should always use this function alter tags of
> >  messages instead of running (notmuch-call-notmuch-process \"tag\" ..)
> >  directly, so that hooks specified in notmuch-before-tag-hook and
> >  notmuch-after-tag-hook will be run."
> > +  ;; Perform some validation
> > +  (when (null tags) (error "No tags given"))
>
> Since this is a non-interactive function and hence is meant to be
> invoked programmatically, I would expect it to accept zero tags.
> Unlike the following check, this is a UI-level check and thus, I
> believe, belongs in interactive functions (even if that requires a
> little duplication).
>

Agreed.  Though I would hate to add the same check to each tag
operation.  Perhaps this check can go to
`notmuch-select-tags-with-completion'?

This is not the main patch in the series.  So I think I would prefer not
to make v2 because of this issue.  If we come up with a good (i.e. no
duplication) solution, I will prepare a separate patch for it.

Regards,
  Dmitry

> > +  (mapc (lambda (tag)
> > +  (unless (string-match-p "^[-+][-+_.[:word:]]+$" tag)
> > +    (error "Tag must be of the form `+this_tag' or `-that_tag'")))
> > + tags)
> >    (run-hooks 'notmuch-before-tag-hook)
> >    (apply 'notmuch-call-notmuch-process
> >   (append (list "tag") tags (list "--" query)))
> > @@ -890,12 +896,6 @@ characters as well as `_.+-'.
> >    (interactive (notmuch-select-tags-with-completion
> >   "Operations (+add -drop): notmuch tag "
> >   '("+" "-")))
> > -  ;; Perform some validation
> > -  (when (null actions) (error "No operations given"))
> > -  (mapc (lambda (action)
> > -  (unless (string-match-p "^[-+][-+_.[:word:]]+$" action)
> > -    (error "Action must be of the form `+this_tag' or `-that_tag'")))
> > - actions)
> >    (apply 'notmuch-tag notmuch-search-query-string actions))
> >  
> >  (defun notmuch-search-buffer-title (query)

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

[Austin Clements]

I'm looking forward to having this.  I think it'll streamline tagging
operations.

Quoth Dmitry Kurochkin on Jan 28 at  8:41 am:
> Before the change, "+" and "-" tagging operations in notmuch-search
> and notmuch-show views accepted only a single tag.  The patch makes
> them use the recently added `notmuch-select-tags-with-completion'
> function, which allows to enter multiple tags with "+" and "-"
> prefixes.  So after the change, "+" and "-" bindings allow to both add
> and remove multiple tags.  The only difference between "+" and "-" is
> the minibuffer initial input ("+" and "-" respectively).

This patch was a little difficult to review because it was largish and
the diff happened to contain a bunch of forward references.  If it's
convenient (don't bother if it's not), could you divide up the next
version a little?  Something simple like sending the show changes as a
separate patch would probably make it a lot easier.

> ---
>  emacs/notmuch-show.el |   65 +++++++------------
>  emacs/notmuch.el      |  165 +++++++++++++++++++++++++------------------------
>  2 files changed, 107 insertions(+), 123 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 84ac624..03eadfb 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -38,8 +38,9 @@
>  
>  (declare-function notmuch-call-notmuch-process "notmuch" (&rest args))
>  (declare-function notmuch-fontify-headers "notmuch" nil)
> -(declare-function notmuch-select-tag-with-completion "notmuch" (prompt &rest search-terms))
> +(declare-function notmuch-select-tags-with-completion "notmuch" (&optional initial-input &rest search-terms))
>  (declare-function notmuch-search-show-thread "notmuch" nil)
> +(declare-function notmuch-update-tags "notmuch" (current-tags changed-tags))
>  
>  (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
>    "Headers that should be shown in a message, in this order.
> @@ -1267,7 +1268,7 @@ Some useful entries are:
>  
>  (defun notmuch-show-mark-read ()
>    "Mark the current message as read."
> -  (notmuch-show-remove-tag "unread"))
> +  (notmuch-show-tag-message "-unread"))
>  
>  ;; Functions for getting attributes of several messages in the current
>  ;; thread.
> @@ -1470,51 +1471,33 @@ than only the current message."
>      (message (format "Command '%s' exited abnormally with code %d"
>       shell-command exit-code))))))))
>  
> -(defun notmuch-show-add-tags-worker (current-tags add-tags)
> -  "Add to `current-tags' with any tags from `add-tags' not
> -currently present and return the result."
> -  (let ((result-tags (copy-sequence current-tags)))
> -    (mapc (lambda (add-tag)
> -    (unless (member add-tag current-tags)
> -      (setq result-tags (push add-tag result-tags))))
> -    add-tags)
> -    (sort result-tags 'string<)))
> -
> -(defun notmuch-show-del-tags-worker (current-tags del-tags)
> -  "Remove any tags in `del-tags' from `current-tags' and return
> -the result."
> -  (let ((result-tags (copy-sequence current-tags)))
> -    (mapc (lambda (del-tag)
> -    (setq result-tags (delete del-tag result-tags)))
> -  del-tags)
> -    result-tags))
> -
> -(defun notmuch-show-add-tag (&rest toadd)
> -  "Add a tag to the current message."
> -  (interactive
> -   (list (notmuch-select-tag-with-completion "Tag to add: ")))
> +(defun notmuch-show-tag-message (&rest changed-tags)
> +  "Change tags for the current message.
>  
> +`Changed-tags' is a list of tag operations for \"notmuch tag\",
> +i.e. a list of tags to change with '+' and '-' prefixes."

Ticks in a docstring indicate functions (and will be hyperlinked as
such by describe-function).  Typically, argument names are indicated
by writing them in all caps.

Also, it probably makes more sense to reference `notmuch-tag' than
"notmuch tag", since this is Lisp land (and, since that will be
helpfully hyperlinked, you probably don't need to explain changed-tags
here).

>    (let* ((current-tags (notmuch-show-get-tags))
> - (new-tags (notmuch-show-add-tags-worker current-tags toadd)))
> -
> + (new-tags (notmuch-update-tags current-tags changed-tags)))
>      (unless (equal current-tags new-tags)
> -      (apply 'notmuch-tag (notmuch-show-get-message-id)
> -     (mapcar (lambda (s) (concat "+" s)) toadd))
> +      (apply 'notmuch-tag (notmuch-show-get-message-id) changed-tags)
>        (notmuch-show-set-tags new-tags))))
>  
> -(defun notmuch-show-remove-tag (&rest toremove)
> -  "Remove a tag from the current message."
> -  (interactive
> -   (list (notmuch-select-tag-with-completion
> -  "Tag to remove: " (notmuch-show-get-message-id))))
> +(defun notmuch-show-tag (&optional initial-input)
> +  "Change tags for the current message, read input from the minibuffer."
> +  (interactive)
> +  (let ((changed-tags (notmuch-select-tags-with-completion
> +       initial-input (notmuch-show-get-message-id))))
> +    (apply 'notmuch-show-tag-message changed-tags)))
>  
> -  (let* ((current-tags (notmuch-show-get-tags))
> - (new-tags (notmuch-show-del-tags-worker current-tags toremove)))
> +(defun notmuch-show-add-tag ()
> +  "Same as `notmuch-show-tag' but sets initial input to '+'."
> +  (interactive)
> +  (notmuch-show-tag "+"))
>  
> -    (unless (equal current-tags new-tags)
> -      (apply 'notmuch-tag (notmuch-show-get-message-id)
> -     (mapcar (lambda (s) (concat "-" s)) toremove))
> -      (notmuch-show-set-tags new-tags))))
> +(defun notmuch-show-remove-tag ()
> +  "Same as `notmuch-show-tag' but sets initial input to '-'."
> +  (interactive)
> +  (notmuch-show-tag "-"))

Should notmuch-show-{add,remove}-tag be considered public functions?
Previously, they were amenable to creating bindings for adding or
removing individual tags, and I believe people have done this.  If
we're okay with breaking backward-compatibility, there should at least
be a NEWS item explaining how to convert such custom bindings to use
notmuch-show-tag-message.

>  
>  (defun notmuch-show-toggle-headers ()
>    "Toggle the visibility of the current message headers."
> @@ -1559,7 +1542,7 @@ argument, hide all of the messages."
>  (defun notmuch-show-archive-thread-internal (show-next)
>    ;; Remove the tag from the current set of messages.
>    (goto-char (point-min))
> -  (loop do (notmuch-show-remove-tag "inbox")
> +  (loop do (notmuch-show-tag-message "-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))
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index ff46617..24b0ea3 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -76,38 +76,56 @@ For example:
>  (defvar notmuch-query-history nil
>    "Variable to store minibuffer history for notmuch queries")
>  
> -(defun notmuch-tag-completions (&optional prefixes search-terms)
> -  (let ((tag-list
> - (split-string
> -  (with-output-to-string
> -    (with-current-buffer standard-output
> -      (apply 'call-process notmuch-command nil t
> -     nil "search-tags" search-terms)))
> -  "\n+" t)))
> -    (if (null prefixes)
> - tag-list
> -      (apply #'append
> -     (mapcar (lambda (tag)
> -       (mapcar (lambda (prefix)
> - (concat prefix tag)) prefixes))
> -     tag-list)))))
> +(defun notmuch-tag-completions (&optional search-terms)
> +  (split-string
> +   (with-output-to-string
> +     (with-current-buffer standard-output
> +       (apply 'call-process notmuch-command nil t
> +      nil "search-tags" search-terms)))
> +   "\n+" t))
>  
>  (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
> -  (let ((tag-list (notmuch-tag-completions nil search-terms)))
> +  (let ((tag-list (notmuch-tag-completions search-terms)))
>      (completing-read prompt tag-list)))
>  
> -(defun notmuch-select-tags-with-completion (prompt &optional prefixes &rest search-terms)
> -  (let ((tag-list (notmuch-tag-completions prefixes search-terms))
> - (crm-separator " ")
> - ;; By default, space is bound to "complete word" function.
> - ;; Re-bind it to insert a space instead.  Note that <tab>
> - ;; still does the completion.
> - (crm-local-completion-map
> - (let ((map (make-sparse-keymap)))
> -   (set-keymap-parent map crm-local-completion-map)
> -   (define-key map " " 'self-insert-command)
> -   map)))
> -    (delete "" (completing-read-multiple prompt tag-list))))
> +(defun notmuch-select-tags-with-completion (&optional initial-input &rest search-terms)

I don't know if notmuch-select-tags-with-completion is the right name
for this now that it hard-codes the +/- prefixes (which seems like the
right thing to do, BTW).  Maybe notmuch-read-tags-add-remove?

> +  (let* ((add-tag-list (mapcar (apply-partially 'concat "+")
> +       (notmuch-tag-completions)))
> + (remove-tag-list (mapcar (apply-partially 'concat "-")
> +  (notmuch-tag-completions search-terms)))

This will make two calls to notmuch search, but often one will
suffice.  It's probably worth optimizing the case were search-terms is
nil.

> + (tag-list (append add-tag-list remove-tag-list))
> + (crm-separator " ")
> + ;; By default, space is bound to "complete word" function.
> + ;; Re-bind it to insert a space instead.  Note that <tab>
> + ;; still does the completion.
> + (crm-local-completion-map
> +  (let ((map (make-sparse-keymap)))
> +    (set-keymap-parent map crm-local-completion-map)
> +    (define-key map " " 'self-insert-command)
> +    map)))
> +    (delete "" (completing-read-multiple
> + "Operations (+add -drop): notmuch tag " tag-list nil

I don't think the "notmuch tag" part is necessary.  From the
perspective of a person who only uses the Emacs UI, this will be
meaningless.  Maybe "Tag changes (+add -drop): " or even just "Tags
(+add -drop): " since the "+add -drop" part implies what you're doing.

> + nil initial-input))))
> +
> +(defun notmuch-update-tags (current-tags changed-tags)

Maybe just "tags" instead of "current-tags"?  Nothing says they have
to be current.  It's just a list of tags.

Also, changed-tags makes it sound like a list of tags, which is isn't.
Maybe tag-changes?

> +  "Update `current-tags' with `changed-tags' and return the result.
> +
> +`Changed-tags' is a list of tag operations given to \"notmuch
> +tag\", i.e. a list of changed tags with '+' and '-' prefixes."

Same comment about ticks and "notmuch tag".

I found this docstring a bit confusing.  I wasn't sure exactly what it
meant to "update current-tags with changed-tags" (though replacing
changed-tags with tag-changes would probably help).  Plus, this
function does not, in fact, update current-tags.  Maybe,

  "Return a copy of TAGS with additions and removals from TAG-CHANGES.

TAG-CHANGES must be a list of tags names, each prefixed with either a
\"+\" to indicate the tag should be added to TAGS if not present or a
\"-\" to indicate that the tag should be removed from TAGS if
present."

> +  (let ((result-tags (copy-sequence current-tags)))
> +    (mapc (lambda (changed-tag)

Consider dolist instead of mapc, though this is a matter of taste.  It
leads to less indentation (and does have precedent in the notmuch
code, though mapc is more common).

Too bad Elisp doesn't have fold.

> +    (unless (string= changed-tag "")
> +      (let ((op (substring changed-tag 0 1))
> +    (tag (substring changed-tag 1)))
> + (cond ((string= op "+")
> +       (unless (member tag result-tags)
> + (push tag result-tags)))
> +      ((string= op "-")
> +       (setq result-tags (delete tag result-tags)))
> +      (t
> +       (error "Changed tag must be of the form `+this_tag' or `-that_tag'"))))))

I would suggest case instead of cond, but, again, that's a matter of
taste.

> +       changed-tags)
> +    (sort result-tags 'string<)))
>  
>  (defun notmuch-foreach-mime-part (function mm-handle)
>    (cond ((stringp (car mm-handle))
> @@ -447,6 +465,10 @@ Complete list of currently available key bindings:
>    "Return a list of threads for the current region"
>    (notmuch-search-properties-in-region 'notmuch-search-thread-id beg end))
>  
> +(defun notmuch-search-find-thread-id-region-search (beg end)
> +  "Return a search string for threads for the current region"
> +  (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or "))
> +
>  (defun notmuch-search-find-authors ()
>    "Return the authors for the current thread"
>    (get-text-property (point) 'notmuch-search-authors))
> @@ -590,74 +612,55 @@ the messages that were tagged"
>   (forward-line 1))
>        output)))
>  
> -(defun notmuch-search-add-tag-thread (tag)
> -  (notmuch-search-add-tag-region tag (point) (point)))
> +(defun notmuch-search-tag-thread (&rest tags)
> +  "Change tags for the currently selected thread.
>  
> -(defun notmuch-search-add-tag-region (tag beg end)
> -  (let ((search-id-string (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or ")))
> -    (notmuch-tag search-id-string (concat "+" tag))
> -    (save-excursion
> -      (let ((last-line (line-number-at-pos end))
> -    (max-line (- (line-number-at-pos (point-max)) 2)))
> - (goto-char beg)
> - (while (<= (line-number-at-pos) (min last-line max-line))
> -  (notmuch-search-set-tags (delete-dups (sort (cons tag (notmuch-search-get-tags)) 'string<)))
> -  (forward-line))))))
> +See `notmuch-search-tag-region' for details."
> +  (apply 'notmuch-search-tag-region (point) (point) tags))
>  
> -(defun notmuch-search-remove-tag-thread (tag)
> -  (notmuch-search-remove-tag-region tag (point) (point)))
> +(defun notmuch-search-tag-region (beg end &rest tags)
> +  "Change tags for threads in the given region.
>  
> -(defun notmuch-search-remove-tag-region (tag beg end)
> -  (let ((search-id-string (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or ")))
> -    (notmuch-tag search-id-string (concat "-" tag))
> +`Tags' is a list of tag operations for \"notmuch tag\", i.e. a
> +list of tags to change with '+' and '-' prefixes.  The tags are
> +added or removed for all threads in the region from `beg' to
> +`end'."

Same comment about ticks and "notmuch tag".

> +  (let ((search-string (notmuch-search-find-thread-id-region-search beg end)))
> +    (apply 'notmuch-tag search-string tags)
>      (save-excursion
>        (let ((last-line (line-number-at-pos end))
>      (max-line (- (line-number-at-pos (point-max)) 2)))
>   (goto-char beg)
>   (while (<= (line-number-at-pos) (min last-line max-line))
> -  (notmuch-search-set-tags (delete tag (notmuch-search-get-tags)))
> +  (notmuch-search-set-tags
> +   (notmuch-update-tags (notmuch-search-get-tags) tags))
>    (forward-line))))))
>  
> -(defun notmuch-search-add-tag (tag)
> -  "Add a tag to the currently selected thread or region.
> -
> -The tag is added to all messages in the currently selected thread
> -or threads in the current region."
> -  (interactive
> -   (list (notmuch-select-tag-with-completion "Tag to add: ")))
> -  (save-excursion
> -    (if (region-active-p)
> - (let* ((beg (region-beginning))
> -       (end (region-end)))
> -  (notmuch-search-add-tag-region tag beg end))
> -      (notmuch-search-add-tag-thread tag))))
> -
> -(defun notmuch-search-remove-tag (tag)
> -  "Remove a tag from the currently selected thread or region.
> +(defun notmuch-search-tag (&optional initial-input)
> +  "Change tags for the currently selected thread or region."
> +  (interactive)
> +  (let* ((beg (if (region-active-p) (region-beginning) (point)))
> + (end (if (region-active-p) (region-end) (point)))

While you're in here, these should probably be `use-region-p'.

> + (search-string (notmuch-search-find-thread-id-region-search beg end))
> + (tags (notmuch-select-tags-with-completion initial-input search-string)))
> +    (apply 'notmuch-search-tag-region beg end tags)))
> +
> +(defun notmuch-search-add-tag ()
> +  "Same as `notmuch-search-tag' but sets initial input to '+'."
> +  (interactive)
> +  (notmuch-search-tag "+"))
>  
> -The tag is removed from all messages in the currently selected
> -thread or threads in the current region."
> -  (interactive
> -   (list (notmuch-select-tag-with-completion
> -  "Tag to remove: "
> -  (if (region-active-p)
> -      (mapconcat 'identity
> - (notmuch-search-find-thread-id-region (region-beginning) (region-end))
> - " ")
> -    (notmuch-search-find-thread-id)))))
> -  (save-excursion
> -    (if (region-active-p)
> - (let* ((beg (region-beginning))
> -       (end (region-end)))
> -  (notmuch-search-remove-tag-region tag beg end))
> -      (notmuch-search-remove-tag-thread tag))))
> +(defun notmuch-search-remove-tag ()
> +  "Same as `notmuch-search-tag' but sets initial input to '-'."
> +  (interactive)
> +  (notmuch-search-tag "-"))
>  
>  (defun notmuch-search-archive-thread ()
>    "Archive the currently selected thread (remove its \"inbox\" tag).
>  
>  This function advances the next thread when finished."
>    (interactive)
> -  (notmuch-search-remove-tag-thread "inbox")
> +  (notmuch-search-tag-thread "-inbox")
>    (notmuch-search-next-thread))
>  
>  (defvar notmuch-search-process-filter-data nil
> @@ -893,9 +896,7 @@ will prompt for tags to be added or removed. Tags prefixed with
>  Each character of the tag name may consist of alphanumeric
>  characters as well as `_.+-'.
>  "
> -  (interactive (notmuch-select-tags-with-completion
> - "Operations (+add -drop): notmuch tag "
> - '("+" "-")))
> +  (interactive (notmuch-select-tags-with-completion))
>    (apply 'notmuch-tag notmuch-search-query-string actions))
>  
>  (defun notmuch-search-buffer-title (query)

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

[Austin Clements]

Quoth Dmitry Kurochkin on Jan 28 at  8:41 am:
> After the recent tagging operations changes, functions bound to "+"
> and "-" in notmuch-search and notmuch-show views always read input
> from the minibuffer.  Use kbd macros instead of calling them directly.

Should this be folded into the previous patch so these tests aren't
temporarily broken?
_______________________________________________
notmuch mailing list
[hidden email]
http://notmuchmail.org/mailman/listinfo/notmuch

[Austin Clements]

Quoth Dmitry Kurochkin on Jan 28 at  8:41 am:

> Some tag-related operations accept a single tag without prefix
> (`notmuch-select-tag-with-completion'), others accept multiple tags
> prefixed with '+' or '-' (`notmuch-select-tags-with-completion').
> Before the change, both functions used a single default minibuffer
> history.  This is inconvenient because you have to skip options with
> incompatible format when going through the history.  The patch adds
> separate history lists for the two functions.  Note that functions
> that accept the same input format (e.g. "+", "-", "*") share the
> history list as before.
> ---
>  emacs/notmuch.el |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index 24b0ea3..9813e0a 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -76,6 +76,14 @@ For example:
>  (defvar notmuch-query-history nil
>    "Variable to store minibuffer history for notmuch queries")
>  
> +(defvar notmuch-select-tag-history nil
> +  "Variable to store notmuch tag history for
> +  `notmuch-select-tag-with-completion'.")
> +
> +(defvar notmuch-select-tags-history nil
> +  "Variable to store notmuch tags history for
> +  `notmuch-select-tags-with-completion'.")
> +

Really these are minibuffer or input histories, not "notmuch tag
history".  Also, the second line shouldn't be indented.  (Definitely
nits, but if you roll a new version, you might as well fix these.)

>  (defun notmuch-tag-completions (&optional search-terms)
>    (split-string
>     (with-output-to-string
> @@ -86,7 +94,7 @@ For example:
>  
>  (defun notmuch-select-tag-with-completion (prompt &rest search-terms)
>    (let ((tag-list (notmuch-tag-completions search-terms)))
> -    (completing-read prompt tag-list)))
> +    (completing-read prompt tag-list nil nil nil 'notmuch-select-tag-history)))
>  
>  (defun notmuch-select-tags-with-completion (&optional initial-input &rest search-terms)
>    (let* ((add-tag-list (mapcar (apply-partially 'concat "+")
> @@ -105,7 +113,7 @@ For example:
>      map)))
>      (delete "" (completing-read-multiple
>   "Operations (+add -drop): notmuch tag " tag-list nil
> - nil initial-input))))
> + nil initial-input 'notmuch-select-tags-history))))
>  
>  (defun notmuch-update-tags (current-tags changed-tags)
>    "Update `current-tags' with `changed-tags' and return the result.

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