From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 4F6FA41488C for ; Sun, 8 Jan 2012 17:09:07 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.799 X-Spam-Level: X-Spam-Status: No, score=-0.799 tagged_above=-999 required=5 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6sIsgKGbirmL for ; Sun, 8 Jan 2012 17:09:06 -0800 (PST) Received: from mail-gx0-f181.google.com (mail-gx0-f181.google.com [209.85.161.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id 267BF41522B for ; Sun, 8 Jan 2012 17:09:06 -0800 (PST) Received: by ggnq2 with SMTP id q2so1904257ggn.26 for ; Sun, 08 Jan 2012 17:09:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=from:to:subject:in-reply-to:references:user-agent:date:message-id :mime-version:content-type:content-transfer-encoding; bh=oELaoowe3Adzw0XL6eZaoP8ugZv55BbXzZIscHWf9ng=; b=lnQP45QX57w5dOzuaEOoG430QkfySR1r6nNIh8VNHbISSWgBNvYHwWjggE3HSz4XmV yhu/P513Zq7k0CuxiXM1JwdcY2Wvl86UaxENSqjGt8kCW3WpDi5rxro/ilNR/X0z2HQh WhuLK0iRhIf+GDxJ0yiXKmgziqZYZQlk38c8I= Received: by 10.101.23.12 with SMTP id a12mr5970071anj.17.1326071344402; Sun, 08 Jan 2012 17:09:04 -0800 (PST) Received: from localhost (c-68-80-94-73.hsd1.pa.comcast.net. [68.80.94.73]) by mx.google.com with ESMTPS id y58sm99970823yhi.17.2012.01.08.17.09.01 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 08 Jan 2012 17:09:03 -0800 (PST) From: Aaron Ecay To: Jameson Graef Rollins , Notmuch Mail Subject: Re: [PATCH 2/4] emacs: repurpose notmuch-show-archive-thread-internal function for general thread tagging In-Reply-To: <1325975294-646-3-git-send-email-jrollins@finestructure.net> References: <1325975294-646-1-git-send-email-jrollins@finestructure.net> <1325975294-646-2-git-send-email-jrollins@finestructure.net> <1325975294-646-3-git-send-email-jrollins@finestructure.net> User-Agent: Notmuch/0.10.1+56~gd709fd6 (http://notmuchmail.org) Emacs/24.0.92.3 (i386-apple-darwin10.8.0) Date: Sun, 08 Jan 2012 20:08:59 -0500 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Jan 2012 01:09:08 -0000 Jameson, Some comments below: On Sat, 7 Jan 2012 14:28:12 -0800, Jameson Graef Rollins wrote: > Instead of having a function that is only used for archiving a thread, > we instead make it useful for any tagging operation. The new > function, notmuch-show-tag-thread-internal, now takes two more > arguments, for the "sign" of the tagging operation ("-" or "+"), and > the tag to be added or removed. This will allow this function to be > used for any generic thread tagging operation. >=20 > The higher level functions that call this function are modified > accordingly. > --- > emacs/notmuch-show.el | 34 ++++++++++++++++++++-------------- > 1 files changed, 20 insertions(+), 14 deletions(-) >=20 > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el > index 5502efd..1e16f05 100644 > --- a/emacs/notmuch-show.el > +++ b/emacs/notmuch-show.el > @@ -1414,20 +1414,26 @@ argument, hide all of the messages." > (interactive) > (backward-button 1)) >=20=20 > -(defun notmuch-show-archive-thread-internal (show-next) > +(defun notmuch-show-tag-thread-internal (sign tag show-next) A couple of comments on the arguments: - It would be good to make show-next &optional. This will enable code to call the fn with only two arguments, and not showing next will be the default behavior. - A more lispy way of specifying the sign would be to use a boolean. Perhaps you could call this =E2=80=9Cremove=E2=80=9D; a value o= f =E2=80=98t=E2=80=99 would remove the tag; =E2=80=98nil=E2=80=99 would add it. Moving this argument= after =E2=80=98tag=E2=80=99 and also making it &optional woudl allow this fn to be called with one arg to add a tag. (Maybe this is too minimalist and API, however.)=20 > ;; Remove the tag from the current set of messages. > (goto-char (point-min)) > - (loop do (notmuch-show-remove-tag "inbox") > - until (not (notmuch-show-goto-message-next))) > - ;; Move to the next item in the search results, if any. > - (let ((parent-buffer notmuch-show-parent-buffer)) > - (notmuch-kill-this-buffer) > - (if parent-buffer > - (progn > - (switch-to-buffer parent-buffer) > - (forward-line) > - (if show-next > - (notmuch-search-show-thread)))))) > + (let ((tag-function)) No second set of parens is needed around tag-function. > + (cond > + ((string=3D sign "-") > + (setq tag-function 'notmuch-show-remove-tag)) > + ((string=3D sign "+") > + (setq tag-function 'notmuch-show-add-tag))) > + (loop do (funcall tag-function tag) > + until (not (notmuch-show-goto-message-next))) > + ;; Move to the next item in the search results, if any. Does it make sense to separate the tagging and the movement? It seems plausible that some code somewhere might want to add/remove a tag from all messages in the thread w/o changing the display. > + (let ((parent-buffer notmuch-show-parent-buffer)) > + (notmuch-kill-this-buffer) > + (if parent-buffer > + (progn > + (switch-to-buffer parent-buffer) > + (forward-line) > + (if show-next > + (notmuch-search-show-thread))))))) --=20 Aaron Ecay