From: Dirk Hohndel <hohndel@infradead.org>
To: Jameson Rollins <jrollins@finestructure.net>, notmuch@notmuchmail.org
Subject: Re: [PATCH] Add 'd'elete keybinding to search and show views
In-Reply-To: <87y6ggmlew.fsf@servo.finestructure.net>
References: <1271891763-10757-1-git-send-email-hohndel@infradead.org>
	<1271891763-10757-2-git-send-email-hohndel@infradead.org>
	<87y6ggmlew.fsf@servo.finestructure.net>
Date: Wed, 21 Apr 2010 17:20:41 -0700
Message-ID: <m3fx2oz6au.fsf@x200.gr8dns.org>
User-Agent: notmuch 0.2-52-ga28d2fe (Emacs 23.1.1/i386-redhat-linux-gnu)
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Wed, 21 Apr 2010 19:32:39 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> 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).

Great. 
 
> I only have a couple of nit-picky comments about your proposed
> implementation:

That's why I sent this out...
 
> 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).

It's the C programmer in me who hates code duplication. This way, if the
algorithm for walking the mails in the thread changes, you only fix it
once. But I see your point about weird options... in C I'd have
constants named THREAD_DELETE and THREAD_ARCHIVE_ONLY that I'd pass
around...

> 
> > +(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.

Well, that would take away a big reason for adding this -
convenience. In the end it's user experience design. The question is
(based on expected behavior) - do you expect a deleted email to stay
visible in your inbox. I don't - and I know that many people do.

So while I agree with the other conclusion in the thread that "deleted"
as tag for deleted messages doesn't have to be configurable - maybe this
feature wants to be configurable.

notmuch-archive-deleted or something like that

/D

-- 
Dirk Hohndel
Intel Open Source Technology Center