[PATCH 2/2] emacs: Correctly quote non-text/plain parts in reply
Quote non-text parts nicely by displaying them with mm-display-part before calling message-cite-original to quote them. HTML-only emails can now be quoted correctly. Mark the test for this feature as not broken. --- emacs/notmuch-mua.el | 20 +++- test/emacs |1 - 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el index 87bd88d..f7af789 100644 --- a/emacs/notmuch-mua.el +++ b/emacs/notmuch-mua.el @@ -21,6 +21,7 @@ (require 'json) (require 'message) +(require 'mm-view) (require 'format-spec) (require 'notmuch-lib) @@ -90,6 +91,19 @@ list." else if (notmuch-match-content-type (plist-get part :content-type) "text/*") collect part)) +(defun notmuch-mua-insert-quotable-part (message part) + (save-restriction +(narrow-to-region (point) (point)) +(insert (notmuch-get-bodypart-content message part + (plist-get part :id) + notmuch-show-process-crypto)) +(let ((handle (mm-make-handle (current-buffer) + (list (plist-get part :content-type + (end-of-orig (point-max))) + (mm-display-part handle) + (kill-region (point-min) end-of-orig)) +(goto-char (point-max + ;; There is a bug in emacs 23's message.el that results in a newline ;; not being inserted after the References header, so the next header ;; is concatenated to the end of it. This function fixes the problem, @@ -169,11 +183,7 @@ list." ;; Get the parts of the original message that should be quoted; this includes ;; all the text parts, except the non-preferred ones in a multipart/alternative. (let ((quotable-parts (notmuch-mua-get-quotable-parts (plist-get original :body - (mapc (lambda (part) - (insert (notmuch-get-bodypart-content original part - (plist-get part :id) - notmuch-show-process-crypto))) - quotable-parts)) + (mapc (apply-partially 'notmuch-mua-insert-quotable-part original) quotable-parts)) (set-mark (point)) (goto-char start) diff --git a/test/emacs b/test/emacs index e648f80..579844f 100755 --- a/test/emacs +++ b/test/emacs @@ -445,7 +445,6 @@ EOF test_expect_equal_file OUTPUT EXPECTED test_begin_subtest "Reply within emacs to an html-only message" -test_subtest_known_broken add_message '[content-type]="text/html"' \ '[body]="Hi,This is an HTML test message.OK?"' test_emacs "(let ((message-hidden-headers '())) -- 1.7.5.4
[PATCH 1/2] test: Replying to an HTML-only message in emacs
With the latest reply infrastructure, we should be able to nicely
quote HTML-only emails. But currently emacs quotes the raw HTML
instead of parsing it first. This commit adds a test for this case.
This test currently marked as broken.
---
test/emacs | 27 +++
1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/test/emacs b/test/emacs
index c7510e9..e648f80 100755
--- a/test/emacs
+++ b/test/emacs
@@ -444,6 +444,33 @@ Alex Botero-Lowry writes:
EOF
test_expect_equal_file OUTPUT EXPECTED
+test_begin_subtest "Reply within emacs to an html-only message"
+test_subtest_known_broken
+add_message '[content-type]="text/html"' \
+ '[body]="Hi,This is an HTML test message.OK?"'
+test_emacs "(let ((message-hidden-headers '()))
+ (notmuch-show \"id:${gen_msg_id}\")
+ (notmuch-show-reply)
+ (test-output))"
+sed -i -e 's,^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX
Emacs/XXX,' OUTPUT
+cat
+Fcc: ${MAIL_DIR}/sent
+References: <${gen_msg_id}>
+User-Agent: Notmuch/XXX Emacs/XXX
+--text follows this line--
+Notmuch Test Suite writes:
+
+> Hi,
+> This is an HTML test message.
+>
+> OK?
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
test_begin_subtest "Quote MML tags in reply"
message_id='test-emacs-mml-quoting at message.id'
add_message [id]="$message_id" \
--
1.7.5.4
[PATCH 0/2] Replying to HTML-only emails
Hi all, My recent reply enhancements were originally intended to allow proper quoting of HTML-only email in reply. While the final version was a big improvement on reply in general, it didn't actually acheive this goal. So, this series finishes that work, using mm-display-part to render the content of the original message correctly before inserting it into the reply message. I've also added a test for this feature. I'm not 100% happy with how the code looks for this, but I couldn't figure out a way to make it work without the kind of ugly narrow-to-region. If anyone has suggestions here I'd like to hear them. Adam Wolfe Gordon (2): test: Replying to an HTML-only message in emacs emacs: Correctly quote non-text/plain parts in reply emacs/notmuch-mua.el | 20 +++- test/emacs | 26 ++ 2 files changed, 41 insertions(+), 5 deletions(-) -- 1.7.5.4
[PATCH v3 1/2] cli: make --entire-thread=false work for format=json.
On Sat, Apr 21, 2012 at 03:15, Mark Walters
wrote:
> The --entire-thread option in notmuch-show.c defaults to true when
> format=json. Previously there was no way to turn this off. This patch
> makes it respect --entire-thread=false.
>
> The one subtlety is that we initialise a notmuch_bool_t to -1 to
> indicate that the option parsing has not set it. This allows the code
> to distinguish between the option being omitted from the command line,
> and the option being set to false on the command line.
>
> Finally, all formats except Json can output empty messages for non
> entire-thread, but in Json format we need to output {} to keep the
> other elements (e.g. the replies to this message) in the correct
> place.
LGTM. You can probably remove the relevant item from devel/TODO as well!
-- Adam
[PATCH v3 2/2] emacs: make elide messages use notmuch-show for omitting messages.
Previously the elide messages code got the entire-thread from
notmuch-show.c and then threw away all non-matching messages. This
version calls notmuch-show.c without the --entire-thread flag so
it never receives the non-matching messages in the first place.
This makes it substantially faster.
---
emacs/notmuch-show.el | 18 ++
1 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 4ee4290..347f90c 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -976,9 +976,9 @@ current buffer, if possible."
"Insert the message tree TREE at depth DEPTH in the current thread."
(let ((msg (car tree))
(replies (cadr tree)))
-(if (or (not notmuch-show-elide-non-matching-messages)
- (plist-get msg :match))
- (notmuch-show-insert-msg msg depth))
+;; We test whether there is a message or just some replies.
+(when msg
+ (notmuch-show-insert-msg msg depth))
(notmuch-show-insert-thread replies (1+ depth
(defun notmuch-show-insert-thread (thread depth)
@@ -1059,16 +1059,18 @@ function is used."
(args (if notmuch-show-query-context
(append (list "\'") basic-args
(list "and (" notmuch-show-query-context ")\'"))
-(append (list "\'") basic-args (list "\'")
- (notmuch-show-insert-forest (notmuch-query-get-threads
-(cons "--exclude=false" args)))
+(append (list "\'") basic-args (list "\'"
+(cli-args (cons "--exclude=false"
+(when notmuch-show-elide-non-matching-messages
+ (list "--entire-thread=false")
+
+ (notmuch-show-insert-forest (notmuch-query-get-threads (append cli-args
args)))
;; If the query context reduced the results to nothing, run
;; the basic query.
(when (and (eq (buffer-size) 0)
notmuch-show-query-context)
(notmuch-show-insert-forest
- (notmuch-query-get-threads
- (cons "--exclude=false" basic-args)
+ (notmuch-query-get-threads (append cli-args basic-args)
(jit-lock-register #'notmuch-show-buttonise-links)
--
1.7.9.1
[PATCH v3 1/2] cli: make --entire-thread=false work for format=json.
The --entire-thread option in notmuch-show.c defaults to true when
format=json. Previously there was no way to turn this off. This patch
makes it respect --entire-thread=false.
The one subtlety is that we initialise a notmuch_bool_t to -1 to
indicate that the option parsing has not set it. This allows the code
to distinguish between the option being omitted from the command line,
and the option being set to false on the command line.
Finally, all formats except Json can output empty messages for non
entire-thread, but in Json format we need to output {} to keep the
other elements (e.g. the replies to this message) in the correct
place.
---
notmuch-show.c | 34 +-
1 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/notmuch-show.c b/notmuch-show.c
index da4a797..327c263 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -800,6 +800,16 @@ format_part_raw (unused (const void *ctx), mime_node_t
*node,
}
static notmuch_status_t
+show_null_message (const notmuch_show_format_t *format)
+{
+/* For all formats except json an empty message output is valid;
+ * for json we need the braces.*/
+if (format == _json)
+ printf ("{}");
+return NOTMUCH_STATUS_SUCCESS;
+}
+
+static notmuch_status_t
show_message (void *ctx,
const notmuch_show_format_t *format,
notmuch_message_t *message,
@@ -862,11 +872,13 @@ show_messages (void *ctx,
if (status && !res)
res = status;
next_indent = indent + 1;
-
- if (!status && format->message_set_sep)
- fputs (format->message_set_sep, stdout);
+ } else {
+ status = show_null_message (format);
}
+ if (!status && format->message_set_sep)
+ fputs (format->message_set_sep, stdout);
+
status = show_messages (ctx,
format,
notmuch_message_get_replies (message),
@@ -984,7 +996,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused
(char *argv[]))
char *query_string;
int opt_index, ret;
const notmuch_show_format_t *format = _text;
-notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
+
+/* We abuse the notmuch_bool_t variable params.entire-thread by
+ * setting it to -1 to denote that the command line parsing has
+ * not set it. We ensure it is set to TRUE or FALSE before passing
+ * it to any other function.*/
+notmuch_show_params_t params = { .part = -1, .entire_thread = -1 };
+
int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
notmuch_bool_t verify = FALSE;
int exclude = EXCLUDE_TRUE;
@@ -1024,7 +1042,9 @@ notmuch_show_command (void *ctx, unused (int argc),
unused (char *argv[]))
switch (format_sel) {
case NOTMUCH_FORMAT_JSON:
format = _json;
- params.entire_thread = TRUE;
+ /* JSON defaults to entire-thread TRUE */
+ if (params.entire_thread == -1)
+ params.entire_thread = TRUE;
break;
case NOTMUCH_FORMAT_TEXT:
format = _text;
@@ -1046,6 +1066,10 @@ notmuch_show_command (void *ctx, unused (int argc),
unused (char *argv[]))
params.raw = TRUE;
break;
}
+/* Default is entire-thread = FALSE except for format=json which
+ * is dealt with above. */
+if (params.entire_thread == -1)
+ params.entire_thread = FALSE;
if (params.decrypt || verify) {
#ifdef GMIME_ATLEAST_26
--
1.7.9.1
[PATCH v3 0/2] Allow JSON to use non-entire thread, and use for elide
This is a rebased version of [1] with the bugfix [2] rolled in and the
style change suggested by Adam in [3].
I haven't added any tests yet: there do not seem to be any tests of
threading in JSON currently. I intend to rebase my show-tests [4] (and
can include/add tests for this). Alternatively there are Pieter's
tests [5] of the elide functionality which would test both patches.
Any suggestions on the preferred approach are gratefuly received!
Description of these patches from the previous version:
The first patch allows --entire-thread=false for notmuch-show.c when
the output format is JSON. In the previous version [2] Austin
suggested that we should output an empty message (i.e., {}) for
non-matching messages rather than just omitting them. This version
does that.
Note the first patch is entirely functional without the second.
The second patch uses the first to implement the "elide" functionality
in the emacs interface on the cli-side rather than on the emacs
side. This is substantially faster in some cases. In current emacs
show view it is a relatively small speed-up which is only noticable
with large threads. However, it will be used by notmuch-pick [3] and
there the speed up will be important. (I have a current version of
notmuch-pick which I will submit in the near future.)
[1] id:"1334077496-9172-1-git-send-email-markwalters1009 at gmail.com"
[2] id:"1334090884-13001-1-git-send-email-markwalters1009 at gmail.com"
[3] id:"CAMoJFUuXmLQR+awJnPKg8ZJAKbHbPAmSrEUVoFj_=dZ76WiKzw at mail.gmail.com"
[4] id:"1332171061-27983-1-git-send-email-markwalters1009 at gmail.com"
[5] id:"1329684990-12504-3-git-send-email-pieter at praet.org"
Mark Walters (2):
cli: make --entire-thread=false work for format=json.
emacs: make elide messages use notmuch-show for omitting messages.
emacs/notmuch-show.el | 18 ++
notmuch-show.c| 34 +-
2 files changed, 39 insertions(+), 13 deletions(-)
--
1.7.9.1
[PATCH] vim: fix regex after "notmuch show" output change
On Fri, Mar 30, 2012 at 1:02 AM, Jakob wrote:
> The new field "excluded" was added to the output and made this regex fail.
> ---
> ?vim/plugin/notmuch.vim | ? ?5 +++--
> ?1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/vim/plugin/notmuch.vim b/vim/plugin/notmuch.vim
> index 21985c7..8f27fb9 100644
> --- a/vim/plugin/notmuch.vim
> +++ b/vim/plugin/notmuch.vim
> @@ -48,7 +48,7 @@ let s:notmuch_defaults = {
> ? ? ? ? \ 'g:notmuch_show_part_end_regexp': ? ? ? ? ?' part}' ? ? ? ? ? ? ? ?
> ? ,
> ? ? ? ? \ 'g:notmuch_show_marker_regexp': ? ? ? ? ? ?'
> \\(message\\|header\\|body\\|attachment\\|part\\)[{}].*$',
> ? ? ? ? \
> - ? ? ? ?\ 'g:notmuch_show_message_parse_regexp': ? ? '\(id:[^ ]*\)
> depth:\([0-9]*\) match:\([0-9]*\) filename:\(.*\)$',
> + ? ? ? ?\ 'g:notmuch_show_message_parse_regexp': ? ? '\(id:[^ ]*\)
> depth:\([0-9]*\) match:\([0-9]*\) excluded:\([0-9]*\) filename:\(.*\)$',
> ? ? ? ? \ 'g:notmuch_show_tags_regexp': ? ? ? ? ? ? ?'(\([^)]*\))$' ? ? ? ? ?
> ? ? ,
> ? ? ? ? \
> ? ? ? ? \ 'g:notmuch_show_signature_regexp': ? ? ? ? '^\(-- \?\|_\+\)$' ? ? ?
> ? ? ,
> @@ -870,7 +870,8 @@ function! s:NM_cmd_show_parse(inlines)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? let msg['id'] = m[1]
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? let msg['depth'] = m[2]
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? let msg['match'] = m[3]
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?let msg['filename'] = m[4]
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?let msg['excluded'] = m[4]
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?let msg['filename'] = m[5]
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? endif
>
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? let in_message = 1
> --
Thanks. Pushed.
--
Felipe Contreras
[PATCH] vim: simplify build
On Wed, Apr 18, 2012 at 7:03 PM, Tomi Ollila wrote: > So, $(CURDIR) has 3 votes :D All right. Pushed :) -- Felipe Contreras
[PATCH v3 1/2] cli: make --entire-thread=false work for format=json.
The --entire-thread option in notmuch-show.c defaults to true when
format=json. Previously there was no way to turn this off. This patch
makes it respect --entire-thread=false.
The one subtlety is that we initialise a notmuch_bool_t to -1 to
indicate that the option parsing has not set it. This allows the code
to distinguish between the option being omitted from the command line,
and the option being set to false on the command line.
Finally, all formats except Json can output empty messages for non
entire-thread, but in Json format we need to output {} to keep the
other elements (e.g. the replies to this message) in the correct
place.
---
notmuch-show.c | 34 +-
1 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/notmuch-show.c b/notmuch-show.c
index da4a797..327c263 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -800,6 +800,16 @@ format_part_raw (unused (const void *ctx), mime_node_t
*node,
}
static notmuch_status_t
+show_null_message (const notmuch_show_format_t *format)
+{
+/* For all formats except json an empty message output is valid;
+ * for json we need the braces.*/
+if (format == format_json)
+ printf ({});
+return NOTMUCH_STATUS_SUCCESS;
+}
+
+static notmuch_status_t
show_message (void *ctx,
const notmuch_show_format_t *format,
notmuch_message_t *message,
@@ -862,11 +872,13 @@ show_messages (void *ctx,
if (status !res)
res = status;
next_indent = indent + 1;
-
- if (!status format-message_set_sep)
- fputs (format-message_set_sep, stdout);
+ } else {
+ status = show_null_message (format);
}
+ if (!status format-message_set_sep)
+ fputs (format-message_set_sep, stdout);
+
status = show_messages (ctx,
format,
notmuch_message_get_replies (message),
@@ -984,7 +996,13 @@ notmuch_show_command (void *ctx, unused (int argc), unused
(char *argv[]))
char *query_string;
int opt_index, ret;
const notmuch_show_format_t *format = format_text;
-notmuch_show_params_t params = { .part = -1, .omit_excluded = TRUE };
+
+/* We abuse the notmuch_bool_t variable params.entire-thread by
+ * setting it to -1 to denote that the command line parsing has
+ * not set it. We ensure it is set to TRUE or FALSE before passing
+ * it to any other function.*/
+notmuch_show_params_t params = { .part = -1, .entire_thread = -1 };
+
int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
notmuch_bool_t verify = FALSE;
int exclude = EXCLUDE_TRUE;
@@ -1024,7 +1042,9 @@ notmuch_show_command (void *ctx, unused (int argc),
unused (char *argv[]))
switch (format_sel) {
case NOTMUCH_FORMAT_JSON:
format = format_json;
- params.entire_thread = TRUE;
+ /* JSON defaults to entire-thread TRUE */
+ if (params.entire_thread == -1)
+ params.entire_thread = TRUE;
break;
case NOTMUCH_FORMAT_TEXT:
format = format_text;
@@ -1046,6 +1066,10 @@ notmuch_show_command (void *ctx, unused (int argc),
unused (char *argv[]))
params.raw = TRUE;
break;
}
+/* Default is entire-thread = FALSE except for format=json which
+ * is dealt with above. */
+if (params.entire_thread == -1)
+ params.entire_thread = FALSE;
if (params.decrypt || verify) {
#ifdef GMIME_ATLEAST_26
--
1.7.9.1
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 0/2] Allow JSON to use non-entire thread, and use for elide
This is a rebased version of [1] with the bugfix [2] rolled in and the
style change suggested by Adam in [3].
I haven't added any tests yet: there do not seem to be any tests of
threading in JSON currently. I intend to rebase my show-tests [4] (and
can include/add tests for this). Alternatively there are Pieter's
tests [5] of the elide functionality which would test both patches.
Any suggestions on the preferred approach are gratefuly received!
Description of these patches from the previous version:
The first patch allows --entire-thread=false for notmuch-show.c when
the output format is JSON. In the previous version [2] Austin
suggested that we should output an empty message (i.e., {}) for
non-matching messages rather than just omitting them. This version
does that.
Note the first patch is entirely functional without the second.
The second patch uses the first to implement the elide functionality
in the emacs interface on the cli-side rather than on the emacs
side. This is substantially faster in some cases. In current emacs
show view it is a relatively small speed-up which is only noticable
with large threads. However, it will be used by notmuch-pick [3] and
there the speed up will be important. (I have a current version of
notmuch-pick which I will submit in the near future.)
[1] id:1334077496-9172-1-git-send-email-markwalters1...@gmail.com
[2] id:1334090884-13001-1-git-send-email-markwalters1...@gmail.com
[3] id:CAMoJFUuXmLQR+awJnPKg8ZJAKbHbPAmSrEUVoFj_=dz76wi...@mail.gmail.com
[4] id:1332171061-27983-1-git-send-email-markwalters1...@gmail.com
[5] id:1329684990-12504-3-git-send-email-pie...@praet.org
Mark Walters (2):
cli: make --entire-thread=false work for format=json.
emacs: make elide messages use notmuch-show for omitting messages.
emacs/notmuch-show.el | 18 ++
notmuch-show.c| 34 +-
2 files changed, 39 insertions(+), 13 deletions(-)
--
1.7.9.1
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 7/7] python: wrap and use notmuch_database_destroy as destructor
Austin Clements amdra...@mit.edu writes: (I think it also doesn't make sense to expose notmuch_database_destroy as a general, public method since it will free all of the other C objects out from under the bindings, resulting in exactly the double free-type crashes that you're trying to avoid. It appears that none of the other Python classes have a destroy method.) Agreed, that sounds right to me. Sebastian pgpI1m9sTJOhd.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 1/2] cli: make --entire-thread=false work for format=json.
On Sat, Apr 21, 2012 at 03:15, Mark Walters markwalters1...@gmail.com wrote:
The --entire-thread option in notmuch-show.c defaults to true when
format=json. Previously there was no way to turn this off. This patch
makes it respect --entire-thread=false.
The one subtlety is that we initialise a notmuch_bool_t to -1 to
indicate that the option parsing has not set it. This allows the code
to distinguish between the option being omitted from the command line,
and the option being set to false on the command line.
Finally, all formats except Json can output empty messages for non
entire-thread, but in Json format we need to output {} to keep the
other elements (e.g. the replies to this message) in the correct
place.
LGTM. You can probably remove the relevant item from devel/TODO as well!
-- Adam
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] test: Replying to an HTML-only message in emacs
With the latest reply infrastructure, we should be able to nicely
quote HTML-only emails. But currently emacs quotes the raw HTML
instead of parsing it first. This commit adds a test for this case.
This test currently marked as broken.
---
test/emacs | 27 +++
1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/test/emacs b/test/emacs
index c7510e9..e648f80 100755
--- a/test/emacs
+++ b/test/emacs
@@ -444,6 +444,33 @@ Alex Botero-Lowry alex.boterolo...@gmail.com writes:
EOF
test_expect_equal_file OUTPUT EXPECTED
+test_begin_subtest Reply within emacs to an html-only message
+test_subtest_known_broken
+add_message '[content-type]=text/html' \
+ '[body]=Hi,br /This is an bHTML/b test message.br /br
/OK?'
+test_emacs (let ((message-hidden-headers '()))
+ (notmuch-show \id:${gen_msg_id}\)
+ (notmuch-show-reply)
+ (test-output))
+sed -i -e 's,^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX
Emacs/XXX,' OUTPUT
+cat EOF EXPECTED
+From: Notmuch Test Suite test_su...@notmuchmail.org
+To:
+Subject: Re: Reply within emacs to an html-only message
+In-Reply-To: ${gen_msg_id}
+Fcc: ${MAIL_DIR}/sent
+References: ${gen_msg_id}
+User-Agent: Notmuch/XXX Emacs/XXX
+--text follows this line--
+Notmuch Test Suite test_su...@notmuchmail.org writes:
+
+ Hi,
+ This is an HTML test message.
+
+ OK?
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
test_begin_subtest Quote MML tags in reply
message_id='test-emacs-mml-quot...@message.id'
add_message [id]=$message_id \
--
1.7.5.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/2] emacs: Correctly quote non-text/plain parts in reply
Quote non-text parts nicely by displaying them with mm-display-part before calling message-cite-original to quote them. HTML-only emails can now be quoted correctly. Mark the test for this feature as not broken. --- emacs/notmuch-mua.el | 20 +++- test/emacs |1 - 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el index 87bd88d..f7af789 100644 --- a/emacs/notmuch-mua.el +++ b/emacs/notmuch-mua.el @@ -21,6 +21,7 @@ (require 'json) (require 'message) +(require 'mm-view) (require 'format-spec) (require 'notmuch-lib) @@ -90,6 +91,19 @@ list. else if (notmuch-match-content-type (plist-get part :content-type) text/*) collect part)) +(defun notmuch-mua-insert-quotable-part (message part) + (save-restriction +(narrow-to-region (point) (point)) +(insert (notmuch-get-bodypart-content message part + (plist-get part :id) + notmuch-show-process-crypto)) +(let ((handle (mm-make-handle (current-buffer) + (list (plist-get part :content-type + (end-of-orig (point-max))) + (mm-display-part handle) + (kill-region (point-min) end-of-orig)) +(goto-char (point-max + ;; There is a bug in emacs 23's message.el that results in a newline ;; not being inserted after the References header, so the next header ;; is concatenated to the end of it. This function fixes the problem, @@ -169,11 +183,7 @@ list. ;; Get the parts of the original message that should be quoted; this includes ;; all the text parts, except the non-preferred ones in a multipart/alternative. (let ((quotable-parts (notmuch-mua-get-quotable-parts (plist-get original :body - (mapc (lambda (part) - (insert (notmuch-get-bodypart-content original part - (plist-get part :id) - notmuch-show-process-crypto))) - quotable-parts)) + (mapc (apply-partially 'notmuch-mua-insert-quotable-part original) quotable-parts)) (set-mark (point)) (goto-char start) diff --git a/test/emacs b/test/emacs index e648f80..579844f 100755 --- a/test/emacs +++ b/test/emacs @@ -445,7 +445,6 @@ EOF test_expect_equal_file OUTPUT EXPECTED test_begin_subtest Reply within emacs to an html-only message -test_subtest_known_broken add_message '[content-type]=text/html' \ '[body]=Hi,br /This is an bHTML/b test message.br /br /OK?' test_emacs (let ((message-hidden-headers '())) -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 0/2] Replying to HTML-only emails
Hi all, My recent reply enhancements were originally intended to allow proper quoting of HTML-only email in reply. While the final version was a big improvement on reply in general, it didn't actually acheive this goal. So, this series finishes that work, using mm-display-part to render the content of the original message correctly before inserting it into the reply message. I've also added a test for this feature. I'm not 100% happy with how the code looks for this, but I couldn't figure out a way to make it work without the kind of ugly narrow-to-region. If anyone has suggestions here I'd like to hear them. Adam Wolfe Gordon (2): test: Replying to an HTML-only message in emacs emacs: Correctly quote non-text/plain parts in reply emacs/notmuch-mua.el | 20 +++- test/emacs | 26 ++ 2 files changed, 41 insertions(+), 5 deletions(-) -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/3] new: Consistently treat fatal errors as fatal
Quoth Mark Walters on Apr 16 at 4:53 pm:
On Mon, 27 Feb 2012, Austin Clements amdra...@mit.edu wrote:
Previously, fatal errors in add_files_recursive were not treated as
fatal by its callers (including itself!) and add_files_recursive
sometimes returned errors on non-fatal conditions. This makes
add_files_recursive errors consistently fatal and updates all callers
to treat them as fatal.
Hi I have attempted to review this but am feeling a little out of my
depth. This patch seems fine except for one thing I am unsure about:
---
notmuch-new.c | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/notmuch-new.c b/notmuch-new.c
index 4f13535..bd9786a 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -308,7 +308,6 @@ add_files_recursive (notmuch_database_t *notmuch,
if (num_fs_entries == -1) {
fprintf (stderr, Error opening directory %s: %s\n,
path, strerror (errno));
- ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
If I understand this, then this change makes a failure to open a
directory non-fatal. In the comment before the function it says
* Also, we don't immediately act on file/directory removal since we
* must ensure that in the case of a rename that the new filename is
* added before the old filename is removed, (so that no information
* is lost from the database).
I am worried that we could fail to find some files because of the
file error above, and then we delete them from the database. Maybe this
could only happen if those emails have just been moved to this
file-error directory?
Hmm. This won't *actively* remove files, since that only happens if
we successfully scan a directory and find a message that's in the
database but not in that directory (*not* scanning the directory won't
add anything to the remove list). However, you are right that if a
message is moved from some other directory into a directory that we
fail to open, that message will be deleted by omission.
I've added the error back, along with a comment explaining it.
Best wishes
Mark
@@ -351,8 +350,10 @@ add_files_recursive (notmuch_database_t *notmuch,
next = talloc_asprintf (notmuch, %s/%s, path, entry-d_name);
status = add_files_recursive (notmuch, next, state);
- if (status ret == NOTMUCH_STATUS_SUCCESS)
+ if (status) {
ret = status;
+ goto DONE;
+ }
talloc_free (next);
next = NULL;
}
@@ -933,6 +934,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
}
ret = add_files (notmuch, db_path, add_files_state);
+if (ret)
+ goto DONE;
gettimeofday (tv_start, NULL);
for (f = add_files_state.removed_files-head; f !interrupted; f =
f-next) {
@@ -965,6 +968,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
}
}
+ DONE:
talloc_free (add_files_state.removed_files);
talloc_free (add_files_state.removed_directories);
talloc_free (add_files_state.directory_mtimes);
@@ -1012,10 +1016,9 @@ notmuch_new_command (void *ctx, int argc, char
*argv[])
printf (\n);
-if (ret) {
- printf (\nNote: At least one error was encountered: %s\n,
+if (ret)
+ printf (\nNote: A fatal error was encountered: %s\n,
notmuch_status_to_string (ret));
-}
notmuch_database_close (notmuch);
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/3] new: Handle fatal errors in remove_filename and _remove_directory
Quoth Mark Walters on Apr 16 at 5:02 pm:
On Mon, 27 Feb 2012, Austin Clements amdra...@mit.edu wrote:
Previously such errors were simply ignored. Now they cause an
immediate cleanup and abort.
This one looks fine except for a minor query.
---
notmuch-new.c | 24 ++--
1 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/notmuch-new.c b/notmuch-new.c
index bd9786a..0cbd479 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -780,8 +780,10 @@ remove_filename (notmuch_database_t *notmuch,
add_files_state-renamed_messages++;
if (add_files_state-synchronize_flags == TRUE)
notmuch_message_maildir_flags_to_tags (message);
-} else
+ status = NOTMUCH_STATUS_SUCCESS;
+} else if (status == NOTMUCH_STATUS_SUCCESS) {
add_files_state-removed_messages++;
+}
notmuch_message_destroy (message);
notmuch_database_end_atomic (notmuch);
return status;
@@ -789,12 +791,13 @@ remove_filename (notmuch_database_t *notmuch,
/* Recursively remove all filenames from the database referring to
* 'path' (or to any of its children). */
-static void
+static notmuch_status_t
_remove_directory (void *ctx,
notmuch_database_t *notmuch,
const char *path,
add_files_state_t *add_files_state)
{
+notmuch_status_t status;
notmuch_directory_t *directory;
notmuch_filenames_t *files, *subdirs;
char *absolute;
@@ -807,8 +810,10 @@ _remove_directory (void *ctx,
{
absolute = talloc_asprintf (ctx, %s/%s, path,
notmuch_filenames_get (files));
- remove_filename (notmuch, absolute, add_files_state);
+ status = remove_filename (notmuch, absolute, add_files_state);
talloc_free (absolute);
+ if (status)
+ return status;
}
for (subdirs = notmuch_directory_get_child_directories (directory);
@@ -817,11 +822,14 @@ _remove_directory (void *ctx,
{
absolute = talloc_asprintf (ctx, %s/%s, path,
notmuch_filenames_get (subdirs));
- _remove_directory (ctx, notmuch, absolute, add_files_state);
+ status = _remove_directory (ctx, notmuch, absolute, add_files_state);
talloc_free (absolute);
+ if (status)
+ return status;
}
notmuch_directory_destroy (directory);
+return NOTMUCH_STATUS_SUCCESS;
}
In the two return status lines above seem to mean we don't call
notmuch_directory_destroy. Does that matter?
Good point. I've fixed this to use the usual goto DONE cleanup idiom.
The other query is not actually about this patch: just something that
came up when reading it. Should notmuch_database_begin_atomic and
notmuch_database_end_atomic always be paired? One of the (existing)
error cases in remove_filename seems to return without calling end.
Yes, they should be. I've added a patch to fix that.
Best wishes
Mark
int
@@ -939,7 +947,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
gettimeofday (tv_start, NULL);
for (f = add_files_state.removed_files-head; f !interrupted; f =
f-next) {
- remove_filename (notmuch, f-filename, add_files_state);
+ ret = remove_filename (notmuch, f-filename, add_files_state);
+ if (ret)
+ goto DONE;
if (do_print_progress) {
do_print_progress = 0;
generic_print_progress (Cleaned up, messages,
@@ -950,7 +960,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
gettimeofday (tv_start, NULL);
for (f = add_files_state.removed_directories-head, i = 0; f
!interrupted; f = f-next, i++) {
- _remove_directory (ctx, notmuch, f-filename, add_files_state);
+ ret = _remove_directory (ctx, notmuch, f-filename, add_files_state);
+ if (ret)
+ goto DONE;
if (do_print_progress) {
do_print_progress = 0;
generic_print_progress (Cleaned up, directories,
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 0/4] Fix some error handling in notmuch new
Version 2 should address Mark's comments. It also adds a patch to fix an additional error handling error he pointed out in remove_filename. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 2/4] new: Handle fatal errors in remove_filename and _remove_directory
Previously such errors were simply ignored. Now they cause an
immediate cleanup and abort.
---
notmuch-new.c | 25 +++--
1 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/notmuch-new.c b/notmuch-new.c
index 15c0b36..92e0489 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -785,8 +785,10 @@ remove_filename (notmuch_database_t *notmuch,
add_files_state-renamed_messages++;
if (add_files_state-synchronize_flags == TRUE)
notmuch_message_maildir_flags_to_tags (message);
-} else
+ status = NOTMUCH_STATUS_SUCCESS;
+} else if (status == NOTMUCH_STATUS_SUCCESS) {
add_files_state-removed_messages++;
+}
notmuch_message_destroy (message);
notmuch_database_end_atomic (notmuch);
return status;
@@ -794,12 +796,13 @@ remove_filename (notmuch_database_t *notmuch,
/* Recursively remove all filenames from the database referring to
* 'path' (or to any of its children). */
-static void
+static notmuch_status_t
_remove_directory (void *ctx,
notmuch_database_t *notmuch,
const char *path,
add_files_state_t *add_files_state)
{
+notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
notmuch_directory_t *directory;
notmuch_filenames_t *files, *subdirs;
char *absolute;
@@ -812,8 +815,10 @@ _remove_directory (void *ctx,
{
absolute = talloc_asprintf (ctx, %s/%s, path,
notmuch_filenames_get (files));
- remove_filename (notmuch, absolute, add_files_state);
+ status = remove_filename (notmuch, absolute, add_files_state);
talloc_free (absolute);
+ if (status)
+ goto DONE;
}
for (subdirs = notmuch_directory_get_child_directories (directory);
@@ -822,11 +827,15 @@ _remove_directory (void *ctx,
{
absolute = talloc_asprintf (ctx, %s/%s, path,
notmuch_filenames_get (subdirs));
- _remove_directory (ctx, notmuch, absolute, add_files_state);
+ status = _remove_directory (ctx, notmuch, absolute, add_files_state);
talloc_free (absolute);
+ if (status)
+ goto DONE;
}
+ DONE:
notmuch_directory_destroy (directory);
+return NOTMUCH_STATUS_SUCCESS;
}
int
@@ -944,7 +953,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
gettimeofday (tv_start, NULL);
for (f = add_files_state.removed_files-head; f !interrupted; f =
f-next) {
- remove_filename (notmuch, f-filename, add_files_state);
+ ret = remove_filename (notmuch, f-filename, add_files_state);
+ if (ret)
+ goto DONE;
if (do_print_progress) {
do_print_progress = 0;
generic_print_progress (Cleaned up, messages,
@@ -955,7 +966,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
gettimeofday (tv_start, NULL);
for (f = add_files_state.removed_directories-head, i = 0; f
!interrupted; f = f-next, i++) {
- _remove_directory (ctx, notmuch, f-filename, add_files_state);
+ ret = _remove_directory (ctx, notmuch, f-filename, add_files_state);
+ if (ret)
+ goto DONE;
if (do_print_progress) {
do_print_progress = 0;
generic_print_progress (Cleaned up, directories,
--
1.7.9.1
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 4/4] new: Fix missing end_atomic in remove_filename on error
Previously, if we failed to find the message by filename in
remove_filename, we would return immediately from the function without
ending its atomic block. Now this code follows the usual goto DONE
idiom to perform cleanup.
---
notmuch-new.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/notmuch-new.c b/notmuch-new.c
index 2103b18..9eebea4 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -779,7 +779,8 @@ remove_filename (notmuch_database_t *notmuch,
return status;
status = notmuch_database_find_message_by_filename (notmuch, path,
message);
if (status || message == NULL)
- return status;
+ goto DONE;
+
status = notmuch_database_remove_message (notmuch, path);
if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) {
add_files_state-renamed_messages++;
@@ -790,6 +791,8 @@ remove_filename (notmuch_database_t *notmuch,
add_files_state-removed_messages++;
}
notmuch_message_destroy (message);
+
+ DONE:
notmuch_database_end_atomic (notmuch);
return status;
}
--
1.7.9.1
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 3/4] new: Print final fatal error message to stderr
This was going to stdout. I removed the newline at the beginning of printing the fatal error message because it wouldn't make sense if you were only looking at the stderr stream (e.g., you had redirected stdout to /dev/null). --- notmuch-new.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/notmuch-new.c b/notmuch-new.c index 92e0489..2103b18 100644 --- a/notmuch-new.c +++ b/notmuch-new.c @@ -1035,8 +1035,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[]) printf (\n); if (ret) - printf (\nNote: A fatal error was encountered: %s\n, - notmuch_status_to_string (ret)); + fprintf (stderr, Note: A fatal error was encountered: %s\n, +notmuch_status_to_string (ret)); notmuch_database_close (notmuch); -- 1.7.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
