[PATCH v2 0/6] reply to sender

2012-01-09 Thread Dmitry Kurochkin
Hi Jani.

I prefer to leave the Emacs UI default reply behavior as is.

Changing it in CLI would not affect me, but I think the default should
be the same as in the Emacs UI.

Regards,
  Dmitry


[PATCH] man: add missing SEE ALSO header to notmuch reply man page

2012-01-09 Thread Xavier Maillard

On Sun,  8 Jan 2012 22:57:22 +0200, Jani Nikula  wrote:
> Signed-off-by: Jani Nikula 
> ---
>  man/man1/notmuch-reply.1 |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

+1

/Xavier


[PATCH] NEWS: add news entry for notmuch reply uninitialized variable bugfix

2012-01-09 Thread David Bremner
On Mon,  9 Jan 2012 11:49:56 +, Jani Nikula  wrote:
> ---
> 
> This is against release branch.
> ---

Pushed to release.

d


[PATCH 4/4] emacs: Use the new JSON reply format.

2012-01-09 Thread Adam Wolfe Gordon
Hi David,

Thanks for the review.  Most of the things you've suggested are easy
changes, and I think obvious improvements, so I'll change them for the
next version.  A bit of discussion on the more involved things below:

On Mon, Jan 9, 2012 at 01:50, David Edmondson  wrote:
> On Sun, ?8 Jan 2012 00:52:42 -0700, Adam Wolfe Gordon  
> wrote:
>> +(defun w3m-region (start end)) ;; From `w3m.el'.
>> +(defun notmuch-mua-quote-part (part)
>> + ?(with-temp-buffer
>> + ? ?(insert part)
>> + ? ?(message-mode)
>> + ? ?(fill-region (point-min) (point-max))
>> + ? ?(goto-char (point-min))
>> + ? ?(perform-replace "^" "> " nil t nil)
>> + ? ?(set-buffer-modified-p nil)
>> + ? ?(buffer-substring (point-min) (point-max
>
> Couldn't all of this be done directly in the reply buffer?

Indeed, it could, I just hadn't thought of it.  I'll do this for the
next version.

> Using w3m means that you should `require' it. What happens when a user
> doesn't have it? (Either the elisp or the command.)

This was my initial thought, but when I looked at notmuch-show.el,
which uses w3m features, I noticed that it doesn't have a require.  To
be clear, this patch requires w3m.el (not just the w3m binary), which
I don't think anything else in notmuch does.

In the previous version I had a customize variable specifying whether
to quote HTML parts, which meant that if the user could set the
customize variable to false and everything would work without w3m.el.
I'd like not to introduce a new prerequisite, so if there's a way to
make w3m.el optional that would be my preference.  Can you provide
some guidance on this?

-- 
Adam Wolfe Gordon


[PATCH 4/4] emacs: Use the new JSON reply format.

2012-01-09 Thread Adam Wolfe Gordon
On Sun, Jan 8, 2012 at 18:27, Aaron Ecay  wrote:
>> +(defun w3m-region (start end)) ;; From `w3m.el'.
>
> What is the purpose of the above line? ?If it is to make the compiler
> aware of the function, you should use ?declare-function? instead. ?Defun
> will erase the original definition of the w3m-region function.

Indeed, it's to make the compiler aware of the function.  I followed
the example of notmuch-show.el (defvar w3m-current-buffer, line 391),
but it sounds like declare-function is a better way to accomplish
this.  I haven't written a whole lot of emacs lisp, so thanks for the
tip.

Given David's comments about requiring w3m instead it might be moot,
but if not I'll make this change for the next version.


[PATCH 0/4] Quoting HTML-only emails in replies redux

2012-01-09 Thread Adam Wolfe Gordon
Thanks for the suggestions.  Specific comments inline:

On Sun, Jan 8, 2012 at 18:36, Aaron Ecay  wrote:
>> There should probably be some customize variables for this in emacs, to 
>> control
>> (for example) whether to quote HTML parts and whether to prefer HTML or
>> plaintext parts for quoting. Any suggestions for what should be customizable
>> would be appreciated.
>
> I think two variables should suffice: one (boolean) to control whether
> to quote standalone text/html parts, and one to control which parts of a
> multipart/alternative part to quote. ?The latter should take possible
> values 'text, 'html, and 'both. ?This requires the emacs reply
> functionality to distinguish between html parts that are part of a
> multipart/alternative and those which are not, which (AFAICT) the
> current code doesn?t do.

The first one I think is obvious, and easy to add.  My previous
version actually had such an option, but I didn't bother with it this
time in the interest of getting the patch out.  I'll add this in and
send a new version.

Regarding the second suggestion, you're right that the current code
doesn't differentiate between text parts that are part of a
multipart/alternative and ones that aren't.  However, it does only
include parts with inline disposition, so it shouldn't end up quoting
stuff intended as attachments.  My thinking in the current operation
was to keep most of the complexity (i.e. going through the MIME parts
to pick out the relevant ones) in the C code, so that it doesn't have
to be implemented in each client - and also so I didn't have to
implement it in emacs lisp ;-).  Do you have some ideas for a JSON
format that's more complex than the current one (to include the
information necessary for making decisions about what to quote), but
less complex than something like notmuch show --format=json (which
would require the client to descend the MIME tree completely to create
a reply)?

A third customization option I was thinking about is a way to control
the format of the first line of the body (On $date, $person wrote).  I
think it would be fairly simple to let people specify an arbitrary
format using any of the headers of the original message, so you would
set it to something like "On %date%, %from% wrote", or "Quoth %from%
(%date%)".  This might be particularly useful to users who correspond
normally in a language other than English.  Anyone have thoughts on
whether this is worth implementing?

> I haven?t tested the patch yet, but it looks very promising. ?Thanks!

Thanks for the suggestions - let me know if you have a chance to try it out.

-- 
Adam Wolfe Gordon


[ANNOUNCE] mutt with notmuch support

2012-01-09 Thread Jeremy Nickurak
FWIW, here's the patch I ended up using to play with this:

diff --git a/mutt_notmuch.c b/mutt_notmuch.c
index 2f21407..a07b1ba 100644
--- a/mutt_notmuch.c
+++ b/mutt_notmuch.c
@@ -636,11 +636,15 @@ char *nm_uri_from_query(CONTEXT *ctx, char *buf,
size_t bufsz)
?static notmuch_message_t *get_nm_message(notmuch_database_t *db, HEADER *hdr)
?{
? ? ? ? notmuch_message_t *msg;
+ ? ? ? notmuch_status_t r;
+
? ? ? ? char *id = nm_header_get_id(hdr);

? ? ? ? dprint(2, (debugfile, "nm: find message (%s)\n", id));

- ? ? ? msg = id && db ? notmuch_database_find_message(db, id) : NULL;
+ ? ? ? if (id && db) {
+ ? ? ? ? ? ? ? r = notmuch_database_find_message(db, id, );
+ ? ? ? }

? ? ? ? FREE();
? ? ? ? return msg;


is there a default reply function? (was: Re: [PATCH v2 5/6] emacs: bind 'r' to reply-to-sender and 'R' to reply-to-all)

2012-01-09 Thread Gregor Zattler
Hi Jeremy, notmuch -developers,
* Jeremy Nickurak  [08. Jan. 2012]:
> On Sun, Jan 8, 2012 at 14:48, Jani Nikula  wrote:
>> It seemed to me that most people wanted this, and nobody spoke for keeping
>> the old binding now that we have reply-to-sender. This as a separate patch
>> so it's easy to drop if needed.
> 
> FWIW, I generally prefer reply-all as the default. 

Actually I think there is no default at all.  The perception of a
default here is because two different functions are on "r" and
"R" key respectively while the "r" key feels as a default.

Other MUAs have different key bindings:

message mode has "r" as reply and "w" as "wide-reply"
mutt has "r" as reply and "g" as "group-reply" and "L" as list-reply

I prefer the mutt way of doing this with three different functions.

> In my experience,
> when a message is sent to a bunch of people, it's usually treated as a
> "forum" discussion where everybody wants to be in on
> everything. 

I think you are right for the majority of cases.  But last week I
got in trouble because I did exactly this.  Perhaps here it is
more important which mistake has more potential for disaster.
Both mistakes could be important but I think there is more
disaster potential in giving information to people who shouldn't
get it than the other way around.  In my perception an UI which clearly
distinguishes two or three reply-functions encourages to think
in advance about which to use.  But last week...

> Also,
> once you've started composing, it's much easier to delete people you
> don't want included than to add people who are no longer referenced in
> the new buffer at all.

True.

Ciao, Gregor
-- 
 -... --- .-. . -.. ..--.. ...-.-


nmbug changes

2012-01-09 Thread Justus Winter
Quoting David Bremner (2012-01-08 23:34:26)
>On Sun, 08 Jan 2012 16:24:06 -, Justus Winter <4winter at 
>informatik.uni-hamburg.de> wrote:
>
>> I'm having trouble with nmbug, I did follow the instructions in this
>> mail and in the wiki, but nmbug never adds tags to my notmuch database
>> (it is supposed to do that, right?).
>
>In case you just want to load the tags in the git repo into your notmuch
>database, try running "nmbug checkout".

That actually worked, but nmbug help says:

   checkout
   Update the notmuch database from git. This is mainly useful to
   discard your changes in notmuch relative to git.
[...]
   merge   Merge changes from FETCH_HEAD into HEAD, and load the result
   into notmuch.

That's why I assumed that merge is the command of choice in my
situation.

>It might be the instructions on "getting started" could be updated here?
>Perhaps someone who has gotten started more recently can chime in here.

If checkout is neccessary, it surely should be added to the wiki
page. Once I've gained some experience with nmbug I'll update the
wiki, though I wouldn't mind if someone beats me to it ;)

Justus


[PATCH] emacs: call "notmuch tag" only once when archiving a thread

2012-01-09 Thread Tomi Ollila
On Mon, 09 Jan 2012 08:41:15 +, Jani Nikula  wrote:
> On Sun, 8 Jan 2012 20:12:59 -0500, Austin Clements  
> wrote:
> > Quoth Aaron Ecay on Jan 08 at  7:56 pm:
> > > On Thu, 05 Jan 2012 22:32:16 +0200, Jani Nikula  
> > > wrote:
> > > 
> > > [...]
> > > 
> > > > In the show view it only modifies the messages that are currently
> > > > visible. This is to make sure you don't accidentally archive things that
> > > > have arrived after refreshing the buffer. I think this is safest.
> > > 
> > > Hmm.  Perhaps it would make sense to add a check in the search view that
> > > the thread being archived[1] has the same number of messages as it did
> > > when the buffer was constructed.  (The information on how many messages
> > > the thread has is in the buffer; we would then compare this to the result
> > > of ?notmuch count thread:000foo? when the user requests to archive.)  If
> > > the counts don?t match, the interface should show a message in the echo
> > > area and (probably) refuse to do the tagging.
> > 
> > That sounds like a clever workaround.
> 
> The downside is that there's still a race condition: you could get new
> messages between checking the number of messages in the thread and
> tagging. The window for error would be much smaller than now, but it's
> still there. (You could check afterwards if this happened, and notify
> the user, "ps, I just tagged N messages more than you intended"...)

And this could also be "false alarm" if the the new messages arrived
after tagging but before checking...

> 
> J.

Tomi


[PATCH] NEWS: add news entry for notmuch reply uninitialized variable bugfix

2012-01-09 Thread Jani Nikula
---

This is against release branch.
---
 NEWS |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 85ff65b..687154d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,8 +1,8 @@
 Notmuch 0.11 (201x-xx-xx)
 =

-New command-line features
--
+Command-Line Interface
+--

 Hooks

@@ -11,6 +11,12 @@ Hooks
   supports "pre-new" and "post-new" hooks that are run before and after
   importing new messages into the database.

+notmuch reply --decrypt bugfix
+
+  The "notmuch reply" command with --decrypt argument had a rarely
+  occurring bug that caused an encrypted message not to be decrypted
+  sometimes. This is now fixed.
+
 Performance
 ---

-- 
1.7.1



[PATCH] emacs: call "notmuch tag" only once when archiving a thread

2012-01-09 Thread Mark Walters

> There's been quite a bit of discussion on fixing this properly.  See,
> for example
> id:"CAH-f9WsPj=1Eu=g3sOePJgCTBFs6HrLdLq18xMEnJ8aZ00yCEg at mail.gmail.com".
> The gist is that we need to include message IDs (or document IDs) in
> the search output and use these in tagging operations, rather than the
> unstable thread:XXX queries.  Unfortunately, actually fixing this got
> stalled since adding this information the text format is a fool's
> errand (having been the fool, I can say this!), so we need to switch
> Emacs over to using the JSON search format first.  However, once
> that's done, it's a relatively simple change.

I will just mention a different possible solution to this problem. I
think it is probably more hassle than its worth but just in case someone
sees how to do it nicely. 

The idea is to try and put the race-free solution into the command-line
notmuch rather than the emacs part.

I will just consider one race to start with: the race in notmuch
search/notmuch tag. notmuch search gives us just the thread-ids and what
is in that thread can change. So we could ask for a "cookie" (some short
unique identifier) and notmuch itself would store the state of its
output (i.e., the thread-ids together with the message-ids and the
matched message-ids).

Then notmuch tag could be called with this cookie to say "apply this tag
to the threads as they were at cookie.

example use
notmuch search --cookie 
prints a cookie followed by the output of the search

notmuch tag --cookie=cookievalue +sometag matched-thread:id .. \
 all-thread:id .. 

and notmuch would replace the matched-thread:id by the (matching)
message-ids in thread it stored at time cookievalue and all-thread by
all the messages at time cookievalue.

The nice feature is that this makes it easy for all clients to be race
free (not just emacs). The backend has the information to hand: there is
no encoding and decoding (via text or JSON) so storing message-ids etc
is simpler.  It also means that if notmuch wants to use document-ids
internally it can: it does not become a public interface.

The downside is trying to do the argument parsing in c and we have do
some reference counting or something to delete the cookies state at some
point.

Anyway it's just a thought.

Best wishes

Mark




[PATCH v2] emacs: Helpers for notmuch developers.

2012-01-09 Thread David Edmondson
On Mon, 09 Jan 2012 06:38:54 -0400, David Bremner  wrote:
> > Oh. It's supposed to delete the existing branch. It did in my test. What
> > happens for you?
> 
> FWIW, I loaded notmuch-dev.el on top of current master.
> 
> The magit buffer shows 
> 
> $ git --no-pager checkout -b 
> review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results
>  master
> fatal: A branch named 
> 'review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results'
>  already exists.
> 
> The backtrace is 
> 
>   signal(error ("Git failed"))
>   error("Git failed")
>   magit-run*(("git" "--no-pager" "checkout" "-b" 
> "review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results"
>  "master"))
>   #[nil "\303\304B\n\"!\207" [magit-git-executable 
> magit-git-standard-options args magit-run* append] 4]()
>   magit-refresh-wrapper(#[nil "\303\304  B\n\"!\207" 
> [magit-git-executable magit-git-standard-options args magit-run* append] 4])
>   magit-run-git("checkout" "-b" 
> "review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results"
>  "master")
>   
> magit-create-branch("review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results"
>  "master")
>   
> notmuch-dev-create-branch("review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results")
>   (let ((default-directory notmuch-dev-temporary-repository-path)) 
> (notmuch-dev-checkout-master) (condition-case nil (notmuch-dev-delete-branch 
> patch-name) (error nil)) (notmuch-dev-create-branch patch-name) 
> (with-temp-file mbox-path (erase-buffer) (call-process notmuch-command nil t 
> nil "show" "--format=mbox" search-terms)) (magit-run* (list 
> magit-git-executable "am" mbox-path)) (magit-status 
> notmuch-dev-temporary-repository-path))
>   (let ((patch-name ...) (mbox-path ...)) 
> (notmuch-dev-make-temporary-repository) (let (...) 
> (notmuch-dev-checkout-master) (condition-case nil ... ...) 
> (notmuch-dev-create-branch patch-name) (with-temp-file mbox-path ... ...) 
> (magit-run* ...) (magit-status notmuch-dev-temporary-repository-path)))
>   notmuch-dev-review-patch("[PATCH] emacs: Don't signal an error when 
> reaching the end of the search results." 
> "id:\"1324370714-28545-1-git-send-email-dme at dme.org\"")
>   notmuch-dev-show-review-patch()
>   call-interactively(notmuch-dev-show-review-patch t nil)
>   execute-extended-command(nil)
>   call-interactively(execute-extended-command nil nil)

Could you try running
  (notmuch-dev-delete-branch 
"review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results")
when inside that repository please?
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120109/855010a1/attachment.pgp>


[PATCH v2 3/6] cli: add support for replying just to the sender in "notmuch reply"

2012-01-09 Thread Mark Walters
> 
> The reply-to-thread is a rare case anyway, regardless of reply-to-all or
> reply-to-sender, and even the current implementation does not gather all
> the recipients from all the messages. Try it out, it seems to me it does
> not quite do what you think it does.
> 
> IMHO it should use oldest-first rather than newest-first sort order, and
> I guess it should really go through the messages being replied to and
> get the recipients from all of them. It's just out of scope for this
> patch set to fix these. But if that gets fixed later, I would like the
> reply-to-sender for multiple messages follow the logic of single-reply,
> just with all the addresses put together.

I completely agree it's a rare case. Having tried the code (yours and
the original) it seems to be very strange:

In its current form it seems to be reply to people on newest message in
thread and include all the bodies with some spurious headers mixed
in. So your code is reply-to-sender applied to people on newest message
in thread, and include all the bodies with some slightly different
spurious headers mixed in.

I like your suggested solution.

Anyway it's a rare case; the current code is odd and yours is no odder
so I agree with leaving it for a later patch (if anyone cares enough!).

Best wishes

Mark



Python bindings for adoption

2012-01-09 Thread Jameson Graef Rollins
On Sun, 08 Jan 2012 16:16:06 -, Justus Winter <4winter at 
informatik.uni-hamburg.de> wrote:
> I've decided to step up as a new maintainer for the libnotmuch python
> bindings. I assume that I'll have to mail an ssh public key to someone
> for repository access, right?

That's great to hear, Justus.  Thanks for stepping up!

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120109/5dc7aa62/attachment.pgp>


[PATCH 1/3] pep8 fixes

2012-01-09 Thread Patrick Totzke
Quoting Sebastian Spaeth (2012-01-02 16:23:05)
>Hi, just tried to apply but it doesn't apply cleanly anymore, can you
>update the patch to latest master? 

no need: you already pushed these three patches on Dec 6, if i'm not mistaken.
best,
/p


[PATCH] emacs: call "notmuch tag" only once when archiving a thread

2012-01-09 Thread David Edmondson
On Mon, 09 Jan 2012 12:38:58 +0200, Tomi Ollila  wrote:
> > The downside is that there's still a race condition: you could get new
> > messages between checking the number of messages in the thread and
> > tagging. The window for error would be much smaller than now, but it's
> > still there. (You could check afterwards if this happened, and notify
> > the user, "ps, I just tagged N messages more than you intended"...)
> 
> And this could also be "false alarm" if the the new messages arrived
> after tagging but before checking...

Or another process could change the tags, meaning that the number stayed
the same but the set changed.

(Wh)
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120109/a2e2ea9d/attachment.pgp>


[PATCH v2 3/6] cli: add support for replying just to the sender in "notmuch reply"

2012-01-09 Thread Jani Nikula
On Sun, 08 Jan 2012 23:23:15 +, Mark Walters  
wrote:
> 
> I like this version (of the whole series) but have two queries. (Note I
> haven't actually tried it out yet: I have just been reading the code.)
> 
> > +   /* Force recipient type in reply-to-sender mode just in case replying to
> > +* user's own message finds recipients in Cc/Bcc fields only.
> > +*/
> > +   if (reply_all)
> > +   recipient_type = reply_to_map[i].recipient_type;
> > +   else
> > +   recipient_type = GMIME_RECIPIENT_TYPE_TO;
> > +
> > +   addr = add_recipients_for_string (reply, config, recipient_type,
> > + recipients, add_recipients);
> > +
> 
> Why force the recipient type?  I do not think notmuch should ever move
> bcc people onto a different header. I think that will bite someone when
> it leaks information.
> 
> I would be happy with either empty headers or with the people staying
> as bcc.
> 
> In the cc: case I have no preference (as those addresses are already
> public) but I would suggest being consistent with bcc so either empty
> headers or keep them in the cc line.

I think you're right. I was trying to clean up a corner case in
reply-to-self, but probably over did it. I'll remove the recipient type
forcing altogether. Then it'll keep addresses in the same header as they
were.

> > for (messages = notmuch_query_search_messages (query);
> > notmuch_messages_valid (messages);
> > notmuch_messages_move_to_next (messages))
> > {
> [...]
> >   (void)add_recipients_from_message (reply, config, message, reply_all);
> 
> Since the above logic is applied to each email individually I think
> working out the recipients when replying to multiple emails (e.g.,
> reply-to-sender on a thread) could be very confusing. Some of the people
> being replied to will have been senders, some recipients (it is very
> likely that the thread contains messages we sent), some could even be
> cc, bcc people. Personally, I would have no idea what to expect from
> reply-to-sender in this case.
> 
> (My personal choice would be not to allow notmuch-reply-to-sender if
> multiple messages are specified. But I can obtain that by unbinding "r"
> in the notmuch-search-mode-map keymap.)

The reply-to-thread is a rare case anyway, regardless of reply-to-all or
reply-to-sender, and even the current implementation does not gather all
the recipients from all the messages. Try it out, it seems to me it does
not quite do what you think it does.

IMHO it should use oldest-first rather than newest-first sort order, and
I guess it should really go through the messages being replied to and
get the recipients from all of them. It's just out of scope for this
patch set to fix these. But if that gets fixed later, I would like the
reply-to-sender for multiple messages follow the logic of single-reply,
just with all the addresses put together.


BR,
Jani.


[PATCH 4/4] emacs: Use the new JSON reply format.

2012-01-09 Thread David Edmondson
On Sun,  8 Jan 2012 00:52:42 -0700, Adam Wolfe Gordon <awg+notmuch at xvx.ca> 
wrote:
> +(defun w3m-region (start end)) ;; From `w3m.el'.
> +(defun notmuch-mua-quote-part (part)
> +  (with-temp-buffer
> +(insert part)
> +(message-mode)
> +(fill-region (point-min) (point-max))
> +(goto-char (point-min))
> +(perform-replace "^" "> " nil t nil)
> +(set-buffer-modified-p nil)
> +(buffer-substring (point-min) (point-max

Couldn't all of this be done directly in the reply buffer?

> +(defun notmuch-mua-parse-html-part (part)
> +  (with-temp-buffer
> +(insert part)
> +(w3m-region (point-min) (point-max))
> +(set-buffer-modified-p nil)
> +(buffer-substring (point-min) (point-max

Blank lines between functions are the house style (as much as there is
such a thing).

Using w3m means that you should `require' it. What happens when a user
doesn't have it? (Either the elisp or the command.)

>  (defun notmuch-mua-reply (query-string  sender)
> -  (let (headers
> +  (let (reply
> + original
> + headers
>   body
> - (args '("reply")))
> + (args '("reply" "--format=json")))

Initialised variables are generally shown before uninitialised. I know
that you didn't do this 'wrong' and that it's an unrelated change, but
it should be fixed. (To the people who say that should be a separate
patch I say 'meh' - life is too short.)

>  (if notmuch-show-process-crypto
>   (setq args (append args '("--decrypt"
>  (setq args (append args (list query-string)))
> -;; This make assumptions about the output of `notmuch reply', but
> -;; really only that the headers come first followed by a blank
> -;; line and then the body.
> +;; Get the reply object as JSON, and parse it into an elisp object.
>  (with-temp-buffer
>(apply 'call-process (append (list notmuch-command nil (list t t) nil) 
> args))
>(goto-char (point-min))
> -  (if (re-search-forward "^$" nil t)
> -   (save-excursion
> - (save-restriction
> -   (narrow-to-region (point-min) (point))
> -   (goto-char (point-min))
> -   (setq headers (mail-header-extract)
> -  (forward-line 1)
> -  (setq body (buffer-substring (point) (point-max
> +  (setq reply (aref (json-read) 0)))
> +
> +;; Get the list of headers
> +(setq headers (cdr (assq 'headers (assq 'reply reply
> +;; Construct the body of the reply.
> +(setq original (cdr (assq 'original reply)))

The scope of (at least) `headers' and `original' could be tightened by
moving them down to the following `let' (and converting it to `let*').

> +
> +;; Start with the prelude, based on the headers of the original message.
> +(let ((original-headers (cdr (assq 'headers original
> +  (setq body (format "On %s, %s wrote:\n"
> +  (cdr (assq 'date original-headers))
> +  (cdr (assq 'from original-headers)
> +
> +;; Extract the body parts and construct a reasonable quoted body.
> +(let* ((body-parts (cdr (assq 'body original)))
> +(find-parts (lambda (type) (delq nil (mapcar (lambda (part)
> +   (if (string= (cdr 
> (assq 'content-type part)) type)
> +   (cdr (assq 
> 'content part
> + body-parts

`find-parts' might be useful in other places. Maybe add it to notmuch-lib.el?

> +(plain-parts (apply find-parts '("text/plain")))
> +(html-parts (apply find-parts '("text/html"
> +  
> +  (if (not (null plain-parts))
> +   (mapc (lambda (part) (setq body (concat body (notmuch-mua-quote-part 
> part plain-parts)
> + (mapc (lambda (part) (setq body (concat body (notmuch-mua-quote-part 
> (notmuch-mua-parse-html-part part) html-parts)))

If you have an 'else' clause, why test '(if (not ..' ?

> +(setq body (concat body "\n"))
> + 

If it already ends with a carriage return, why do this?
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120109/f04f6f96/attachment.pgp>


[PATCH] emacs: call "notmuch tag" only once when archiving a thread

2012-01-09 Thread Jani Nikula
On Sun, 8 Jan 2012 20:12:59 -0500, Austin Clements  wrote:
> Quoth Aaron Ecay on Jan 08 at  7:56 pm:
> > On Thu, 05 Jan 2012 22:32:16 +0200, Jani Nikula  wrote:
> > 
> > [...]
> > 
> > > In the show view it only modifies the messages that are currently
> > > visible. This is to make sure you don't accidentally archive things that
> > > have arrived after refreshing the buffer. I think this is safest.
> > 
> > Hmm.  Perhaps it would make sense to add a check in the search view that
> > the thread being archived[1] has the same number of messages as it did
> > when the buffer was constructed.  (The information on how many messages
> > the thread has is in the buffer; we would then compare this to the result
> > of ?notmuch count thread:000foo? when the user requests to archive.)  If
> > the counts don?t match, the interface should show a message in the echo
> > area and (probably) refuse to do the tagging.
> 
> That sounds like a clever workaround.

The downside is that there's still a race condition: you could get new
messages between checking the number of messages in the thread and
tagging. The window for error would be much smaller than now, but it's
still there. (You could check afterwards if this happened, and notify
the user, "ps, I just tagged N messages more than you intended"...)

J.


[PATCH] emacs: Helpers for notmuch developers.

2012-01-09 Thread David Edmondson
On Sat, 07 Jan 2012 00:56:19 +0200, Tomi Ollila  wrote:
> What do you think of this approach related to your way cloning the
> repo and then deleting/creating the branch. Just that developer may
> mess with the repository contents and then there is tedious working
> tree cleanup to be done (especially those who are not so fluent using
> git).

This sounds good - I learn more about git all of the time. I'll try to
apply some of your suggestions.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120109/52699756/attachment.pgp>


[PATCH v2] emacs: Helpers for notmuch developers.

2012-01-09 Thread David Edmondson
On Sat, 07 Jan 2012 07:53:54 -0400, David Bremner  wrote:
> On Fri,  6 Jan 2012 10:03:19 +, David Edmondson  wrote:
> > ---
> > 
> > - Prefix the branch name with 'review/'
> > - Avoid `shell-command', which also results in better error reporting
> >   when 'git-am' fails.
> > 
> 
> One thing I noticed is that reviewing a patch for the second time fails
> because the branch already exists. I'm not sure if this is covered under
> "upkeep of the repo" but it is a little inconvenient.

Oh. It's supposed to delete the existing branch. It did in my test. What
happens for you?
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120109/b8ec2a5a/attachment.pgp>


[PATCH v2 0/6] reply to sender

2012-01-09 Thread David Edmondson
On Sun,  8 Jan 2012 23:48:29 +0200, Jani Nikula  wrote:
>   emacs: add support for replying just to the sender
>   emacs: bind 'r' to reply-to-sender and 'R' to reply-to-all

These both look fine to me.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120109/97ea1154/attachment.pgp>


[PATCH] emacs: call "notmuch tag" only once when archiving a thread

2012-01-09 Thread David Edmondson
On Sun, 8 Jan 2012 20:12:59 -0500, Austin Clements  wrote:
> ... so we need to switch Emacs over to using the JSON search format
> first.

Is anyone working on this? I made an attempt ages ago[1], but have not kept
it working.

Footnotes: 
[1]  id:"1291114825-3513-1-git-send-email-dme at dme.org"
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20120109/8468968e/attachment.pgp>


0.11 is frozen, please update NEWS

2012-01-09 Thread David Bremner
On Mon, 02 Jan 2012 07:58:16 -0400, David Bremner  wrote:
> 
> 
> I have tagged 0.11_rc2 and uploaded 0.11~rc2-1 to Debian experimental.
> This includes only the single non-doc commit, fixing a python bindings
> segfault. NEWS items are still solicited.

OK, I have tagged 0.11_rc3 and pushed 0.11~rc3-1 to Debian
experimental. 

I'm pretty determined to release this, after I get one more small NEWS
patch from Jani. I'd appreciate any feedback for either the release
branch or the debian experimental packages.

David


Python bindings for adoption

2012-01-09 Thread sebast...@sspaeth.de
Hurray, thanks Justus. That is much appreciated.
Spaetz



David Bremner  schrieb:

>On Sun, 08 Jan 2012 16:16:06 -, Justus Winter
><4winter at informatik.uni-hamburg.de> wrote:
>> 
>> I've decided to step up as a new maintainer for the libnotmuch python
>> bindings. I assume that I'll have to mail an ssh public key to
>someone
>> for repository access, right?
>> 
>
>That's right. Carl (in copy) handles repository access.
>
>d



[PATCH v2] emacs: Helpers for notmuch developers.

2012-01-09 Thread David Bremner
On Mon, 09 Jan 2012 08:23:53 +, David Edmondson  wrote:
> On Sat, 07 Jan 2012 07:53:54 -0400, David Bremner  
> wrote:
> > On Fri,  6 Jan 2012 10:03:19 +, David Edmondson  wrote:
> > > ---
> > > 
> > > - Prefix the branch name with 'review/'
> > > - Avoid `shell-command', which also results in better error reporting
> > >   when 'git-am' fails.
> > > 
> > 
> > One thing I noticed is that reviewing a patch for the second time fails
> > because the branch already exists. I'm not sure if this is covered under
> > "upkeep of the repo" but it is a little inconvenient.
> 
> Oh. It's supposed to delete the existing branch. It did in my test. What
> happens for you?

FWIW, I loaded notmuch-dev.el on top of current master.

The magit buffer shows 

$ git --no-pager checkout -b 
review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results
 master
fatal: A branch named 
'review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results'
 already exists.

The backtrace is 

  signal(error ("Git failed"))
  error("Git failed")
  magit-run*(("git" "--no-pager" "checkout" "-b" 
"review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results"
 "master"))
  #[nil "\303\304  B\n\"!\207" [magit-git-executable 
magit-git-standard-options args magit-run* append] 4]()
  magit-refresh-wrapper(#[nil "\303\304B\n\"!\207" 
[magit-git-executable magit-git-standard-options args magit-run* append] 4])
  magit-run-git("checkout" "-b" 
"review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results"
 "master")
  
magit-create-branch("review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results"
 "master")
  
notmuch-dev-create-branch("review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results")
  (let ((default-directory notmuch-dev-temporary-repository-path)) 
(notmuch-dev-checkout-master) (condition-case nil (notmuch-dev-delete-branch 
patch-name) (error nil)) (notmuch-dev-create-branch patch-name) (with-temp-file 
mbox-path (erase-buffer) (call-process notmuch-command nil t nil "show" 
"--format=mbox" search-terms)) (magit-run* (list magit-git-executable "am" 
mbox-path)) (magit-status notmuch-dev-temporary-repository-path))
  (let ((patch-name ...) (mbox-path ...)) 
(notmuch-dev-make-temporary-repository) (let (...) 
(notmuch-dev-checkout-master) (condition-case nil ... ...) 
(notmuch-dev-create-branch patch-name) (with-temp-file mbox-path ... ...) 
(magit-run* ...) (magit-status notmuch-dev-temporary-repository-path)))
  notmuch-dev-review-patch("[PATCH] emacs: Don't signal an error when reaching 
the end of the search results." "id:\"1324370714-28545-1-git-send-email-dme at 
dme.org\"")
  notmuch-dev-show-review-patch()
  call-interactively(notmuch-dev-show-review-patch t nil)
  execute-extended-command(nil)
  call-interactively(execute-extended-command nil nil)


[PATCH 2/4] emacs: repurpose notmuch-show-archive-thread-internal function for general thread tagging

2012-01-09 Thread Aaron Ecay
On Sun, 08 Jan 2012 18:49:56 -0800, Jameson Graef Rollins  wrote:
> Thanks so much for the review, Aaron.
> 
> On Sun, 08 Jan 2012 20:08:59 -0500, Aaron Ecay  wrote:
> > A couple of comments on the arguments:
> > - It would be good to make show-next   This will enable code
> >   to call the fn with only two arguments, and not showing next will be
> >   the default behavior.
> 
> That's a nice idea.  Probably better for a separate patch, though.

This patch introduces show-next as a new argument to the function.  So it
can and should make it , if that is the appropriate semantics
for it to have.

> 
> > - A more lispy way of specifying the sign would be to use a
> >   boolean.  Perhaps you could call this ?remove?; a value of ?t? would
> >   remove the tag; ?nil? would add it.  Moving this argument after ?tag?
> >   and also making it  woudl allow this fn to be called with one
> >   arg to add a tag.  (Maybe this is too minimalist and API, however.) 
> 
> That might be more lispy, but it seems a lot less clear to me.  It might
> save a few keystrokes when coding, but it would definitely make the code
> a lot harder to read ("remove" to add a tag?).  I think I would prefer
> people to give the sign explicitly.

Using a string value makes it harder to interface with other code.  For
example, the prefix argument (C-u) is delivered to emacs commands as a
boolean value (nil if no arg, something truthy if the arg is given).  A
plausible custom end user function/keybinding would be one to add a tag
to the open messages, or remove that tag if the prefix arg is given to
the same command.  (So that ?d? deletes and ?C-u d? undeletes, for
example.)  In order to do that, the user?s code has to convert the
prefix arg into a string.  Making something ?lispy? isn?t just about
code readability/saving keystrokes, but also refers to how well the
code interfaces with the conventions used by the rest of the emacs
environment.

That said, here?s an alternate proposal: provide two functions as the
?external? API, namely ?notmuch-show-{add,remove}-tag-thread? (by
parallelism with ?notmuch-show-{add,remove}-tag?).  These could be
thin wrappers around ?notmuch-show-tag-thread-internal?, which would
then not be intended to be called by user code.

> 
> > No second set of parens is needed around tag-function.
> 
> Yeah, I've seen this either way.  I guess it's just a stylistic
> choice.

Using double parens is semantically correct, but makes the code less
idiomatic and harder to read (IMO).  To test my intuition, I looked at
?let? invocations in the Emacs source that have a non-initialized
variable (because the multiple variable case is hard to grep for).
Double parens are used only 3% of the time (44 double vs 1468 single).
Make of this data what you will.

-- 
Aaron Ecay


Re: [PATCH] emacs: call notmuch tag only once when archiving a thread

2012-01-09 Thread David Edmondson
On Sun, 8 Jan 2012 20:12:59 -0500, Austin Clements amdra...@mit.edu wrote:
 ... so we need to switch Emacs over to using the JSON search format
 first.

Is anyone working on this? I made an attempt ages ago[1], but have not kept
it working.

Footnotes: 
[1]  id:1291114825-3513-1-git-send-email-...@dme.org


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


Re: [PATCH v2 0/6] reply to sender

2012-01-09 Thread David Edmondson
On Sun,  8 Jan 2012 23:48:29 +0200, Jani Nikula j...@nikula.org wrote:
   emacs: add support for replying just to the sender
   emacs: bind 'r' to reply-to-sender and 'R' to reply-to-all

These both look fine to me.


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


Re: [PATCH v2] emacs: Helpers for notmuch developers.

2012-01-09 Thread David Edmondson
On Sat, 07 Jan 2012 07:53:54 -0400, David Bremner da...@tethera.net wrote:
 On Fri,  6 Jan 2012 10:03:19 +, David Edmondson d...@dme.org wrote:
  ---
  
  - Prefix the branch name with 'review/'
  - Avoid `shell-command', which also results in better error reporting
when 'git-am' fails.
  
 
 One thing I noticed is that reviewing a patch for the second time fails
 because the branch already exists. I'm not sure if this is covered under
 upkeep of the repo but it is a little inconvenient.

Oh. It's supposed to delete the existing branch. It did in my test. What
happens for you?


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


Re: [PATCH] emacs: Helpers for notmuch developers.

2012-01-09 Thread David Edmondson
On Sat, 07 Jan 2012 00:56:19 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
 What do you think of this approach related to your way cloning the
 repo and then deleting/creating the branch. Just that developer may
 mess with the repository contents and then there is tedious working
 tree cleanup to be done (especially those who are not so fluent using
 git).

This sounds good - I learn more about git all of the time. I'll try to
apply some of your suggestions.


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


Re: [PATCH] emacs: call notmuch tag only once when archiving a thread

2012-01-09 Thread Jani Nikula
On Sun, 8 Jan 2012 20:12:59 -0500, Austin Clements amdra...@mit.edu wrote:
 Quoth Aaron Ecay on Jan 08 at  7:56 pm:
  On Thu, 05 Jan 2012 22:32:16 +0200, Jani Nikula j...@nikula.org wrote:
  
  [...]
  
   In the show view it only modifies the messages that are currently
   visible. This is to make sure you don't accidentally archive things that
   have arrived after refreshing the buffer. I think this is safest.
  
  Hmm.  Perhaps it would make sense to add a check in the search view that
  the thread being archived[1] has the same number of messages as it did
  when the buffer was constructed.  (The information on how many messages
  the thread has is in the buffer; we would then compare this to the result
  of “notmuch count thread:000foo” when the user requests to archive.)  If
  the counts don’t match, the interface should show a message in the echo
  area and (probably) refuse to do the tagging.
 
 That sounds like a clever workaround.

The downside is that there's still a race condition: you could get new
messages between checking the number of messages in the thread and
tagging. The window for error would be much smaller than now, but it's
still there. (You could check afterwards if this happened, and notify
the user, ps, I just tagged N messages more than you intended...)

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


Re: [PATCH 4/4] emacs: Use the new JSON reply format.

2012-01-09 Thread David Edmondson
On Sun,  8 Jan 2012 00:52:42 -0700, Adam Wolfe Gordon awg+notm...@xvx.ca 
wrote:
 +(defun w3m-region (start end)) ;; From `w3m.el'.
 +(defun notmuch-mua-quote-part (part)
 +  (with-temp-buffer
 +(insert part)
 +(message-mode)
 +(fill-region (point-min) (point-max))
 +(goto-char (point-min))
 +(perform-replace ^   nil t nil)
 +(set-buffer-modified-p nil)
 +(buffer-substring (point-min) (point-max

Couldn't all of this be done directly in the reply buffer?

 +(defun notmuch-mua-parse-html-part (part)
 +  (with-temp-buffer
 +(insert part)
 +(w3m-region (point-min) (point-max))
 +(set-buffer-modified-p nil)
 +(buffer-substring (point-min) (point-max

Blank lines between functions are the house style (as much as there is
such a thing).

Using w3m means that you should `require' it. What happens when a user
doesn't have it? (Either the elisp or the command.)

  (defun notmuch-mua-reply (query-string optional sender)
 -  (let (headers
 +  (let (reply
 + original
 + headers
   body
 - (args '(reply)))
 + (args '(reply --format=json)))

Initialised variables are generally shown before uninitialised. I know
that you didn't do this 'wrong' and that it's an unrelated change, but
it should be fixed. (To the people who say that should be a separate
patch I say 'meh' - life is too short.)

  (if notmuch-show-process-crypto
   (setq args (append args '(--decrypt
  (setq args (append args (list query-string)))
 -;; This make assumptions about the output of `notmuch reply', but
 -;; really only that the headers come first followed by a blank
 -;; line and then the body.
 +;; Get the reply object as JSON, and parse it into an elisp object.
  (with-temp-buffer
(apply 'call-process (append (list notmuch-command nil (list t t) nil) 
 args))
(goto-char (point-min))
 -  (if (re-search-forward ^$ nil t)
 -   (save-excursion
 - (save-restriction
 -   (narrow-to-region (point-min) (point))
 -   (goto-char (point-min))
 -   (setq headers (mail-header-extract)
 -  (forward-line 1)
 -  (setq body (buffer-substring (point) (point-max
 +  (setq reply (aref (json-read) 0)))
 +
 +;; Get the list of headers
 +(setq headers (cdr (assq 'headers (assq 'reply reply
 +;; Construct the body of the reply.
 +(setq original (cdr (assq 'original reply)))

The scope of (at least) `headers' and `original' could be tightened by
moving them down to the following `let' (and converting it to `let*').

 +
 +;; Start with the prelude, based on the headers of the original message.
 +(let ((original-headers (cdr (assq 'headers original
 +  (setq body (format On %s, %s wrote:\n
 +  (cdr (assq 'date original-headers))
 +  (cdr (assq 'from original-headers)
 +
 +;; Extract the body parts and construct a reasonable quoted body.
 +(let* ((body-parts (cdr (assq 'body original)))
 +(find-parts (lambda (type) (delq nil (mapcar (lambda (part)
 +   (if (string= (cdr 
 (assq 'content-type part)) type)
 +   (cdr (assq 
 'content part
 + body-parts

`find-parts' might be useful in other places. Maybe add it to notmuch-lib.el?

 +(plain-parts (apply find-parts '(text/plain)))
 +(html-parts (apply find-parts '(text/html
 +  
 +  (if (not (null plain-parts))
 +   (mapc (lambda (part) (setq body (concat body (notmuch-mua-quote-part 
 part plain-parts)
 + (mapc (lambda (part) (setq body (concat body (notmuch-mua-quote-part 
 (notmuch-mua-parse-html-part part) html-parts)))

If you have an 'else' clause, why test '(if (not ..' ?

 +(setq body (concat body \n))
 + 

If it already ends with a carriage return, why do this?


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


Re: [PATCH v2 3/6] cli: add support for replying just to the sender in notmuch reply

2012-01-09 Thread Jani Nikula
On Sun, 08 Jan 2012 23:23:15 +, Mark Walters markwalters1...@gmail.com 
wrote:
 
 I like this version (of the whole series) but have two queries. (Note I
 haven't actually tried it out yet: I have just been reading the code.)
 
  +   /* Force recipient type in reply-to-sender mode just in case replying to
  +* user's own message finds recipients in Cc/Bcc fields only.
  +*/
  +   if (reply_all)
  +   recipient_type = reply_to_map[i].recipient_type;
  +   else
  +   recipient_type = GMIME_RECIPIENT_TYPE_TO;
  +
  +   addr = add_recipients_for_string (reply, config, recipient_type,
  + recipients, add_recipients);
  +
 
 Why force the recipient type?  I do not think notmuch should ever move
 bcc people onto a different header. I think that will bite someone when
 it leaks information.
 
 I would be happy with either empty headers or with the people staying
 as bcc.
 
 In the cc: case I have no preference (as those addresses are already
 public) but I would suggest being consistent with bcc so either empty
 headers or keep them in the cc line.

I think you're right. I was trying to clean up a corner case in
reply-to-self, but probably over did it. I'll remove the recipient type
forcing altogether. Then it'll keep addresses in the same header as they
were.

  for (messages = notmuch_query_search_messages (query);
  notmuch_messages_valid (messages);
  notmuch_messages_move_to_next (messages))
  {
 [...]
(void)add_recipients_from_message (reply, config, message, reply_all);
 
 Since the above logic is applied to each email individually I think
 working out the recipients when replying to multiple emails (e.g.,
 reply-to-sender on a thread) could be very confusing. Some of the people
 being replied to will have been senders, some recipients (it is very
 likely that the thread contains messages we sent), some could even be
 cc, bcc people. Personally, I would have no idea what to expect from
 reply-to-sender in this case.
 
 (My personal choice would be not to allow notmuch-reply-to-sender if
 multiple messages are specified. But I can obtain that by unbinding r
 in the notmuch-search-mode-map keymap.)

The reply-to-thread is a rare case anyway, regardless of reply-to-all or
reply-to-sender, and even the current implementation does not gather all
the recipients from all the messages. Try it out, it seems to me it does
not quite do what you think it does.

IMHO it should use oldest-first rather than newest-first sort order, and
I guess it should really go through the messages being replied to and
get the recipients from all of them. It's just out of scope for this
patch set to fix these. But if that gets fixed later, I would like the
reply-to-sender for multiple messages follow the logic of single-reply,
just with all the addresses put together.


BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2] emacs: Helpers for notmuch developers.

2012-01-09 Thread David Bremner
On Mon, 09 Jan 2012 08:23:53 +, David Edmondson d...@dme.org wrote:
 On Sat, 07 Jan 2012 07:53:54 -0400, David Bremner da...@tethera.net wrote:
  On Fri,  6 Jan 2012 10:03:19 +, David Edmondson d...@dme.org wrote:
   ---
   
   - Prefix the branch name with 'review/'
   - Avoid `shell-command', which also results in better error reporting
 when 'git-am' fails.
   
  
  One thing I noticed is that reviewing a patch for the second time fails
  because the branch already exists. I'm not sure if this is covered under
  upkeep of the repo but it is a little inconvenient.
 
 Oh. It's supposed to delete the existing branch. It did in my test. What
 happens for you?

FWIW, I loaded notmuch-dev.el on top of current master.

The magit buffer shows 

$ git --no-pager checkout -b 
review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results
 master
fatal: A branch named 
'review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results'
 already exists.

The backtrace is 

  signal(error (Git failed))
  error(Git failed)
  magit-run*((git --no-pager checkout -b 
review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results
 master))
  #[nil \303\304  B\n\!\207 [magit-git-executable 
magit-git-standard-options args magit-run* append] 4]()
  magit-refresh-wrapper(#[nil \303\304B\n\!\207 
[magit-git-executable magit-git-standard-options args magit-run* append] 4])
  magit-run-git(checkout -b 
review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results
 master)
  
magit-create-branch(review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results
 master)
  
notmuch-dev-create-branch(review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results)
  (let ((default-directory notmuch-dev-temporary-repository-path)) 
(notmuch-dev-checkout-master) (condition-case nil (notmuch-dev-delete-branch 
patch-name) (error nil)) (notmuch-dev-create-branch patch-name) (with-temp-file 
mbox-path (erase-buffer) (call-process notmuch-command nil t nil show 
--format=mbox search-terms)) (magit-run* (list magit-git-executable am 
mbox-path)) (magit-status notmuch-dev-temporary-repository-path))
  (let ((patch-name ...) (mbox-path ...)) 
(notmuch-dev-make-temporary-repository) (let (...) 
(notmuch-dev-checkout-master) (condition-case nil ... ...) 
(notmuch-dev-create-branch patch-name) (with-temp-file mbox-path ... ...) 
(magit-run* ...) (magit-status notmuch-dev-temporary-repository-path)))
  notmuch-dev-review-patch([PATCH] emacs: Don't signal an error when reaching 
the end of the search results. 
id:\1324370714-28545-1-git-send-email-...@dme.org\)
  notmuch-dev-show-review-patch()
  call-interactively(notmuch-dev-show-review-patch t nil)
  execute-extended-command(nil)
  call-interactively(execute-extended-command nil nil)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] emacs: call notmuch tag only once when archiving a thread

2012-01-09 Thread Tomi Ollila
On Mon, 09 Jan 2012 08:41:15 +, Jani Nikula j...@nikula.org wrote:
 On Sun, 8 Jan 2012 20:12:59 -0500, Austin Clements amdra...@mit.edu wrote:
  Quoth Aaron Ecay on Jan 08 at  7:56 pm:
   On Thu, 05 Jan 2012 22:32:16 +0200, Jani Nikula j...@nikula.org wrote:
   
   [...]
   
In the show view it only modifies the messages that are currently
visible. This is to make sure you don't accidentally archive things that
have arrived after refreshing the buffer. I think this is safest.
   
   Hmm.  Perhaps it would make sense to add a check in the search view that
   the thread being archived[1] has the same number of messages as it did
   when the buffer was constructed.  (The information on how many messages
   the thread has is in the buffer; we would then compare this to the result
   of “notmuch count thread:000foo” when the user requests to archive.)  If
   the counts don’t match, the interface should show a message in the echo
   area and (probably) refuse to do the tagging.
  
  That sounds like a clever workaround.
 
 The downside is that there's still a race condition: you could get new
 messages between checking the number of messages in the thread and
 tagging. The window for error would be much smaller than now, but it's
 still there. (You could check afterwards if this happened, and notify
 the user, ps, I just tagged N messages more than you intended...)

And this could also be false alarm if the the new messages arrived
after tagging but before checking...

 
 J.

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


Re: [PATCH] emacs: call notmuch tag only once when archiving a thread

2012-01-09 Thread David Edmondson
On Mon, 09 Jan 2012 12:38:58 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
  The downside is that there's still a race condition: you could get new
  messages between checking the number of messages in the thread and
  tagging. The window for error would be much smaller than now, but it's
  still there. (You could check afterwards if this happened, and notify
  the user, ps, I just tagged N messages more than you intended...)
 
 And this could also be false alarm if the the new messages arrived
 after tagging but before checking...

Or another process could change the tags, meaning that the number stayed
the same but the set changed.

(Wh)


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


Re: [PATCH v2 3/6] cli: add support for replying just to the sender in notmuch reply

2012-01-09 Thread Mark Walters
 
 The reply-to-thread is a rare case anyway, regardless of reply-to-all or
 reply-to-sender, and even the current implementation does not gather all
 the recipients from all the messages. Try it out, it seems to me it does
 not quite do what you think it does.
 
 IMHO it should use oldest-first rather than newest-first sort order, and
 I guess it should really go through the messages being replied to and
 get the recipients from all of them. It's just out of scope for this
 patch set to fix these. But if that gets fixed later, I would like the
 reply-to-sender for multiple messages follow the logic of single-reply,
 just with all the addresses put together.

I completely agree it's a rare case. Having tried the code (yours and
the original) it seems to be very strange:

In its current form it seems to be reply to people on newest message in
thread and include all the bodies with some spurious headers mixed
in. So your code is reply-to-sender applied to people on newest message
in thread, and include all the bodies with some slightly different
spurious headers mixed in.

I like your suggested solution.

Anyway it's a rare case; the current code is odd and yours is no odder
so I agree with leaving it for a later patch (if anyone cares enough!).

Best wishes

Mark

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


Re: [PATCH v2] emacs: Helpers for notmuch developers.

2012-01-09 Thread David Edmondson
On Mon, 09 Jan 2012 06:38:54 -0400, David Bremner da...@tethera.net wrote:
  Oh. It's supposed to delete the existing branch. It did in my test. What
  happens for you?
 
 FWIW, I loaded notmuch-dev.el on top of current master.
 
 The magit buffer shows 
 
 $ git --no-pager checkout -b 
 review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results
  master
 fatal: A branch named 
 'review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results'
  already exists.
 
 The backtrace is 
 
   signal(error (Git failed))
   error(Git failed)
   magit-run*((git --no-pager checkout -b 
 review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results
  master))
   #[nil \303\304B\n\!\207 [magit-git-executable 
 magit-git-standard-options args magit-run* append] 4]()
   magit-refresh-wrapper(#[nil \303\304  B\n\!\207 
 [magit-git-executable magit-git-standard-options args magit-run* append] 4])
   magit-run-git(checkout -b 
 review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results
  master)
   
 magit-create-branch(review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results
  master)
   
 notmuch-dev-create-branch(review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results)
   (let ((default-directory notmuch-dev-temporary-repository-path)) 
 (notmuch-dev-checkout-master) (condition-case nil (notmuch-dev-delete-branch 
 patch-name) (error nil)) (notmuch-dev-create-branch patch-name) 
 (with-temp-file mbox-path (erase-buffer) (call-process notmuch-command nil t 
 nil show --format=mbox search-terms)) (magit-run* (list 
 magit-git-executable am mbox-path)) (magit-status 
 notmuch-dev-temporary-repository-path))
   (let ((patch-name ...) (mbox-path ...)) 
 (notmuch-dev-make-temporary-repository) (let (...) 
 (notmuch-dev-checkout-master) (condition-case nil ... ...) 
 (notmuch-dev-create-branch patch-name) (with-temp-file mbox-path ... ...) 
 (magit-run* ...) (magit-status notmuch-dev-temporary-repository-path)))
   notmuch-dev-review-patch([PATCH] emacs: Don't signal an error when 
 reaching the end of the search results. 
 id:\1324370714-28545-1-git-send-email-...@dme.org\)
   notmuch-dev-show-review-patch()
   call-interactively(notmuch-dev-show-review-patch t nil)
   execute-extended-command(nil)
   call-interactively(execute-extended-command nil nil)

Could you try running
  (notmuch-dev-delete-branch 
review/patch-emacs-don't-signal-an-error-when-reaching-the-end-of-the-search-results)
when inside that repository please?


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


Re: [PATCH] emacs: call notmuch tag only once when archiving a thread

2012-01-09 Thread Mark Walters

 There's been quite a bit of discussion on fixing this properly.  See,
 for example
 id:CAH-f9WsPj=1Eu=g3soepjgctbfs6hrldlq18xmenj8az00y...@mail.gmail.com.
 The gist is that we need to include message IDs (or document IDs) in
 the search output and use these in tagging operations, rather than the
 unstable thread:XXX queries.  Unfortunately, actually fixing this got
 stalled since adding this information the text format is a fool's
 errand (having been the fool, I can say this!), so we need to switch
 Emacs over to using the JSON search format first.  However, once
 that's done, it's a relatively simple change.

I will just mention a different possible solution to this problem. I
think it is probably more hassle than its worth but just in case someone
sees how to do it nicely. 

The idea is to try and put the race-free solution into the command-line
notmuch rather than the emacs part.

I will just consider one race to start with: the race in notmuch
search/notmuch tag. notmuch search gives us just the thread-ids and what
is in that thread can change. So we could ask for a cookie (some short
unique identifier) and notmuch itself would store the state of its
output (i.e., the thread-ids together with the message-ids and the
matched message-ids).

Then notmuch tag could be called with this cookie to say apply this tag
to the threads as they were at cookie.

example use
notmuch search --cookie query
prints a cookie followed by the output of the search

notmuch tag --cookie=cookievalue +sometag matched-thread:id .. \
 all-thread:id .. other search terms

and notmuch would replace the matched-thread:id by the (matching)
message-ids in thread it stored at time cookievalue and all-thread by
all the messages at time cookievalue.

The nice feature is that this makes it easy for all clients to be race
free (not just emacs). The backend has the information to hand: there is
no encoding and decoding (via text or JSON) so storing message-ids etc
is simpler.  It also means that if notmuch wants to use document-ids
internally it can: it does not become a public interface.

The downside is trying to do the argument parsing in c and we have do
some reference counting or something to delete the cookies state at some
point.

Anyway it's just a thought.

Best wishes

Mark


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


[PATCH] NEWS: add news entry for notmuch reply uninitialized variable bugfix

2012-01-09 Thread Jani Nikula
---

This is against release branch.
---
 NEWS |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 85ff65b..687154d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,8 +1,8 @@
 Notmuch 0.11 (201x-xx-xx)
 =
 
-New command-line features
--
+Command-Line Interface
+--
 
 Hooks
 
@@ -11,6 +11,12 @@ Hooks
   supports pre-new and post-new hooks that are run before and after
   importing new messages into the database.
 
+notmuch reply --decrypt bugfix
+
+  The notmuch reply command with --decrypt argument had a rarely
+  occurring bug that caused an encrypted message not to be decrypted
+  sometimes. This is now fixed.
+
 Performance
 ---
 
-- 
1.7.1

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


Re: 0.11 is frozen, please update NEWS

2012-01-09 Thread David Bremner
On Mon, 02 Jan 2012 07:58:16 -0400, David Bremner da...@tethera.net wrote:
 
 
 I have tagged 0.11_rc2 and uploaded 0.11~rc2-1 to Debian experimental.
 This includes only the single non-doc commit, fixing a python bindings
 segfault. NEWS items are still solicited.

OK, I have tagged 0.11_rc3 and pushed 0.11~rc3-1 to Debian
experimental. 

I'm pretty determined to release this, after I get one more small NEWS
patch from Jani. I'd appreciate any feedback for either the release
branch or the debian experimental packages.

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


Re: nmbug changes

2012-01-09 Thread Justus Winter
Quoting David Bremner (2012-01-08 23:34:26)
On Sun, 08 Jan 2012 16:24:06 -, Justus Winter 
4win...@informatik.uni-hamburg.de wrote:

 I'm having trouble with nmbug, I did follow the instructions in this
 mail and in the wiki, but nmbug never adds tags to my notmuch database
 (it is supposed to do that, right?).

In case you just want to load the tags in the git repo into your notmuch
database, try running nmbug checkout.

That actually worked, but nmbug help says:

   checkout
   Update the notmuch database from git. This is mainly useful to
   discard your changes in notmuch relative to git.
[...]
   merge   Merge changes from FETCH_HEAD into HEAD, and load the result
   into notmuch.

That's why I assumed that merge is the command of choice in my
situation.

It might be the instructions on getting started could be updated here?
Perhaps someone who has gotten started more recently can chime in here.

If checkout is neccessary, it surely should be added to the wiki
page. Once I've gained some experience with nmbug I'll update the
wiki, though I wouldn't mind if someone beats me to it ;)

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