[PATCH 0/5] some fixes for emacs client

[PATCH 0/5] some fixes for emacs client

[Dmitry Kurochkin]

Hi all.

Thanks for such a great email client! I wanted to move away from gmail web
interface for a while, but could not find a suitable client. Now I am moving
to notmuch, and it is a pleasure to use so far.

Here are some small but useful (at least for me) fixes for the emacs client:

* Quote id:"message-id" queries. Fixes errors when message-id contains special
  chars, e.g. `..'.

* Use message-field-value to get headers in fcc. Fixes cases when message
  contains lines starting with From:, common when forwarding.

* Add notmuch-hook. I use it to start server, which is used for new mail
  notifications and notmuch-hello buffer updates.

* Add notmuch-hello-hook. I use it to turn off show-trailing-whitespace.

This is the first time I use git send-email. Sorry if I did it wrong.

Regards,
  Dmitry

[PATCH 1/5] Use notmuch-show-get-message-id in notmuch-show-get-bodypart-content.

[Dmitry Kurochkin]

---
 emacs/notmuch-show.el |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 84c6cd6..a845efc 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -349,7 +349,7 @@ current buffer, if possible."
 
 (defun notmuch-show-get-bodypart-content (msg part nth)
   (or (plist-get part :content)
-      (notmuch-show-get-bodypart-internal (concat "id:" (plist-get msg :id)) nth)))
+      (notmuch-show-get-bodypart-internal (notmuch-show-get-message-id msg) nth)))
 
 ;; \f
 
@@ -728,9 +728,9 @@ All currently available key bindings:
 		   (notmuch-show-get-message-properties))))
     (plist-get props prop)))
 
-(defun notmuch-show-get-message-id ()
+(defun notmuch-show-get-message-id (&optional props)
   "Return the message id of the current message."
-  (concat "id:" (notmuch-show-get-prop :id)))
+  (concat "id:" (notmuch-show-get-prop :id props)))
 
 ;; dme: Would it make sense to use a macro for many of these?
 
-- 
1.7.1

[PATCH 2/5] Add quotes around id:"message-id" queries.

[Dmitry Kurochkin]

---
 emacs/notmuch-message.el |    2 +-
 emacs/notmuch-show.el    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-message.el b/emacs/notmuch-message.el
index d5c96c2..4bc76b2 100644
--- a/emacs/notmuch-message.el
+++ b/emacs/notmuch-message.el
@@ -45,7 +45,7 @@ the \"inbox\" and \"todo\", you would set
 			       str))
 			  notmuch-message-replied-tags)))
 	(apply 'notmuch-call-notmuch-process "tag"
-	       (append tags (list (concat "id:" (car (car rep)))) nil))))))
+	       (append tags (list (concat "id:\"" (car (car rep)) "\"")) nil))))))
 
 (add-hook 'message-send-hook 'notmuch-message-mark-replied)
 
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index a845efc..c5e88f4 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -730,7 +730,7 @@ All currently available key bindings:
 
 (defun notmuch-show-get-message-id (&optional props)
   "Return the message id of the current message."
-  (concat "id:" (notmuch-show-get-prop :id props)))
+  (concat "id:\"" (notmuch-show-get-prop :id props) "\""))
 
 ;; dme: Would it make sense to use a macro for many of these?
 
-- 
1.7.1

[PATCH 3/5] Use message-field-value instead of message-fetch-field.

[Dmitry Kurochkin]

---
 emacs/notmuch-maildir-fcc.el |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-maildir-fcc.el b/emacs/notmuch-maildir-fcc.el
index 8bb41a8..c3e4505 100644
--- a/emacs/notmuch-maildir-fcc.el
+++ b/emacs/notmuch-maildir-fcc.el
@@ -83,18 +83,18 @@
           ;; notmuch-fcc-dirs is a string, just use it as subdir
           (setq subdir notmuch-fcc-dirs)
         ;; else: it's a list of alists (("sent") ("name1" . "sent1"))
-        (setq subdir (cdr (assoc-string (message-fetch-field "from") notmuch-fcc-dirs t)))
+        (setq subdir (cdr (assoc-string (message-field-value "from") notmuch-fcc-dirs t)))
          ;; if we found no hit, use the first entry as default fallback
          (unless subdir (setq subdir (car (car notmuch-fcc-dirs)))))
 
   ;; if there is no fcc header yet, add ours
-  (unless (message-fetch-field "fcc")
+  (unless (message-field-value "fcc")
     (message-add-header (concat "Fcc: "
                                 (file-name-as-directory message-directory)
                                 subdir)))
 
   ;; finally test if fcc points to a valid maildir
-  (let ((fcc-header (message-fetch-field "fcc")))
+  (let ((fcc-header (message-field-value "fcc")))
     (unless (notmuch-maildir-fcc-dir-is-maildir-p fcc-header)
       (cond ((not (file-writable-p fcc-header))
              (error (format "%s is not a maildir, but you don't have permission to create one." fcc-header)))
-- 
1.7.1

[PATCH 4/5] Add notmuch-hook. Called when notmuch is started, before notmuch-hello buffer is created.

[Dmitry Kurochkin]

---
 emacs/notmuch.el |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 5ab3df7..4900a27 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -185,6 +185,11 @@ For a mouse binding, return nil."
       (set-buffer-modified-p nil)
       (view-buffer (current-buffer) 'kill-buffer-if-not-modified))))
 
+(defcustom notmuch-hook nil
+  "Functions to call when notmuch is started, before `notmuch-hello' buffer is created."
+  :group 'notmuch
+  :type 'hook)
+
 (defcustom notmuch-search-hook '(hl-line-mode)
   "List of functions to call when notmuch displays the search results."
   :type 'hook
@@ -895,6 +900,7 @@ current search results AND that are tagged with the given tag."
 (defun notmuch ()
   "Run notmuch and display saved searches, known tags, etc."
   (interactive)
+  (run-hooks 'notmuch-hook)
   (notmuch-hello))
 
 (setq mail-user-agent 'notmuch-user-agent)
-- 
1.7.1

[PATCH 5/5] Add notmuch-hello-hook. Called every time notmuch-hello buffer is updated.

[Dmitry Kurochkin]

---
 emacs/notmuch-hello.el |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index f8ae332..5777e99 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -32,6 +32,11 @@
 (defvar notmuch-hello-search-bar-marker nil
   "The position of the search bar within the notmuch-hello buffer.")
 
+(defcustom notmuch-hello-hook nil
+  "Functions to call after populating a `notmuch-hello' buffer."
+  :group 'notmuch
+  :type 'hook)
+
 (defcustom notmuch-recent-searches-max 10
   "The number of recent searches to store and display."
   :type 'integer
@@ -508,7 +513,9 @@ Complete list of currently available key bindings:
 	  (widget-forward 1)))
 
       (unless (widget-at)
-	(notmuch-hello-goto-search)))))
+	(notmuch-hello-goto-search))))
+
+  (run-hooks 'notmuch-hello-hook))
 
 ;;;###autoload
 (defun notmuch-folder ()
-- 
1.7.1

Re: [PATCH 2/5] Add quotes around id:"message-id" queries.

[Olly Betts]

On 2010-07-01, Dmitry Kurochkin wrote:
> -  (concat "id:" (notmuch-show-get-prop :id props)))
> +  (concat "id:\"" (notmuch-show-get-prop :id props) "\""))

This is probably a good idea (the ".." example is arguably a Xapian bug so
that should be fixed soon, but you find all sorts of junk in message-ids.

However, the quoting feature this relies on was added in Xapian 1.0.18 (and
1.1.4), and with older versions this will break for *all* message-ids (even
those which currently work).

Also, if you're going to quote the message-id, you should escaping any "
characters in the message-id (by doubling them).

Cheers,
    Olly

Re: [PATCH 2/5] Add quotes around id:"message-id" queries.

[Dmitry Kurochkin]

On Fri, 2 Jul 2010 04:41:43 +0000 (UTC), Olly Betts <olly@survex.com> wrote:
> On 2010-07-01, Dmitry Kurochkin wrote:
> > -  (concat "id:" (notmuch-show-get-prop :id props)))
> > +  (concat "id:\"" (notmuch-show-get-prop :id props) "\""))
> 
> This is probably a good idea (the ".." example is arguably a Xapian bug so
> that should be fixed soon, but you find all sorts of junk in message-ids.
> 

If I comment out add_valuerangeprocessor call in
notmuch_database_open(), ids with .. are matched fine with no quotes.

So it seems that xapian uses the ValueRangeProcessor for all terms
while it should be used for one value parsing only. Is this correct?

Is there a xapian bug for this?

> However, the quoting feature this relies on was added in Xapian 1.0.18 (and
> 1.1.4), and with older versions this will break for *all* message-ids (even
> those which currently work).
> 

Indeed.

> Also, if you're going to quote the message-id, you should escaping any "
> characters in the message-id (by doubling them).
> 

Right.

I have found a xapian bug #128 "Allow queryparser to treat some prefixes
as literal text". Seems to be just what we need here. Perhaps instead of
quoting in emacs client, we can wait for the value range parsing fix
(can be fixed in minor release?) and use #128 when it is available. IMHO
should be good enought in most cases. What do you think?

Regards,
  Dmitry

> Cheers,
>     Olly
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 2/5] Add quotes around id:"message-id" queries.

[Olly Betts]

On Fri, Jul 02, 2010 at 05:04:46PM +0400, Dmitry Kurochkin wrote:
> On Fri, 2 Jul 2010 04:41:43 +0000 (UTC), Olly Betts <olly@survex.com> wrote:
> > On 2010-07-01, Dmitry Kurochkin wrote:
> > > -  (concat "id:" (notmuch-show-get-prop :id props)))
> > > +  (concat "id:\"" (notmuch-show-get-prop :id props) "\""))
> > 
> > This is probably a good idea (the ".." example is arguably a Xapian bug so
> > that should be fixed soon, but you find all sorts of junk in message-ids.
> 
> If I comment out add_valuerangeprocessor call in
> notmuch_database_open(), ids with .. are matched fine with no quotes.

Yes, the code which checks for ranges is disabled if there are no possible
ranges to find.

> So it seems that xapian uses the ValueRangeProcessor for all terms
> while it should be used for one value parsing only. Is this correct?

The issue is that if there's a ".." in there you have to ask the VRPs to
find out if it is a range they understand or not, so they have to be called
first in such cases (otherwise the same prefix couldn't be made to work for
ranges and single term filters).  There needs to be some sort of fallback
to considering boolean filters if there isn't a valid range though.

> Is there a xapian bug for this?

I couldn't find a ticket for it, but I was aware of the issue.

I've committed a fix to Xapian now (r14790 on trunk), which should be in
Xapian 1.2.3 when it gets released.

> I have found a xapian bug #128 "Allow queryparser to treat some prefixes
> as literal text". Seems to be just what we need here. Perhaps instead of
> quoting in emacs client, we can wait for the value range parsing fix
> (can be fixed in minor release?) and use #128 when it is available. IMHO
> should be good enought in most cases. What do you think?

The main problem at the moment is with "..", which is now fixed on trunk.
So any Xapian version with #128 fully addressed will handle ".." in
message-ids fine anyway.

With current trunk, message-ids with whitespace or ')' in will still
misbehave unless you quote them.  If the "FieldProcessor" idea in #128 were
implemented, you could arrange for whitespace and ')' to be included, but
then it would be impossible to end a message-id term - it would span to the
end of the query string, which I think would surprise most users.

The ability to quote terms discussed in #128 is already implemented (that
is what you've been using!) and I think that using this selectively is
probably the best way to deal with this.

If you only try to quote message-ids which either:

* contain whitespace, "..", or ')'
* start with '"'

Then the only cases which break with older Xapian will be those which
wouldn't work there anyway, plus message-ids which start with a '"' (which 
seem rare - I couldn't find any in my mail folders).

Cheers,
    Olly