Re: [PATCH 7/8] emacs: modify show tag functions to use new notmuch-tag interface

2012-04-09 Thread Jameson Graef Rollins
On Sat, Apr 07 2012, Mark Walters markwalters1...@gmail.com wrote:
 I think this is what is making the two tests fail: they count the number
 of invocations of notmuch and in case there is one invocation of notmuch
 show and one of notmuch tag -unread message-id, where before it was just
 the single notmuch show.

Good call, Mark.  After a bit of testing it looks like that is what's
going on.  I was confused, since I had thought that the call to
notmuch-show should have involved two notmuch calls originally as well,
one for retrieving the message and the other removing the unread tag.
However, it appears the messages in those tests don't have unread tags
after all.  Not sure why, but that explains it.

So I guess the upshot is that moving all the common prompting and tag
validation stuff into notmuch-tag means that in certain cases there will
be extra notmuch calls, even if no tags are changed.  Is that a problem?

What I can do, though, is add extra validation to notmuch-tag to not
actually call notmuch tag, or any of the pre- and post- tagging hooks,
if no tags are changing.  This will still require one call to notmuch to
retrieve the current set of tags for the query, but at least it wont tag
or call the hooks if nothing is changing.  That seems reasonable to me,
but please let me know if you think it's not.

I've pasted below a new version of notmuch-tag that addresses these
issues.  Let me know what you think, and I'll resubmit the series.

jamie.


(defun notmuch-tag (query optional tag-changes)
  Add/remove tags in TAG-CHANGES to messages matching QUERY.

QUERY should be a string containing the search-terms.
TAG-CHANGES can take multiple forms.  If TAG-CHANGES is a list of
strings of the form \+tag\ or \-tag\ then those are the tag
changes applied.  If TAG-CHANGES is a string then it is
interpreted as a single tag change.  If TAG-CHANGES is the string
\-\ or \+\, or null, then the user is prompted to enter the
tag changes.

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
  (if (string-or-null-p tag-changes)
  (if (or (string= tag-changes -) (string= tag-changes +) (null 
tag-changes))
  (setq tag-changes (notmuch-read-tag-changes tag-changes query))
(setq tag-changes (list tag-changes
  (mapc (lambda (tag-change)
  (unless (string-match-p ^[-+]\\S-+$ tag-change)
(error Tag must be of the form `+this_tag' or `-that_tag')))
tag-changes)
  (let* ((current-tags (notmuch-tag-completions (list query)))
 (new-tags (notmuch-update-tags current-tags tag-changes)))
(if (equal current-tags new-tags)
;; if no tags are changing, return nil
nil
  (run-hooks 'notmuch-before-tag-hook)
  (apply 'notmuch-call-notmuch-process tag
 (append tag-changes (list -- query)))
  (run-hooks 'notmuch-after-tag-hook)
  ;; otherwise, return the list of actual changed tags
  tag-changes)))


pgpoGB9js6jsH.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH 7/8] emacs: modify show tag functions to use new notmuch-tag interface

2012-04-07 Thread Mark Walters

On Sun, 08 Apr 2012, Jameson Graef Rollins jroll...@finestructure.net wrote:
 The main change here is to modify argument parsing so as to not force
 tag-changes to be a list, and to let notmuch-tag handle prompting the
 user when required.  doc strings are also updated and cleaned up.
 ---
  emacs/notmuch-show.el |   26 +++---
  1 files changed, 15 insertions(+), 11 deletions(-)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index a4c313d..69bca02 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -1641,22 +1641,26 @@ TAG-CHANGES is a list of tag operations for 
 `notmuch-tag'.
(let* ((current-tags (notmuch-show-get-tags))
(new-tags (notmuch-update-tags current-tags tag-changes)))
  (unless (equal current-tags new-tags)
 -  (apply 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
 +  (funcall 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
(notmuch-show-set-tags new-tags
  
 -(defun notmuch-show-tag (optional initial-input)
 -  Change tags for the current message, read input from the minibuffer.
 +(defun notmuch-show-tag (optional tag-changes)
 +  Change tags for the current message.
 +
 +See `notmuch-tag' for information on the format of TAG-CHANGES.
(interactive)
 -  (let ((tag-changes (notmuch-read-tag-changes
 -   initial-input (notmuch-show-get-message-id
 -(apply 'notmuch-show-tag-message tag-changes)))
 +  (setq tag-changes (funcall 'notmuch-tag (notmuch-show-get-message-id) 
 tag-changes))
 +  (let* ((current-tags (notmuch-show-get-tags))
 +  (new-tags (notmuch-update-tags current-tags tag-changes)))
 +(unless (equal current-tags new-tags)
 +  (notmuch-show-set-tags new-tags

Hi. If I am following this correctly the setq line funcalls notmuch tag
regardless of whether there will be a tag change.
 
whereas the code from patch 8/8
-(defun notmuch-show-tag-message (rest tag-changes)
-  Change tags for the current message.
-
-TAG-CHANGES is a list of tag operations for `notmuch-tag'.
-  (let* ((current-tags (notmuch-show-get-tags))
-   (new-tags (notmuch-update-tags current-tags tag-changes)))
-(unless (equal current-tags new-tags)
-  (funcall 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
-  (notmuch-show-set-tags new-tags
-

seems to only do the funcall when the tags change.

I think this is what is making the two tests fail: they count the number
of invocations of notmuch and in case there is one invocation of notmuch
show and one of notmuch tag -unread message-id, where before it was just
the single notmuch show.

Best wishes

Mark
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch