Re: [PATCH] Add 'd'elete keybinding to search and show views

Subject: Re: [PATCH] Add 'd'elete keybinding to search and show views

Date: Wed, 21 Apr 2010 19:32:39 -0400

To: Dirk Hohndel, notmuch@notmuchmail.org

Cc:

From: Jameson Rollins


On Wed, 21 Apr 2010 16:16:02 -0700, Dirk Hohndel <hohndel@infradead.org> wrote:
> Straight forward addition to the Emacs UI. The 'd' keybinding
> is implemented very similar to the 'a' keybinding - it only
> adds a +deleted tag as well. This tag is used by notmuchsync
> to delete (-p "prune") files in the mailstore.
> 
> I'm sending this mostly as an RFC - I use this and like it, but
> people seem to have strong feelings as to how they want to deal 
> with deleting email (or for some people, how they don't want to
> do that at all).

Hey, Dirk.  I'm definitely on board with this idea, and have in fact
been doing exactly the same thing with my personal customizations as you
propose (including using the 'd' key).

I only have a couple of nit-picky comments about your proposed
implementation:

On Wed, 21 Apr 2010 16:16:03 -0700, Dirk Hohndel <hohndel@infradead.org> wrote:
> -(defun notmuch-show-archive-thread-internal (show-next)
> +(defun notmuch-show-archive-or-delete-thread-internal (show-next delete)
>    ;; Remove the tag from the current set of messages.
>    (goto-char (point-min))
> -  (loop do (notmuch-show-remove-tag "inbox")
> +  (loop do (progn
> +	     (notmuch-show-remove-tag "inbox")
> +	     (if delete
> +		 (notmuch-show-add-tag "deleted")))
>  	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))
> @@ -925,6 +929,20 @@ to stdout or stderr will appear in the *Messages* buffer."
>  	  (if show-next
>  	      (notmuch-search-show-thread))))))

I'm not sure I like the idea of piggybacking on the
notmuch-show-archive-thread-internal function.  Why not just make a new
separate notmuch-show-delete-thread-internal function?  I think it makes
the code clearer and easier to parse for the calls to these functions
(otherwise it's a little unclear what "t t" and "nil nil" and so on
mean).

> +(defun notmuch-search-delete-thread ()
> +  "Delete the currently selected thread (remove its \"inbox\" tag and add \"deleted\" tag).
> +
> +This function advances the next thread when finished."
> +  (interactive)
> +  (notmuch-search-remove-tag-thread "inbox")
> +  (notmuch-search-add-tag-thread "deleted")
> +  (forward-line))

I'm also not a fan of these functions (notmuch-search-delete-thread and
notmuch-show-delete-thread) removing the "inbox" tag.  Just because I
want to mark a messages as deleted doesn't mean that I want to archive
it.  I would really like to keep these concepts distinct if possible.

jamie.
part-000.sig (application/pgp-signature)

Thread: