1 Return-Path: <racin@free.fr>
\r
2 X-Original-To: notmuch@notmuchmail.org
\r
3 Delivered-To: notmuch@notmuchmail.org
\r
4 Received: from localhost (localhost [127.0.0.1])
\r
5 by olra.theworths.org (Postfix) with ESMTP id 5358F431FBF
\r
6 for <notmuch@notmuchmail.org>; Wed, 24 Feb 2010 16:00:20 -0800 (PST)
\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
\r
11 X-Spam-Status: No, score=-1.501 tagged_above=-999 required=5 tests=[AWL=1.098,
\r
12 BAYES_00=-2.599] autolearn=ham
\r
13 Received: from olra.theworths.org ([127.0.0.1])
\r
14 by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)
\r
15 with ESMTP id CVObnzTnPjk4 for <notmuch@notmuchmail.org>;
\r
16 Wed, 24 Feb 2010 16:00:18 -0800 (PST)
\r
17 Received: from smtp5-g21.free.fr (smtp5-g21.free.fr [212.27.42.5])
\r
18 by olra.theworths.org (Postfix) with ESMTP id 97B61431FAE
\r
19 for <notmuch@notmuchmail.org>; Wed, 24 Feb 2010 16:00:16 -0800 (PST)
\r
20 Received: from smtp5-g21.free.fr (localhost [127.0.0.1])
\r
21 by smtp5-g21.free.fr (Postfix) with ESMTP id 823BAD48069;
\r
22 Thu, 25 Feb 2010 01:00:06 +0100 (CET)
\r
23 Received: from zimbra1-e1.priv.proxad.net (zimbra1-e1.priv.proxad.net
\r
25 by smtp5-g21.free.fr (Postfix) with ESMTP id 6B055D48049;
\r
26 Thu, 25 Feb 2010 01:00:04 +0100 (CET)
\r
27 Date: Thu, 25 Feb 2010 01:00:04 +0100 (CET)
\r
29 To: Carl Worth <cworth@cworth.org>
\r
31 <484265811.5160231267056003988.JavaMail.root@zimbra1-e1.priv.proxad.net>
\r
32 In-Reply-To: <87tyt6wjtf.fsf@yoom.home.cworth.org>
\r
34 Content-Type: text/plain; charset=utf-8
\r
35 Content-Transfer-Encoding: 7bit
\r
36 X-Originating-IP: [82.239.207.166]
\r
37 X-Mailer: Zimbra 5.0 (ZimbraWebClient - FF3.0
\r
38 (Linux)/5.0.15_GA_2815.UBUNTU8_64)
\r
39 X-Authenticated-User: racin@free.fr
\r
40 Cc: notmuch@notmuchmail.org
\r
41 Subject: Re: [notmuch] [PATCH] Support for deletion (patch included)
\r
42 X-BeenThere: notmuch@notmuchmail.org
\r
43 X-Mailman-Version: 2.1.13
\r
45 List-Id: "Use and development of the notmuch mail system."
\r
46 <notmuch.notmuchmail.org>
\r
47 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
\r
48 <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
\r
49 List-Archive: <http://notmuchmail.org/pipermail/notmuch>
\r
50 List-Post: <mailto:notmuch@notmuchmail.org>
\r
51 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
\r
52 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
\r
53 <mailto:notmuch-request@notmuchmail.org?subject=subscribe>
\r
54 X-List-Received-Date: Thu, 25 Feb 2010 00:00:20 -0000
\r
58 > Could you also write a commit message describing what the patch does?
\r
59 > The easiest way for me to apply that would be if you would create a git
\r
60 > commit, then run "git format-patch origin/master" and mail the resulting
\r
61 > files, (the "git send-email" command can be used here, or you can insert
\r
62 > the files into a mail-composition buffer and modify them as needed).
\r
65 OK, here it is (comments below). I had trouble splitting the patches into a patch series; I
\r
66 found git add -p, but isn't there a better interface for selecting patches?
\r
68 >From bdee9558d93bffb97c80632f522288e059deb7c2 Mon Sep 17 00:00:00 2001
\r
69 From: Matthieu Lemerre <racin@racin.rez-gif.supelec.fr>
\r
70 Date: Thu, 25 Feb 2010 00:24:24 +0100
\r
71 Subject: [PATCH 1/2] Add and use notmuch-show-forall-in-thread macro
\r
74 notmuch.el | 17 +++++++++++------
\r
75 1 files changed, 11 insertions(+), 6 deletions(-)
\r
77 diff --git a/notmuch.el b/notmuch.el
\r
78 index 6482170..5d7342a 100644
\r
81 @@ -321,17 +321,22 @@ pseudoheader summary"
\r
82 (cons (notmuch-show-get-message-id) nil)))
\r
83 (notmuch-show-set-tags (sort (set-difference tags toremove :test 'string=) 'string<))))))
\r
85 -(defun notmuch-show-archive-thread-maybe-mark-read (markread)
\r
87 +(defmacro notmuch-show-forall-in-thread (&rest body)
\r
88 + "Executes BODY with point in all messages of the current thread."
\r
90 (goto-char (point-min))
\r
93 - (notmuch-show-remove-tag "unread" "inbox")
\r
94 - (notmuch-show-remove-tag "inbox"))
\r
98 (if (not (re-search-forward notmuch-show-message-begin-regexp nil t))
\r
99 - (goto-char (point-max)))))
\r
100 + (goto-char (point-max))))))
\r
102 +(defun notmuch-show-archive-thread-maybe-mark-read (markread)
\r
103 + (notmuch-show-forall-in-thread
\r
105 + (notmuch-show-remove-tag "unread" "inbox")
\r
106 + (notmuch-show-remove-tag "inbox")))
\r
107 (let ((parent-buffer notmuch-show-parent-buffer))
\r
114 This first patch is helpful for factorizing out code. Basically, it allows to
\r
115 apply a "message-only" command to all the thread.
\r
117 >From 0073152e3fa7dd11d88de28e87eec7762cdbbbeb Mon Sep 17 00:00:00 2001
\r
118 From: Matthieu Lemerre <racin@racin.rez-gif.supelec.fr>
\r
119 Date: Thu, 25 Feb 2010 00:25:51 +0100
\r
120 Subject: [PATCH 2/2] Add support for deletion in the emacs interface
\r
123 notmuch.el | 56 +++++++++++++++++++++++++++++++++++++++++++++++++-------
\r
124 1 files changed, 49 insertions(+), 7 deletions(-)
\r
126 diff --git a/notmuch.el b/notmuch.el
\r
127 index 5d7342a..0285573 100644
\r
131 (define-key map "x" 'notmuch-show-archive-thread-then-exit)
\r
132 (define-key map "A" 'notmuch-show-mark-read-then-archive-thread)
\r
133 (define-key map "a" 'notmuch-show-archive-thread)
\r
134 + (define-key map "d" 'notmuch-show-delete-thread)
\r
135 + (define-key map "D" 'notmuch-show-delete-message)
\r
136 (define-key map "p" 'notmuch-show-previous-message)
\r
137 (define-key map "N" 'notmuch-show-mark-read-then-next-open-message)
\r
138 (define-key map "n" 'notmuch-show-next-message)
\r
139 @@ -380,6 +382,23 @@ buffer."
\r
140 (notmuch-show-archive-thread)
\r
141 (kill-this-buffer))
\r
143 +(defun notmuch-show-delete-message ()
\r
144 + "Delete current message (sets its deleted tag)."
\r
146 + (notmuch-show-add-tag "deleted"))
\r
148 +(defun notmuch-show-delete-thread()
\r
149 + "Delete each message in thread."
\r
151 + (notmuch-show-forall-in-thread
\r
152 + (notmuch-show-delete-message)))
\r
154 +(defun notmuch-show-delete-thread-and-exit()
\r
155 + "Delete each message in thread, then exit back to search results."
\r
157 + (notmuch-show-delete-thread)
\r
158 + (kill-this-buffer))
\r
160 (defun notmuch-show-mark-read-then-archive-then-exit ()
\r
161 "Remove unread tags from thread, then archive and exit to search results."
\r
163 @@ -1227,6 +1246,7 @@ matching this search term are shown if non-nil. "
\r
164 (define-key map [mouse-1] 'notmuch-search-show-thread)
\r
165 (define-key map "*" 'notmuch-search-operate-all)
\r
166 (define-key map "a" 'notmuch-search-archive-thread)
\r
167 + (define-key map "d" 'notmuch-search-delete-thread)
\r
168 (define-key map "-" 'notmuch-search-remove-tag)
\r
169 (define-key map "+" 'notmuch-search-add-tag)
\r
170 (define-key map (kbd "RET") 'notmuch-search-show-thread)
\r
171 @@ -1235,6 +1255,7 @@ matching this search term are shown if non-nil. "
\r
172 (fset 'notmuch-search-mode-map notmuch-search-mode-map)
\r
174 (defvar notmuch-search-query-string)
\r
175 +(defvar notmuch-search-history nil)
\r
176 (defvar notmuch-search-oldest-first t
\r
177 "Show the oldest mail first in the search-mode")
\r
179 @@ -1446,6 +1467,13 @@ This function advances the next thread when finished."
\r
180 (notmuch-search-remove-tag "inbox")
\r
183 +(defun notmuch-search-delete-thread ()
\r
184 + "Mark the currently selected thread as deleted (set its \"deleted\" tag).
\r
185 +This function advances the next thread when finished."
\r
187 + (notmuch-search-add-tag "deleted")
\r
190 (defun notmuch-search-process-sentinel (proc msg)
\r
191 "Add a message to let user know when \"notmuch search\" exits"
\r
192 (let ((buffer (process-buffer proc))
\r
193 @@ -1520,10 +1548,22 @@ characters as well as `_.+-'.
\r
194 (append action-split (list notmuch-search-query-string) nil))))
\r
197 -(defun notmuch-search (query &optional oldest-first)
\r
198 - "Run \"notmuch search\" with the given query string and display results."
\r
199 - (interactive "sNotmuch search: ")
\r
200 - (let ((buffer (get-buffer-create (concat "*notmuch-search-" query "*"))))
\r
201 +(defun notmuch-search (query &optional oldest-first include-deleted)
\r
202 + "Run \"notmuch search\" with the given query string and display results.
\r
204 +With prefix argument, include deleted items.
\r
206 + (interactive (let* ((prefix current-prefix-arg)
\r
207 + (query (if prefix
\r
208 + (read-string "Notmuch search (including deleted): "
\r
209 + notmuch-search-query-string
\r
210 + 'notmuch-search-history)
\r
211 + (read-string "Notmuch search: " nil
\r
212 + 'notmuch-search-history))))
\r
213 + (list query nil prefix)))
\r
214 + (let ((real-query (if include-deleted query
\r
215 + (concat "not tag:deleted and (" query ")")))
\r
216 + (buffer (get-buffer-create (concat "*notmuch-search-" query "*"))))
\r
217 (switch-to-buffer buffer)
\r
218 (notmuch-search-mode)
\r
219 (set 'notmuch-search-query-string query)
\r
220 @@ -1539,7 +1579,7 @@ characters as well as `_.+-'.
\r
221 (let ((proc (start-process-shell-command
\r
222 "notmuch-search" buffer notmuch-command "search"
\r
223 (if oldest-first "--sort=oldest-first" "--sort=newest-first")
\r
224 - (shell-quote-argument query))))
\r
225 + (shell-quote-argument real-query))))
\r
226 (set-process-sentinel proc 'notmuch-search-process-sentinel)
\r
227 (set-process-filter proc 'notmuch-search-process-filter))))
\r
228 (run-hooks 'notmuch-search-hook)))
\r
229 @@ -1587,7 +1627,7 @@ search."
\r
231 Runs a new search matching only messages that match both the
\r
232 current search results AND the additional query string provided."
\r
233 - (interactive "sFilter search: ")
\r
234 + (interactive "sFilter search:")
\r
235 (let ((grouped-query (if (string-match-p notmuch-search-disjunctive-regexp query) (concat "( " query " )") query)))
\r
236 (notmuch-search (concat notmuch-search-query-string " and " grouped-query) notmuch-search-oldest-first)))
\r
238 @@ -1630,7 +1670,9 @@ current search results AND that are tagged with the given tag."
\r
240 (fset 'notmuch-folder-mode-map notmuch-folder-mode-map)
\r
242 -(defcustom notmuch-folders (quote (("inbox" . "tag:inbox") ("unread" . "tag:unread")))
\r
243 +(defcustom notmuch-folders (quote (("inbox" . "tag:inbox")
\r
244 + ("unread" . "tag:unread")
\r
245 + ("deleted" . "tag:deleted")))
\r
246 "List of searches for the notmuch folder view"
\r
247 :type '(alist :key-type (string) :value-type (string))
\r
253 This second patch is the reworked patch, after your comments.
\r
255 > A couple of minor comments on the patch:
\r
257 > > (define-key map "a" 'notmuch-search-archive-thread)
\r
258 > > + (define-key map "d" 'notmuch-search-mark-as-deleted)
\r
260 > For consistency, let's name this notmuch-search-delete-thread.
\r
262 > And we'll probably want a notmuch-show-delete-message function as well,
\r
265 Did both. I have however a question: how to handle in the UI the
\r
266 thread/message distinction? For now, I set "d" in notmuch-show buffer
\r
267 to act as "delete the current thread", and "D" as "delete the current
\r
268 message". I thought that as "d" meant in notmuch summary "delete the
\r
269 current thread", the bindings would be more consistent. But now I
\r
270 wonder if this may be counter intuitive, and if we should use "d" for
\r
271 "delete current message" and "D" for "delete current thread". What do
\r
274 It would be nice to have, in general a convenient way to handle the
\r
275 thread/message distinction in the UI (e.g. for applying other tags).
\r
278 > > +(defvar notmuch-search-history nil)
\r
280 > Excellent! I've wanted custom search history for a while, and just
\r
281 > didn't know how to do it with (interactive "s"). It looks easy enough
\r
282 > with read-string as you're doing here. But this is independent
\r
283 > functionality, so would be preferred as an independent patch/commit.
\r
285 In fact I needed this; that way when you do "C-u s", the previous
\r
286 search is shown, and to include deleted items you just have to do "C-u
\r
287 s RET". So I made them in a single commit; but I can split them if
\r
288 you really want to.
\r
291 > > (forward-line))
\r
294 > > +(defun notmuch-search-mark-as-deleted ()
\r
295 > > + "Mark the currently selected thread as deleted (set its \"deleted\" tag).
\r
296 > > +This function advances the next thread when finished."
\r
297 > > + (interactive)
\r
298 > > + (notmuch-search-add-tag "deleted")
\r
299 > > + (forward-line))
\r
302 > > (defun notmuch-search-process-sentinel (proc msg)
\r
304 > Watch that extra whitespace. The convention is a single line of
\r
305 > whitespace between each function.
\r
308 My bad. I did not see this in my review. Corrected this in the updated patch.
\r
310 > And should we also archive the thread before removing the deleted tag?
\r
312 I don't understand this. Do you mean: should we also archive the
\r
313 thread before setting the deleted tag? I don't known why we should
\r
314 bother to do it, as the tag is not show by the next results.
\r
316 I would tend to think that archiving should be an explicit action,
\r
317 that should call a hook. For instance, I'd like my mailflow to be like
\r
320 - get mail with offlineimap => a maildir directory, indexed by notmuch
\r
322 - mails stay in inbox until I archive it. That way I can still access
\r
325 - archiving puts an "archived" tag (and removes the inbox and unread
\r
326 tags), and move the mail to another directory (or uses git to
\r
329 - deleting puts a deleted tag (but the messages stays inbox as long as
\r
330 I do not explicitely remove this).
\r
332 The main reason is that many mail servers only allow small quotas, so
\r
333 you have to carefully select which mail should stay inbox (only the
\r
334 mail that you need external access).
\r
336 In brief, this mail flow needs to delete threads but not to archive
\r
337 it. I think that we could provide hooks, so that you can archive
\r
338 mails when you delete them if you like to.
\r
340 By the way in my patch commentary, you said that you planned to change
\r
341 the "inbox" and "unread" treatments to be more explicit. Do you still
\r
342 plan to do that? I can maybe also provide a patch.
\r
344 > > +With prefix argument, include deleted items.
\r
346 > That's a pretty good interface I think.
\r
348 > Another approach would be to do something like what sup does---that
\r
349 > would be to scan the search terms and if it contains "tag:deleted" and
\r
350 > all then don't prepend the "not tag:deleted and" to the search
\r
351 > string. The assumption there is that if the user is explicitly
\r
352 > mentioning the deleted tag, then we should just rely on the user to
\r
353 > explicitly describe how the tag should be treated.
\r
355 > That's perhaps not an unreasonable heuristic, and might be done even in
\r
356 > addition to the prefix-argument approach. But that could be an
\r
357 > additional commit, and I won't require it.
\r
359 Hmm, that's what I did want to do initially. In that case, I think
\r
360 that notmuch should be able to output parse trees from notmuch search
\r
361 strings as JSON; this would help do the parsing in the various
\r
364 I also had this idea that may solve the "tag should be
\r
365 orthogonal/command-line UI problem". We observe that "archived" is
\r
366 contrary to "inbox", "read" contrary to "unread", "deleted" contrary
\r
367 to "undeleted", and so on.
\r
369 So it may be a good idea to have "pseudo-tags", that would help in the
\r
370 interface (searching archived mails with search not tag:inbox is not
\r
371 very intuitive...), as synonyms for "not tag:countrary-tag"
\r
373 Then for each "pseudo-tag", notmuch could hard-code which is included
\r
374 by default. Adding the other tags is easy: just add "and tag:deleted"
\r
375 or "and tag:read" or "and tag:archived". Moreover, this would simplify
\r
376 UI implementation as well, without requiring parsing of the strings.
\r
378 Another, even simpler possibility would be to add a "--all" flag to
\r
379 notmuch. Without it, it would omit some of the tags (like the deleted
\r
380 ones), which would be convenient as a text interface. With it, it
\r
381 would show all tags, which would be convenient for use with scripts
\r
382 and UI, and mor in the "tags are orthogonal" spirit. Again, this would
\r
383 not require to parse search strings, and imho would be much cleaner.
\r
385 > > + (interactive (let* ((prefix current-prefix-arg)
\r
386 > > + (query (if prefix
\r
387 > > + (read-string "Notmuch search (including deleted): "
\r
388 > > + notmuch-search-query-string
\r
389 > > + 'notmuch-search-history)
\r
390 > > + (read-string "Notmuch search: " nil
\r
391 > > + 'notmuch-search-history))))
\r
393 > Why is the second (initial-input) argument non-nil in one case, but nil
\r
394 > in the other? The documentation for `read-string' says the argument is
\r
395 > deprecated and should be nil in all new code.
\r
397 The idea is that you do not have default search terms if you do
\r
398 regular searchs. But if you want to include deleted items, then you
\r
399 have the last search string. This way, you handle the common case when
\r
400 you serach for a term, do not find it, then decide to try again with
\r
403 Now, maybe the C-u interface is not such a good idea, and a toggle
\r
404 button would be better. Don't know, but I find this interface
\r
408 > > + (list query nil prefix)))
\r
409 > > + (let ((real-query (if include-deleted query
\r
410 > > + (concat "not tag:deleted and (" query ")")))
\r
411 > > + (buffer (get-buffer-create (concat "*notmuch-search-" query
\r
414 > Does the include-deleted case actually work? I don't see anything in the
\r
415 > code that sets this variable. (I'm just reviewing here--I haven't tested
\r
418 This is the way interactive work.
\r
419 (defun notmuch-search (query &optional oldest-first include-deleted)
\r
421 + (interactive (let* ((prefix current-prefix-arg)
\r
423 + (list query nil prefix)))
\r
425 The (list query nil prefix) at the end of the (interactive) form will
\r
426 set query to query, nil to oldest-first, and prefix to
\r
430 > > @@ -1351,7 +1374,6 @@ search."
\r
432 > > Runs a new search matching only messages that match both the
\r
433 > > current search results AND the additional query string provided."
\r
434 > > - (interactive "sFilter search: ")
\r
435 > > (let ((grouped-query (if (string-match-p notmuch-search-disjunctive-regexp query) (concat "( " query " )") query)))
\r
436 > > (notmuch-search (concat notmuch-search-query-string " and " grouped-query) notmuch-search-oldest-first)))
\r
438 > Is this just an accidental chunk in the patch? I don't see why this
\r
439 > function should become non-interactive now.
\r
441 My bad, corrected that also.
\r
projects / notmuch-archives.git / blob