[PATCH 2/2] cli: pick the user's address in a group list as from address

2012-01-14 Thread Jani Nikula
Messages received to a group list were not replied to using the from
address in the list. Fix it.

Signed-off-by: Jani Nikula 
---
 notmuch-reply.c |2 +-
 test/reply  |1 -
 2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index da3acce..0f682db 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -203,7 +203,7 @@ scan_address_list (InternetAddressList *list,
if (group_list == NULL)
continue;

-   n += scan_address_list (group_list, config, message, type, NULL);
+   n += scan_address_list (group_list, config, message, type, 
user_from);
} else {
InternetAddressMailbox *mailbox;
const char *name;
diff --git a/test/reply b/test/reply
index 196535a..e4e16eb 100755
--- a/test/reply
+++ b/test/reply
@@ -73,7 +73,6 @@ On Tue, 05 Jan 2010 15:43:56 -, Sender  wrote:
 > reply from alternate address"

 test_begin_subtest "Reply from address in named group list"
-test_subtest_known_broken
 add_message '[from]="Sender "' \
 '[to]=group:test_suite at notmuchmail.org,someone at 
example.com\;' \
  [cc]=test_suite_other at notmuchmail.org \
-- 
1.7.5.4



[PATCH 1/2] test: add known broken test for reply from address in named group list

2012-01-14 Thread Jani Nikula
If a message was received to the user's address that was in a named
group list, notmuch reply does not use that address for picking the
from address.

Groups lists are of the form: foo:bar at example.com,baz at example.com;

Signed-off-by: Jani Nikula 
---
 test/reply |   19 +++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/test/reply b/test/reply
index c0b8e26..196535a 100755
--- a/test/reply
+++ b/test/reply
@@ -72,6 +72,25 @@ References: <${gen_msg_id}>
 On Tue, 05 Jan 2010 15:43:56 -, Sender  wrote:
 > reply from alternate address"

+test_begin_subtest "Reply from address in named group list"
+test_subtest_known_broken
+add_message '[from]="Sender "' \
+'[to]=group:test_suite at notmuchmail.org,someone at 
example.com\;' \
+ [cc]=test_suite_other at notmuchmail.org \
+ [subject]=notmuch-reply-test \
+'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
+'[body]="Reply from address in named group list"'
+
+output=$(notmuch reply id:${gen_msg_id})
+test_expect_equal "$output" "From: Notmuch Test Suite 
+Subject: Re: notmuch-reply-test
+To: Sender , someone at example.com
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -, Sender  wrote:
+> Reply from address in named group list"
+
 test_begin_subtest "Support for Reply-To"
 add_message '[from]="Sender "' \
  [to]=test_suite at notmuchmail.org \
-- 
1.7.5.4



[PATCH 2/4] reply: Add a JSON reply format.

2012-01-14 Thread Jani Nikula
On Sun,  8 Jan 2012 00:52:40 -0700, Adam Wolfe Gordon  
wrote:
> From: Adam Wolfe Gordon 
> 
> This new JSON format for replies includes headers generated for a reply
> message as well as the headers and all text parts of the original message.
> Using this data, a client can intelligently create a reply. For example,
> the emacs client will be able to create replies with quoted HTML parts by
> parsing the HTML parts using w3m.

Hi Adam, this is a drive-by-review on some things I spotted, but can't
say I would've thought the whole thing through. I'm pretty ignorant
about MIME parts etc. Please find comments inline.

The reply-to-sender set was just merged. You'll have conflicts both here
and in emacs code, but they should be trivial to sort out.


BR,
Jani.


> ---
>  notmuch-reply.c |  269 
> +++
>  1 files changed, 230 insertions(+), 39 deletions(-)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index f8d5f64..82df396 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -30,6 +30,15 @@ reply_headers_message_part (GMimeMessage *message);
>  static void
>  reply_part_content (GMimeObject *part);
>  
> +static void
> +reply_part_start_json (GMimeObject *part, int *part_count);
> +
> +static void
> +reply_part_content_json (GMimeObject *part);
> +
> +static void
> +reply_part_end_json (GMimeObject *part);
> +

I know there are existing forward declarations like this, but would it
be much trouble to arrange the code so that they were not needed at all?

>  static const notmuch_show_format_t format_reply = {
>  "",
>   "", NULL,
> @@ -46,6 +55,22 @@ static const notmuch_show_format_t format_reply = {
>  ""
>  };
>  
> +static const notmuch_show_format_t format_json = {
> +"",
> + "", NULL,
> + "", NULL, NULL, "",
> + "",
> + reply_part_start_json,
> + NULL,
> + NULL,
> + reply_part_content_json,
> + reply_part_end_json,
> + "",
> + "",
> + "", "",
> +""
> +};
> +
>  static void
>  show_reply_headers (GMimeMessage *message)
>  {
> @@ -147,6 +172,78 @@ reply_part_content (GMimeObject *part)
>  }
>  }
>  
> +static void
> +reply_part_start_json (GMimeObject *part, unused(int *part_count))
> +{
> +GMimeContentType *content_type = g_mime_object_get_content_type 
> (GMIME_OBJECT (part));
> +GMimeContentDisposition *disposition = 
> g_mime_object_get_content_disposition (part);
> +
> +if (g_mime_content_type_is_type (content_type, "text", "*") &&
> + (!disposition ||
> +  strcmp (disposition->disposition, GMIME_DISPOSITION_INLINE) == 0))
> +{
> + printf("{ ");
> +}
> +}
> +
> +static void
> +reply_part_end_json (GMimeObject *part)
> +{
> +GMimeContentType *content_type = g_mime_object_get_content_type 
> (GMIME_OBJECT (part));
> +GMimeContentDisposition *disposition = 
> g_mime_object_get_content_disposition (part);
> +
> +if (g_mime_content_type_is_type (content_type, "text", "*") &&
> + (!disposition ||
> +  strcmp (disposition->disposition, GMIME_DISPOSITION_INLINE) == 0))
> + printf ("}, ");
> +}

The two functions above, while small, are almost identical. Please move
the common stuff to a common helper, and you can have something like
this:

if (the_common_function (part))
printf ("}, ");

> +
> +static void
> +reply_part_content_json (GMimeObject *part)
> +{
> +GMimeContentType *content_type = g_mime_object_get_content_type 
> (GMIME_OBJECT (part));
> +GMimeContentDisposition *disposition = 
> g_mime_object_get_content_disposition (part);
> +
> +void *ctx = talloc_new (NULL);
> +
> +/* We only care about inline text parts for reply purposes */
> +if (g_mime_content_type_is_type (content_type, "text", "*") &&
> + (!disposition ||
> +  strcmp (disposition->disposition, GMIME_DISPOSITION_INLINE) == 0))

Oh, you can use the common helper here too.

> +{
> + GMimeStream *stream_memory = NULL, *stream_filter = NULL;

No need to initialize stream_memory here.

> + GMimeDataWrapper *wrapper;
> + GByteArray *part_content;
> + const char *charset;
> +
> + printf("\"content-type\": %s, \"content\": ",
> +json_quote_str(ctx, 
> g_mime_content_type_to_string(content_type)));

The style in notmuch is to have a space before the opening brace in
function calls. Check elsewhere also. I always forget that too. :)

> +
> + charset = g_mime_object_get_content_type_parameter (part, "charset");

AFAICT charset declaration and the above call could be moved inside "if
(stream_memory)" below.

> + stream_memory = g_mime_stream_mem_new ();
> + if (stream_memory) {
> + stream_filter = g_mime_stream_filter_new(stream_memory);
> + if (charset) {
> + g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
> +   

[RFC] vim plugin rewrite II

2012-01-14 Thread David Bremner
On Sat, 14 Jan 2012 08:54:43 +0100, anton at khirnov.net wrote:
> 
> The advantages over current vim client are still the following:
> * sending and displaying/saving attachments
> * much better unicode support
> * tag name and search commands completion
> * proper representation of the thread structure
> * easier to extend thanks to python's massive standard library
> 
> Please comment.

Hi Anton;

I'm not a vim user, so I probably can't say anything very helpful, but
it does sound like some nice improvements.  Hopefully one of our vim
front-end users can comment a bit more.  One thing I wondered about is
if your version is still completely synchronous, with the resulting
problems dealing with large (say, more than 1 messages) search
results?

All the best,

David




[PATCH v3 10/10] test: new random message-id and tags dump/restore test.

2012-01-14 Thread David Bremner
From: David Bremner 

The idea is to generate a random dump file, and then generate minimal
mail messages from that.

Currently this relies on the behaviour of the gmime message-id parser
that if it cannot find a leading '<' in the Message-Id field, it just
copies the string without attempting to parse it. The alternative is
to settle for some weaker notion of round-tripping in this test.
---
 test/dump-restore |   25 +
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index 60ae68e..c354a44 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -160,4 +160,29 @@ test_begin_subtest 'format=sup, restore=default'
 notmuch dump --format=sup > OUTPUT.$test_count
 test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count

+test_begin_subtest 'random message-ids and tags'
+
+ count=0
+
+${TEST_DIRECTORY}/random-dump sort > EXPECTED.$test_count
+
+while read id tags ; do
+   file=${MAIL_DIR}/cur/dump-$count:2,
+   count=$((count + 1))
+   printf 'From: dump-test at example.com\n' > ${file}
+   printf 'To: example at example.net\n' > ${file}
+   printf 'Subject: StrangeMessageIDsAndTags\n' > ${file}
+   # the missing angle-braces are intentional, since they trigger
+   # gmime to pass the message-id through unparsed.
+   printf 'Message-Id: ' >> $file
+   ${TEST_DIRECTORY}/hex-xcode --direction=decode $id >> $file
+done < EXPECTED.$test_count
+
+notmuch new
+notmuch restore < EXPECTED.$test_count
+notmuch dump --format=notmuch -- subject:StrangeMessageIDsAndTags | \
+   sed 's/ *$//' | sort > OUTPUT.$test_count
+
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
 test_done
-- 
1.7.7.3



[PATCH v3 09/10] random-dump.c: new test-binary to generate dump files

2012-01-14 Thread David Bremner
From: David Bremner 

This binary creates a "torture test" dump file for the new dump
format.
---
 test/Makefile.local |4 ++
 test/basic  |2 +-
 test/random-dump.c  |  144 +++
 3 files changed, 149 insertions(+), 1 deletions(-)
 create mode 100644 test/random-dump.c

diff --git a/test/Makefile.local b/test/Makefile.local
index ba697f4..b59f837 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -16,6 +16,9 @@ $(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o 
util/libutil.a
 $(dir)/hex-xcode: $(dir)/hex-xcode.o command-line-arguments.o util/libutil.a
$(call quiet,CC) -I. $^ -o $@ -ltalloc

+$(dir)/random-dump:  $(dir)/random-dump.o command-line-arguments.o 
util/libutil.a
+   $(call quiet,CC) -I. $^ -o $@ -ltalloc -lm
+
 $(dir)/smtp-dummy: $(smtp_dummy_modules)
$(call quiet,CC) $^ -o $@

@@ -25,6 +28,7 @@ $(dir)/symbol-test: $(dir)/symbol-test.o
 .PHONY: test check

 test-binaries: $(dir)/arg-test $(dir)/hex-xcode \
+   $(dir)/random-dump \
 $(dir)/smtp-dummy $(dir)/symbol-test

 test:  all test-binaries
diff --git a/test/basic b/test/basic
index af57026..e3a6cef 100755
--- a/test/basic
+++ b/test/basic
@@ -54,7 +54,7 @@ test_begin_subtest 'Ensure that all available tests will be 
run by notmuch-test'
 eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test)
 tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
 available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -executable -printf 
'%f\n' | \
-sed -r -e 
"/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test|hex-xcode)$/d"
 | \
+sed -r -e 
"/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test|hex-xcode|random-dump)$/d"
 | \
 sort)
 test_expect_equal "$tests_in_suite" "$available"

diff --git a/test/random-dump.c b/test/random-dump.c
new file mode 100644
index 000..1949425
--- /dev/null
+++ b/test/random-dump.c
@@ -0,0 +1,144 @@
+/*
+   Generate a random dump file in 'notmuch' format.
+   Generated message-id's and tags are intentionally nasty.
+
+   We restrict ourselves to 7 bit message-ids, because generating
+   random valid UTF-8 seems like work. And invalid UTF-8 can't be
+   round-tripped via Xapian.
+
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "math.h"
+#include "hex-escape.h"
+#include "command-line-arguments.h"
+
+static void
+hex_out (void *ctx, char *buf)
+{
+static char *encoded_buf = NULL;
+static size_t encoded_buf_size = 0;
+
+if (hex_encode (ctx, buf, _buf, _buf_size) != HEX_SUCCESS) 
{
+   fprintf (stderr, "Hex encoding failed");
+   exit (1);
+}
+
+fputs (encoded_buf, stdout);
+}
+
+static void
+random_chars (char *buf, int from, int stop, int max_char,
+ const char *blacklist)
+{
+int i;
+
+for (i = from; i < stop; i++) {
+   do {
+   buf[i] = ' ' + (random () % (max_char - ' '));
+   } while (blacklist && strchr (blacklist, buf[i]));
+}
+}
+
+static void
+random_tag (void *ctx, size_t len)
+{
+static char *buf = NULL;
+static size_t buf_len = 0;
+
+int use = (random () % (len - 1)) + 1;
+
+if (len > buf_len) {
+   buf = talloc_realloc (ctx, buf, char, len);
+   buf_len = len;
+}
+
+random_chars (buf, 0, use, 255, NULL);
+
+buf[use] = '\0';
+
+hex_out (ctx, buf);
+}
+
+static void
+random_message_id (void *ctx, size_t len)
+{
+static char *buf = NULL;
+static size_t buf_len = 0;
+
+int lhs_len = (random () % (len / 2 - 1)) + 1;
+
+int rhs_len = (random () % len / 2) + 1;
+
+const char *blacklist = "\n\r@<>[]()";
+
+if (len > buf_len) {
+   buf = talloc_realloc (ctx, buf, char, len);
+   buf_len = len;
+}
+
+random_chars (buf, 0, lhs_len, 127, blacklist);
+
+buf[lhs_len] = '@';
+
+random_chars (buf, lhs_len + 1, lhs_len + rhs_len + 1, 127, blacklist);
+
+hex_out (ctx, buf);
+}
+
+int
+main (int argc, char **argv)
+{
+
+void *ctx = talloc_new (NULL);
+int num_lines = 500;
+int max_tags = 10;
+int message_id_len = 100;
+int tag_len = 50;
+int seed = 734569;
+
+int pad_tag = 0, pad_mid = 0;
+
+notmuch_opt_desc_t options[] = {
+   { NOTMUCH_OPT_INT, _lines, "num-lines", 'n', 0 },
+   { NOTMUCH_OPT_INT, _tags, "max-tags", 'm', 0 },
+   { NOTMUCH_OPT_INT, _id_len, "message-id-len", 'M', 0 },
+   { NOTMUCH_OPT_INT, _len, "tag-len", 't', 0 },
+   { NOTMUCH_OPT_INT, , "tag-len", 't', 0 },
+   { 0, 0, 0, 0, 0 }
+};
+
+int opt_index = parse_arguments (argc, argv, options, 1);
+
+if (opt_index < 0)
+   exit (1);
+
+pad_mid = ((int) log10 (num_lines) + 1);
+pad_tag = ((int) log10 (max_tags)) + 1;
+
+srandom (seed);
+
+int line;
+for (line = 0; line < num_lines; line++) {
+
+   printf ("%0*d-", pad_mid, line);
+
+   random_message_id (ctx, 

[PATCH v3 08/10] notmuch-{dump, restore}.1: document new format options

2012-01-14 Thread David Bremner
From: David Bremner 

More or less arbitrarily, notmuch-dump.1 gets the more detailed
description of the format.
---
 man/man1/notmuch-dump.1|   59 ++-
 man/man1/notmuch-restore.1 |   60 ++-
 2 files changed, 111 insertions(+), 8 deletions(-)

diff --git a/man/man1/notmuch-dump.1 b/man/man1/notmuch-dump.1
index 9ccf35d..f4f7964 100644
--- a/man/man1/notmuch-dump.1
+++ b/man/man1/notmuch-dump.1
@@ -5,7 +5,8 @@ notmuch-dump \- Creates a plain-text dump of the tags of each 
message.
 .SH SYNOPSIS

 .B "notmuch dump"
-.RI "[ <" filename "> ] [--]"
+.RI "[ <" filename "> ]"
+.RB  [ "\-\-format=(sup|notmuch)"  "] [--]"
 .RI "[ <" search-term ">...]"

 .SH DESCRIPTION
@@ -20,6 +21,62 @@ recreated from the messages themselves.  The output of 
notmuch dump is
 therefore the only critical thing to backup (and much more friendly to
 incremental backup than the native database files.)

+.TP 4
+.B \-\-format=(sup|notmuch)
+
+Notmuch restore supports two plain text dump formats, both with one message-id
+per line, followed by a list of tags.
+
+.RS 4
+.TP 4
+.B sup
+
+The
+.B sup
+dump file format is specifically chosen to be
+compatible with the format of files produced by sup-dump.
+So if you've previously been using sup for mail, then the
+.B "notmuch restore"
+command provides you a way to import all of your tags (or labels as
+sup calls them).
+Each line has the following form
+
+.RS 4
+.RI < message-id >
+.B (
+.RI < tag "> ..."
+.B )
+
+with zero or more tags are separated by spaces. Note that (malformed)
+message-ids may contained arbitrary non-null characters. Note also
+that tags with spaces will not be correctly restored with this format.
+
+.RE
+
+.RE
+.RS 4
+.TP 4
+.B notmuch
+
+The
+.B notmuch
+dump format is intended to more robust against malformed message-ids
+and tags containing whitespace or non-\fBascii\fR(7) characters.
+Each line has the form
+
+.RS 4
+.RI < encoded-message-id "> <" encoded-tag "> ..."
+
+where encoded means that every byte not matching the regex
+.B [A-Za-z0-9+-_@=.:,]
+is replace by
+.B %nn
+where nn is the two digit hex encoding.
+
+
+.RE
+
+
 With no search terms, a dump of all messages in the database will be
 generated.  A "--" argument instructs notmuch that the
 remaining arguments are search terms.
diff --git a/man/man1/notmuch-restore.1 b/man/man1/notmuch-restore.1
index 2191df0..3fb0e99 100644
--- a/man/man1/notmuch-restore.1
+++ b/man/man1/notmuch-restore.1
@@ -5,7 +5,7 @@ notmuch-restore \- Restores the tags from the given file (see 
notmuch dump).
 .SH SYNOPSIS

 .B "notmuch restore"
-.RB [ "--accumulate" ]
+.RI  [  options "...]"
 .RI "[ <" filename "> ]"

 .SH DESCRIPTION
@@ -15,21 +15,67 @@ Restores the tags from the given file (see

 The input is read from the given filename, if any, or from stdin.

-Note: The dump file format is specifically chosen to be
+
+Supported options for
+.B restore
+include
+.RS 4
+.TP 4
+.B \-\-accumulate
+
+The union of the existing and new tags is applied, instead of
+replacing each message's tags as they are read in from the dump file.
+
+.RE
+.RS 4
+.TP 4
+.B \-\-format=(sup|notmuch|auto)
+
+Notmuch restore supports two plain text dump formats, with one message-id
+per line, followed by a list of tags.
+For details of the actual formats, see \fBnotmuch-dump\fR(1).
+
+.RS 4
+.TP 4
+.B sup
+
+The
+.B sup
+dump file format is specifically chosen to be
 compatible with the format of files produced by sup-dump.
 So if you've previously been using sup for mail, then the
 .B "notmuch restore"
 command provides you a way to import all of your tags (or labels as
 sup calls them).

-The --accumulate switch causes the union of the existing and new tags to be
-applied, instead of replacing each message's tags as they are read in from the
-dump file.
+.RE
+.RS 4
+.TP 4
+.B notmuch

-See \fBnotmuch-search-terms\fR(7)
-for details of the supported syntax for .
+The
+.B notmuch
+dump format is intended to more robust against malformed message-ids
+and tags containing whitespace or non-\fBascii\fR(7) characters.  This
+format hex-escapes all characters those outside of a small character
+set, intended to be suitable for e.g. pathnames in most UNIX-like
+systems.

 .RE
+
+.RS 4
+.TP 4
+.B auto
+
+This option (the default) tries to guess the format from the
+input. For correctly formed input in either supported format, this
+heuristic, based the fact that notmuch format contains no parentheses,
+should be accurate.
+
+.RE
+
+.RE
+
 .SH SEE ALSO

 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-- 
1.7.7.3



[PATCH v3 07/10] test: second set of dump/restore --format=notmuch tests

2012-01-14 Thread David Bremner
From: David Bremner 

These one need the completed functionality in notmuch-restore. Fairly
exotic tags are tested, but no weird message id's.

We test each possible input to autodetection, both explicit (with
--format=auto) and implicit (without --format).
---
 test/dump-restore |   66 +
 1 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index 0e210d3..60ae68e 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -94,4 +94,70 @@ notmuch dump --format=sup -- from:cworth | tr -d \(\) > 
EXPECTED.$test_count
 notmuch dump --format=notmuch -- from:cworth > OUTPUT.$test_count
 test_expect_equal_file OUTPUT.$test_count EXPECTED.$test_count

+test_begin_subtest "format=notmuch, # round-trip"
+notmuch dump --format=sup | sort > EXPECTED.$test_count
+notmuch dump --format=notmuch | notmuch restore --format=notmuch
+notmuch dump --format=sup | sort > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+tag1='comic_swear=$&^%$^%\\//-+$^%$'
+enc1=$($TEST_DIRECTORY/hex-xcode --direction=encode "$tag1")
+
+tag2=$(printf 'this\n tag\t has\n spaces')
+enc2=$($TEST_DIRECTORY/hex-xcode --direction=encode "$tag2")
+
+enc3='%c3%91%c3%a5%c3%b0%c3%a3%c3%a5%c3%a9-%c3%8f%c3%8a'
+tag3=$($TEST_DIRECTORY/hex-xcode --direction=decode $enc3)
+
+notmuch dump --format=notmuch > BACKUP
+
+notmuch tag +"$tag1" +"$tag2" +"$tag3" -inbox -unread "*"
+
+test_begin_subtest 'format=notmuch, round trip with strange tags'
+notmuch dump --format=notmuch > EXPECTED.$test_count
+notmuch dump --format=notmuch | notmuch restore --format=notmuch
+notmuch dump --format=notmuch > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=notmuch, checking encoded output'
+cp /dev/null EXPECTED.$test_count
+notmuch dump --format=notmuch -- from:cworth |\
+awk "{ print \$1 \" $enc1 $enc2 $enc3\" }" > EXPECTED.$test_count
+
+notmuch dump --format=notmuch -- from:cworth  > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'restoring sane tags'
+notmuch restore --format=notmuch < BACKUP
+notmuch dump --format=notmuch > OUTPUT.$test_count
+test_expect_equal_file BACKUP OUTPUT.$test_count
+
+test_begin_subtest 'format=notmuch, restore=auto'
+notmuch dump --format=notmuch > EXPECTED.$test_count
+notmuch tag -inbox -unread "*"
+notmuch restore --format=auto < EXPECTED.$test_count
+notmuch dump --format=notmuch > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=sup, restore=auto'
+notmuch dump --format=sup > EXPECTED.$test_count
+notmuch tag -inbox -unread "*"
+notmuch restore --format=auto < EXPECTED.$test_count
+notmuch dump --format=sup > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=notmuch, restore=default'
+notmuch dump --format=notmuch > EXPECTED.$test_count
+notmuch tag -inbox -unread "*"
+notmuch restore < EXPECTED.$test_count
+notmuch dump --format=notmuch > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest 'format=sup, restore=default'
+notmuch dump --format=sup > EXPECTED.$test_count
+notmuch tag -inbox -unread "*"
+notmuch restore < EXPECTED.$test_count
+notmuch dump --format=sup > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
 test_done
-- 
1.7.7.3



[PATCH v3 06/10] notmuch-restore: add 'notmuch format' support, auto detect

2012-01-14 Thread David Bremner
From: David Bremner 

This is format is whitespace separated tokens, encoded by
util/hex-escape.c

The format detection heuristic relies on the fact that '(' is not part
of the character set used by hex-escape. Since hex-escape is designed
to be OK for pathnames (and shells), this seems like a reasonable
assumption.

In principle the --format argument to notmuch-restore is notmuch
needed at this point, but it adds literally 5 lines of argument
description, so I left it.
---
 dump-restore-private.h |5 +-
 notmuch-restore.c  |  111 ++-
 2 files changed, 92 insertions(+), 24 deletions(-)

diff --git a/dump-restore-private.h b/dump-restore-private.h
index 34a5022..67795e5 100644
--- a/dump-restore-private.h
+++ b/dump-restore-private.h
@@ -5,8 +5,9 @@
 #include "command-line-arguments.h"

 typedef enum dump_formats {
-DUMP_FORMAT_SUP,
-DUMP_FORMAT_NOTMUCH
+DUMP_FORMAT_AUTO,
+DUMP_FORMAT_NOTMUCH,
+DUMP_FORMAT_SUP
 } dump_format_t;

 #endif
diff --git a/notmuch-restore.c b/notmuch-restore.c
index 87d9772..3fdfecc 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -19,6 +19,7 @@
  */

 #include "notmuch-client.h"
+#include "dump-restore-private.h"

 int
 notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
@@ -35,6 +36,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 regex_t regex;
 int rerr;
 int opt_index;
+int input_format = DUMP_FORMAT_AUTO;

 config = notmuch_config_open (ctx, NULL, NULL);
 if (config == NULL)
@@ -48,6 +50,11 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
 synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);

 notmuch_opt_desc_t options[] = {
+   { NOTMUCH_OPT_KEYWORD, _format, "format", 'f',
+ (notmuch_keyword_t []){ { "auto", DUMP_FORMAT_AUTO },
+ { "notmuch", DUMP_FORMAT_NOTMUCH },
+ { "sup", DUMP_FORMAT_SUP },
+ {0, 0} } },
{ NOTMUCH_OPT_POSITION, _file_name, 0, 0, 0 },
{ NOTMUCH_OPT_BOOLEAN,  , "accumulate", 'a', 0 },
{ 0, 0, 0, 0, 0 }
@@ -77,37 +84,85 @@ notmuch_restore_command (unused (void *ctx), int argc, char 
*argv[])
return 1;
 }

-/* Dump output is one line per message. We match a sequence of
- * non-space characters for the message-id, then one or more
- * spaces, then a list of space-separated tags as a sequence of
- * characters within literal '(' and ')'. */
-if ( xregcomp (,
-  "^([^ ]+) \\(([^)]*)\\)$",
-  REG_EXTENDED) )
-   INTERNAL_ERROR("compile time constant regex failed.");
+
+/* These are out here to re-use the buffers with hex_decode */
+
+char *message_id = NULL;
+size_t message_id_size = 0;
+char *tag = NULL;
+size_t tag_size = 0;
+notmuch_bool_t first_line = TRUE;

 while ((line_len = getline (, _size, input)) != -1) {
regmatch_t match[3];
-   char *message_id, *file_tags, *tag, *next;
+   char  *file_tags, *next;
notmuch_message_t *message = NULL;
+
notmuch_status_t status;
notmuch_tags_t *db_tags;
char *db_tags_str;

chomp_newline (line);
+   if (first_line && input_format == DUMP_FORMAT_AUTO) {
+   char *p;

-   rerr = xregexec (, line, 3, match, 0);
-   if (rerr == REG_NOMATCH)
-   {
-   fprintf (stderr, "Warning: Ignoring invalid input line: %s\n",
-line);
+   for (p = line; *p; p++) {
+   if (*p == '(')
+   input_format = DUMP_FORMAT_SUP;
+   }
+
+   if (input_format == DUMP_FORMAT_AUTO)
+   input_format = DUMP_FORMAT_NOTMUCH;
+
+   }
+
+   /* sup dump output is one line per message. We match a
+* sequence of non-space characters for the message-id, then
+* one or more spaces, then a list of space-separated tags as
+* a sequence of characters within literal '(' and ')'. */
+   if (first_line && input_format == DUMP_FORMAT_SUP) {
+   if ( xregcomp (,
+  "^([^ ]+) \\(([^)]*)\\)$",
+  REG_EXTENDED) )
+   INTERNAL_ERROR("compile time constant regex failed.");
+   }
+
+
+   /* Silently ignore blank lines */
+
+   if (line[0] == '\0') {
continue;
}

-   message_id = xstrndup (line + match[1].rm_so,
-  match[1].rm_eo - match[1].rm_so);
-   file_tags = xstrndup (line + match[2].rm_so,
- match[2].rm_eo - match[2].rm_so);
+   if (input_format == DUMP_FORMAT_SUP) {
+   rerr = xregexec (, line, 3, match, 0);
+   if (rerr == REG_NOMATCH)
+   {
+   fprintf (stderr, "Warning: Ignoring invalid input line: %s\n",
+ 

[PATCH v3 05/10] test: add test for dump --format=notmuch

2012-01-14 Thread David Bremner
From: David Bremner 

The first test is really to test our assumptions about the corpus,
namely that a certain set of message-id's is safe (i.e. doesn't change
under hex-escaping).  We then check dump output as best we can without
functionality-to-come in notmuch-restore.
---
 test/dump-restore |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index 439e998..0e210d3 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -82,4 +82,16 @@ test_begin_subtest "dump outfile -- from:cworth"
 notmuch dump dump-outfile-dash-inbox.actual -- from:cworth
 test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual

+test_begin_subtest "Check for a safe set of message-ids"
+notmuch search --output=messages from:cworth > EXPECTED.$test_count
+notmuch search --output=messages from:cworth |\
+   $TEST_DIRECTORY/hex-xcode --direction=encode > OUTPUT.$test_count
+test_expect_equal_file OUTPUT.$test_count EXPECTED.$test_count
+
+# we have observed that cworth has sane message-ids, and hopefully sane tags.
+test_begin_subtest "dump --format=notmuch -- from:cworth"
+notmuch dump --format=sup -- from:cworth | tr -d \(\) > EXPECTED.$test_count
+notmuch dump --format=notmuch -- from:cworth > OUTPUT.$test_count
+test_expect_equal_file OUTPUT.$test_count EXPECTED.$test_count
+
 test_done
-- 
1.7.7.3



[PATCH v3 04/10] notmuch-dump: add --format=(notmuch|sup)

2012-01-14 Thread David Bremner
From: David Bremner 

sup is the old format, and remains the default.

Each line of the notmuch format is "msg_id tag tag...tag" where each
space seperated token is 'hex-encoded' to remove troubling characters.
In particular this format won't have the same problem with e.g. spaces
in message-ids or tags; they will be round-trip-able.
---
 dump-restore-private.h |   12 
 notmuch-dump.c |   47 +++
 2 files changed, 51 insertions(+), 8 deletions(-)
 create mode 100644 dump-restore-private.h

diff --git a/dump-restore-private.h b/dump-restore-private.h
new file mode 100644
index 000..34a5022
--- /dev/null
+++ b/dump-restore-private.h
@@ -0,0 +1,12 @@
+#ifndef DUMP_RESTORE_PRIVATE_H
+#define DUMP_RESTORE_PRIVATE_H
+
+#include "hex-escape.h"
+#include "command-line-arguments.h"
+
+typedef enum dump_formats {
+DUMP_FORMAT_SUP,
+DUMP_FORMAT_NOTMUCH
+} dump_format_t;
+
+#endif
diff --git a/notmuch-dump.c b/notmuch-dump.c
index a735875..0231db2 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -19,6 +19,7 @@
  */

 #include "notmuch-client.h"
+#include "dump-restore-private.h"

 int
 notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
@@ -44,9 +45,15 @@ notmuch_dump_command (unused (void *ctx), int argc, char 
*argv[])
 char *output_file_name = NULL;
 int opt_index;

+int output_format = DUMP_FORMAT_SUP;
+
 notmuch_opt_desc_t options[] = {
-   { NOTMUCH_OPT_POSITION, _file_name, 0, 0, 0  },
-   { 0, 0, 0, 0, 0 }
+   { NOTMUCH_OPT_KEYWORD, _format, "format", 'f',
+ (notmuch_keyword_t []){ { "sup", DUMP_FORMAT_SUP },
+ { "notmuch", DUMP_FORMAT_NOTMUCH },
+ {0, 0} } },
+   { NOTMUCH_OPT_POSITION, _file_name, 0, 0, 0 },
+   { 0,0, 0, 0, 0 }
 };

 opt_index = parse_arguments (argc, argv, options, 1);
@@ -85,29 +92,53 @@ notmuch_dump_command (unused (void *ctx), int argc, char 
*argv[])
  */
 notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED);

+char *buffer = NULL;
+size_t buffer_size = 0;
+
 for (messages = notmuch_query_search_messages (query);
 notmuch_messages_valid (messages);
 notmuch_messages_move_to_next (messages))
 {
int first = 1;
-   message = notmuch_messages_get (messages);
+   const char *message_id;

-   fprintf (output,
-"%s (", notmuch_message_get_message_id (message));
+   message = notmuch_messages_get (messages);
+   message_id = notmuch_message_get_message_id (message);
+
+   if (output_format == DUMP_FORMAT_SUP) {
+   fprintf (output, "%s (", message_id);
+   } else {
+   if (hex_encode (notmuch, message_id,
+   , _size) != HEX_SUCCESS)
+   return 1;
+   fprintf (output, "%s ", buffer);
+   }

for (tags = notmuch_message_get_tags (message);
 notmuch_tags_valid (tags);
 notmuch_tags_move_to_next (tags))
{
+   const char *tag_str = notmuch_tags_get (tags);
+
if (! first)
-   fprintf (output, " ");
+   fputs (" ", output);

-   fprintf (output, "%s", notmuch_tags_get (tags));
+   if (output_format == DUMP_FORMAT_SUP) {
+   fputs (tag_str, output);
+   } else {
+   if (hex_encode (notmuch, tag_str,
+   , _size) != HEX_SUCCESS)
+   return 1;

+   fputs (buffer, output);
+   }
first = 0;
}

-   fprintf (output, ")\n");
+   if (output_format == DUMP_FORMAT_SUP)
+   fputs (")\n", output);
+   else
+   fputs ("\n", output);

notmuch_message_destroy (message);
 }
-- 
1.7.7.3



[PATCH v3 03/10] test/hex-escaping: new test for hex escaping routines

2012-01-14 Thread David Bremner
From: David Bremner 

These are more like unit tests, to (try to) make sure the library
functionality is working before building more complicated things on
top of it.
---
 test/hex-escaping |   26 ++
 test/notmuch-test |1 +
 2 files changed, 27 insertions(+), 0 deletions(-)
 create mode 100755 test/hex-escaping

diff --git a/test/hex-escaping b/test/hex-escaping
new file mode 100755
index 000..f34cc8c
--- /dev/null
+++ b/test/hex-escaping
@@ -0,0 +1,26 @@
+#!/usr/bin/env bash
+test_description="hex encoding and decoding"
+. ./test-lib.sh
+
+test_begin_subtest "round trip"
+find $TEST_DIRECTORY/corpus -type f -print | sort | xargs cat > EXPECTED
+$TEST_DIRECTORY/hex-xcode --direction=encode < EXPECTED | 
$TEST_DIRECTORY/hex-xcode --direction=decode > OUTPUT
+test_expect_equal_file OUTPUT EXPECTED
+
+test_begin_subtest "punctuation"
+tag1='comic_swear=$&^%$^%\\//-+$^%$'
+tag_enc1=$($TEST_DIRECTORY/hex-xcode --direction=encode "$tag1")
+test_expect_equal "$tag_enc1" 
"comic_swear=%24%26%5e%25%24%5e%25%5c%5c%2f%2f-+%24%5e%25%24"
+
+test_begin_subtest "round trip newlines"
+printf 'this\n tag\t has\n spaces\n' > EXPECTED.$test_count
+$TEST_DIRECTORY/hex-xcode --direction=encode  < EXPECTED.$test_count |\
+   $TEST_DIRECTORY/hex-xcode --direction=decode > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+
+test_begin_subtest "round trip 8bit chars"
+echo '%c3%91%c3%a5%c3%b0%c3%a3%c3%a5%c3%a9-%c3%8f%c3%8a' > EXPECTED.$test_count
+$TEST_DIRECTORY/hex-xcode --direction=decode  < EXPECTED.$test_count |\
+   $TEST_DIRECTORY/hex-xcode --direction=encode > OUTPUT.$test_count
+test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index 6a99ae3..cbe2ef9 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -52,6 +52,7 @@ TESTS="
   python
   hooks
   argument-parsing
+  hex-escaping
 "
 TESTS=${NOTMUCH_TESTS:=$TESTS}

-- 
1.7.7.3



[PATCH v3 02/10] test/hex-xcode: new test binary

2012-01-14 Thread David Bremner
From: David Bremner 

This program is used both as a test-bed/unit-tester for
../util/hex-escape.c, and also as a utility in future tests of dump
and restore.
---
 test/.gitignore |1 +
 test/Makefile.local |6 ++-
 test/basic  |2 +-
 test/hex-xcode.c|  103 +++
 4 files changed, 110 insertions(+), 2 deletions(-)
 create mode 100644 test/hex-xcode.c

diff --git a/test/.gitignore b/test/.gitignore
index e63c689..be7ab5e 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -3,4 +3,5 @@ corpus.mail
 smtp-dummy
 symbol-test
 arg-test
+hex-xcode
 tmp.*
diff --git a/test/Makefile.local b/test/Makefile.local
index fa2df73..ba697f4 100644
--- a/test/Makefile.local
+++ b/test/Makefile.local
@@ -13,6 +13,9 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o)
 $(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o util/libutil.a
$(call quiet,CC) -I. $^ -o $@

+$(dir)/hex-xcode: $(dir)/hex-xcode.o command-line-arguments.o util/libutil.a
+   $(call quiet,CC) -I. $^ -o $@ -ltalloc
+
 $(dir)/smtp-dummy: $(smtp_dummy_modules)
$(call quiet,CC) $^ -o $@

@@ -21,7 +24,8 @@ $(dir)/symbol-test: $(dir)/symbol-test.o

 .PHONY: test check

-test-binaries: $(dir)/arg-test $(dir)/smtp-dummy $(dir)/symbol-test
+test-binaries: $(dir)/arg-test $(dir)/hex-xcode \
+$(dir)/smtp-dummy $(dir)/symbol-test

 test:  all test-binaries
@${dir}/notmuch-test $(OPTIONS)
diff --git a/test/basic b/test/basic
index d6aed24..af57026 100755
--- a/test/basic
+++ b/test/basic
@@ -54,7 +54,7 @@ test_begin_subtest 'Ensure that all available tests will be 
run by notmuch-test'
 eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test)
 tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
 available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -executable -printf 
'%f\n' | \
-sed -r -e 
"/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test)$/d"
 | \
+sed -r -e 
"/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test|hex-xcode)$/d"
 | \
 sort)
 test_expect_equal "$tests_in_suite" "$available"

diff --git a/test/hex-xcode.c b/test/hex-xcode.c
new file mode 100644
index 000..eec6541
--- /dev/null
+++ b/test/hex-xcode.c
@@ -0,0 +1,103 @@
+/* No, nothing to to with IDE from Apple Inc.
+   testbed for ../util/hex-escape.c.
+
+   usage:
+   hex-xcode [--direction=(encode|decode)] [--omit-newline] < file
+   hex-xcode [--direction=(encode|decode)] [--omit-newline] arg1 arg2 arg3 ...
+
+ */
+
+#include "notmuch-client.h"
+#include "hex-escape.h"
+#include 
+
+
+enum direction {
+ENCODE,
+DECODE
+};
+
+static int
+xcode (void *ctx, enum direction dir, char *in, char **buf_p, size_t *size_p)
+{
+hex_status_t status;
+
+if (dir == ENCODE)
+   status = hex_encode (ctx, in, buf_p, size_p);
+else
+   status = hex_decode (ctx, in, buf_p, size_p);
+
+if (status == HEX_SUCCESS)
+   fputs (*buf_p, stdout);
+
+return status;
+}
+
+
+int
+main (int argc, char **argv)
+{
+
+
+enum direction dir = DECODE;
+int omit_newline = FALSE;
+
+notmuch_opt_desc_t options[] = {
+   { NOTMUCH_OPT_KEYWORD, , "direction", 'd',
+ (notmuch_keyword_t []){ { "encode", ENCODE },
+ { "decode", DECODE },
+ { 0, 0 } } },
+   { NOTMUCH_OPT_BOOLEAN, _newline, "omit-newline", 'n', 0 },
+   { 0, 0, 0, 0, 0 }
+};
+
+int opt_index = parse_arguments (argc, argv, options, 1);
+
+if (opt_index < 0)
+   exit (1);
+
+void *ctx = talloc_new (NULL);
+
+char *line = NULL;
+size_t line_size;
+ssize_t line_len;
+
+char *buffer = NULL;
+size_t buf_size = 0;
+
+notmuch_bool_t read_stdin = TRUE;
+
+for (; opt_index < argc; opt_index++) {
+
+   if (xcode (ctx, dir, argv[opt_index],
+  , _size) != HEX_SUCCESS)
+   return 1;
+
+   if (!omit_newline)
+   putchar ('\n');
+
+   read_stdin = FALSE;
+}
+
+if (!read_stdin)
+   return 0;
+
+while ((line_len = getline (, _size, stdin)) != -1) {
+
+   chomp_newline (line);
+
+   if (xcode (ctx, dir, line, , _size) != HEX_SUCCESS)
+   return 1;
+
+   if (!omit_newline)
+   putchar ('\n');
+
+}
+
+if (line)
+   free (line);
+
+talloc_free (ctx);
+
+return 0;
+}
-- 
1.7.7.3



[PATCH v3 01/10] hex-escape: (en|de)code strings to/from restricted character set

2012-01-14 Thread David Bremner
From: David Bremner 

The character set is chosen to be suitable for pathnames, and the same
as that used by contrib/nmbug
---
 util/Makefile.local |2 +-
 util/hex-escape.c   |  156 +++
 util/hex-escape.h   |   32 +++
 3 files changed, 189 insertions(+), 1 deletions(-)
 create mode 100644 util/hex-escape.c
 create mode 100644 util/hex-escape.h

diff --git a/util/Makefile.local b/util/Makefile.local
index 26e4c3f..2e63932 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -3,7 +3,7 @@
 dir := util
 extra_cflags += -I$(srcdir)/$(dir)

-libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c
+libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c

 libutil_modules := $(libutil_c_srcs:.c=.o)

diff --git a/util/hex-escape.c b/util/hex-escape.c
new file mode 100644
index 000..6c1260b
--- /dev/null
+++ b/util/hex-escape.c
@@ -0,0 +1,156 @@
+/* hex-escape.c -  Manage encoding and decoding of byte strings into path names
+ *
+ * Copyright (c) 2011 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner 
+ */
+
+#include 
+#include 
+#include 
+#include "error_util.h"
+#include "hex-escape.h"
+
+static const size_t default_buf_size = 1024;
+
+static const char *output_charset =
+"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+-_@=.:,";
+
+static const int escape_char = '%';
+
+static int
+is_output (char c)
+{
+return (strchr (output_charset, c) != NULL);
+}
+
+static int
+maybe_realloc (void *ctx, size_t needed, char **out, size_t *out_size)
+{
+if (*out_size < needed) {
+
+   if (*out == NULL)
+   *out = talloc_size (ctx, needed);
+   else
+   *out = talloc_realloc (ctx, *out, char, needed);
+
+   if (*out == NULL)
+   return 0;
+
+   *out_size = needed;
+}
+return 1;
+}
+
+hex_status_t
+hex_encode (void *ctx, const char *in, char **out, size_t *out_size)
+{
+
+const unsigned char *p;
+char *q;
+
+size_t escape_count = 0;
+size_t len = 0;
+size_t needed;
+
+assert (ctx); assert (in); assert (out); assert (out_size);
+
+for (p = (unsigned char *) in; *p; p++) {
+   escape_count += (!is_output (*p));
+   len++;
+}
+
+needed = len + escape_count * 2 + 1;
+
+if (*out == NULL)
+   *out_size = 0;
+
+if (!maybe_realloc (ctx, needed, out, out_size))
+   return HEX_OUT_OF_MEMORY;
+
+q = *out;
+p = (unsigned char *) in;
+
+while (*p) {
+   if (is_output (*p)) {
+   *q++ = *p++;
+   } else {
+   sprintf (q, "%%%02x", *p++);
+   q += 3;
+   }
+}
+
+*q = '\0';
+return HEX_SUCCESS;
+}
+
+hex_status_t
+hex_decode (void *ctx, const char *in, char **out, size_t * out_size)
+{
+
+char buf[3];
+
+const char *p;
+unsigned char *q;
+
+size_t escape_count = 0;
+size_t needed = 0;
+
+assert (ctx); assert (in); assert (out); assert (out_size);
+
+size_t len = strlen (in);
+
+for (p = in; *p; p++)
+   escape_count += (*p == escape_char);
+
+needed = len - escape_count * 2 + 1;
+
+if (!maybe_realloc (ctx, needed, out, out_size))
+   return HEX_OUT_OF_MEMORY;
+
+p = in;
+q = (unsigned char *) *out;
+buf[2] = 0;
+
+while (*p) {
+
+   if (*p == escape_char) {
+
+   char *endp;
+
+   if (len < 3)
+   return HEX_SYNTAX_ERROR;
+
+   buf[0] = p[1];
+   buf[1] = p[2];
+
+   *q = strtol (buf, , 16);
+
+   if (endp != buf + 2)
+   return HEX_SYNTAX_ERROR;
+
+   len -= 3;
+   p += 3;
+   q++;
+   } else {
+   *q++ = *p++;
+   }
+}
+
+*q = '\0';
+
+return HEX_SUCCESS;
+}
diff --git a/util/hex-escape.h b/util/hex-escape.h
new file mode 100644
index 000..e409626
--- /dev/null
+++ b/util/hex-escape.h
@@ -0,0 +1,32 @@
+#ifndef _HEX_ESCAPE_H
+#define _HEX_ESCAPE_H
+
+typedef enum hex_status {
+HEX_SUCCESS = 0,
+HEX_SYNTAX_ERROR,
+HEX_OUT_OF_MEMORY
+} hex_status_t;
+
+/*
+ * The API is modelled on that for getline.
+ *
+ * If 'out' points to a NULL pointer a char array of the appropriate
+ * size is allocated using talloc, and out_size is updated.
+ *
+ * If 'out' points to a non-NULL pointer, it assumed to describe 

New dump/restore format.

2012-01-14 Thread David Bremner
This version of this series add fairly extensive testing with strange
message ids full of spaces and punctuation, and some documentation.



The overloading of show (was Re: [PATCH] Output unmodified Content-Type header value for JSON format.)

2012-01-14 Thread Austin Clements
(was in reply to id:87ehv2proa.fsf at praet.org, but I wanted to start a
new top-level thread)

Quoth Pieter Praet on Jan 14 at 10:19 am:
> On Thu, 12 Jan 2012 12:28:40 -0500, Austin Clements  
> wrote:
> > Quoth Pieter Praet on Jan 12 at  6:07 pm:
> > > On Tue, 22 Nov 2011 22:40:21 -0500, Austin Clements  
> > > wrote:
> > > > Quoth Jameson Graef Rollins on Nov 20 at 12:10 pm:
> > > > > The open question seems to be how we handle the content encoding
> > > > > parameters.  My argument is that those should either be used by 
> > > > > notmuch
> > > > > to properly encode the content for the consumer.  If that's not
> > > > > possible, then just those parameters needed by the consumer to decode
> > > > > the content should be output.
> > > > 
> > > > If notmuch is going to include part content in the JSON output (which
> > > > perhaps it shouldn't, as per recent IRC discussions), then it must
> > > > handle content encodings because JSON must be Unicode and therefore
> > > > the content strings in the JSON must be Unicode.
> > > 
> > > Having missed the IRC discussions: what is the rationale for not
> > > including (specific types of?) part content in the JSON output ?
> > > Eg. how about inline attached text/x-patch ?
> > 
> > Technically the IRC discussion was about not including *any* part
> > content in the JSON output, and always using show --format=raw or
> > similar to retrieve desired parts.  Currently, notmuch includes part
> > content in the JSON only for text/*, *except* when it's text/html.  I
> > assume non-text parts are omitted because binary data is hard to
> > represent in JSON and text/html is omitted because some people don't
> > need it.  However, this leads to some peculiar asymmetry in the Emacs
> > code where sometimes it pulls part content out of the JSON and
> > sometimes it retrieves it using show --format=raw.  This in turn leads
> > to asymmetry in content encoding handling, since notmuch handles
> > content encoding for parts included in the JSON (and there's no good
> > way around that since JSON is Unicode), but not for parts retrieved as
> > raw.
> > 
> > The idea discussed on IRC was to remove all part content from the JSON
> > output and to always use show to retrieve it, possibly beefing up
> > show's support for content decoding (and possibly introducing a way to
> > retrieve multiple raw parts at once to avoid re-parsing).  This would
> > get the JSON format out of the business of guessing what consumers
> > need, simplify the Emacs code, and normalize content encoding
> > handling.
> 
> Full ACK.
> 
> One concern though (IIUC): Due to the prevalence of retarded MUA's, not
> outputting 'text/plain' and/or 'text/html' parts is unfortunately all
> too often equivalent to not outputting anything at all, so wouldn't we,
> in essence, be reducing `show --format=json' to an ever-so-slightly
> augmented `search --format=json' ?

I'm not sure I fully understand what you're saying, but there are
several levels of structure here:

1. Threads (query results)
2. Thread structure
3. Message structure (MIME)
4. Part content

Currently, search returns 1; show --format=json returns 2, 3, and
sometimes 4 (but sometimes not); and show --format=raw returns 4.
Notably, 1 does not require opening message files (neither does 2),
which I consider an important distinction between search and show.

Some of the discussion has been about putting 4 squarely in the realm
of show --format=raw.  One counterargument (which has grown on me
since this discussion) is that the part content included in
--format=json can be thought of as pre-fetching content that clients
are likely to need in order to avoid re-parsing the message in most
circumstances.  I believe this is not the *intent* of the current
code, though without a specification of the JSON format it's hard to
tell.

Other discussion (more interesting, in my mind) has been about
separating retrieving thread structure, 2, from retrieving message
structure, 3.  To me, splitting these feels much more natural than
what we do now, which seems to be inflexibly bound to the specific way
the Emacs show mode currently works.  The thread structure is readily
available from the database, so I think separating these would open up
some new UI opportunities, particularly easy and fast thread outlining
and navigation.  I believe it would also simplify the code and address
some irritating asymmetries in the way notmuch show handles the --part
argument.


[PATCH v3 2/2] search: Support automatic tag exclusions

2012-01-14 Thread Austin Clements
This adds a "search" section to the config file and an
"auto_tag_exclusions" setting in that section.  The search and count
commands pass tag tags from the configuration to the library.
---
 notmuch-client.h |8 
 notmuch-config.c |   42 ++
 notmuch-count.c  |8 
 notmuch-search.c |8 
 test/search  |   18 ++
 5 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 517c010..62ede28 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -235,6 +235,14 @@ void
 notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config,
  notmuch_bool_t synchronize_flags);

+const char **
+notmuch_config_get_auto_exclude_tags (notmuch_config_t *config, size_t 
*length);
+
+void
+notmuch_config_set_auto_exclude_tags (notmuch_config_t *config,
+ const char *list[],
+ size_t length);
+
 int
 notmuch_run_hook (const char *db_path, const char *hook);

diff --git a/notmuch-config.c b/notmuch-config.c
index d697138..3d4d5b9 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -84,6 +84,15 @@ static const char maildir_config_comment[] =
 "\tand update tags, while the \"notmuch tag\" and \"notmuch restore\"\n"
 "\tcommands will notice tag changes and update flags in filenames\n";

+static const char search_config_comment[] =
+" Search configuration\n"
+"\n"
+" The following option is supported here:\n"
+"\n"
+"\tauto_exclude_tags  A ;-separated list of tags that will be\n"
+"\t excluded from search results by default.  Using an excluded tag\n"
+"\t in a query will override that exclusion.\n";
+
 struct _notmuch_config {
 char *filename;
 GKeyFile *key_file;
@@ -96,6 +105,8 @@ struct _notmuch_config {
 const char **new_tags;
 size_t new_tags_length;
 notmuch_bool_t maildir_synchronize_flags;
+const char **auto_exclude_tags;
+size_t auto_exclude_tags_length;
 };

 static int
@@ -221,6 +232,7 @@ notmuch_config_open (void *ctx,
 int file_had_new_group;
 int file_had_user_group;
 int file_had_maildir_group;
+int file_had_search_group;

 if (is_new_ret)
*is_new_ret = 0;
@@ -252,6 +264,8 @@ notmuch_config_open (void *ctx,
 config->new_tags = NULL;
 config->new_tags_length = 0;
 config->maildir_synchronize_flags = TRUE;
+config->auto_exclude_tags = NULL;
+config->auto_exclude_tags_length = 0;

 if (! g_key_file_load_from_file (config->key_file,
 config->filename,
@@ -295,6 +309,7 @@ notmuch_config_open (void *ctx,
 file_had_new_group = g_key_file_has_group (config->key_file, "new");
 file_had_user_group = g_key_file_has_group (config->key_file, "user");
 file_had_maildir_group = g_key_file_has_group (config->key_file, 
"maildir");
+file_had_search_group = g_key_file_has_group (config->key_file, "search");


 if (notmuch_config_get_database_path (config) == NULL) {
@@ -345,6 +360,11 @@ notmuch_config_open (void *ctx,
notmuch_config_set_new_tags (config, tags, 2);
 }

+if (notmuch_config_get_auto_exclude_tags (config, ) == NULL) {
+   const char *tags[] = { "deleted", "spam" };
+   notmuch_config_set_auto_exclude_tags (config, tags, 2);
+}
+
 error = NULL;
 config->maildir_synchronize_flags =
g_key_file_get_boolean (config->key_file,
@@ -387,6 +407,11 @@ notmuch_config_open (void *ctx,
maildir_config_comment, NULL);
 }

+if (! file_had_search_group) {
+   g_key_file_set_comment (config->key_file, "search", NULL,
+   search_config_comment, NULL);
+}
+
 if (is_new_ret)
*is_new_ret = is_new;

@@ -597,6 +622,23 @@ notmuch_config_set_new_tags (notmuch_config_t *config,
 &(config->new_tags));
 }

+const char **
+notmuch_config_get_auto_exclude_tags (notmuch_config_t *config, size_t *length)
+{
+return _config_get_list (config, "search", "auto_exclude_tags",
+&(config->auto_exclude_tags),
+&(config->auto_exclude_tags_length), length);
+}
+
+void
+notmuch_config_set_auto_exclude_tags (notmuch_config_t *config,
+ const char *list[],
+ size_t length)
+{
+_config_set_list (config, "search", "auto_exclude_tags", list, length,
+ &(config->auto_exclude_tags));
+}
+
 /* Given a configuration item of the form . return the
  * component group and key. If any error occurs, print a message on
  * stderr and return 1. Otherwise, return 0.
diff --git a/notmuch-count.c b/notmuch-count.c
index 0982f99..f77861e 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -35,6 +35,9 @@ notmuch_count_command (void *ctx, int argc, 

[PATCH v3 1/2] lib: Add support for automatically excluding tags from queries

2012-01-14 Thread Austin Clements
This is useful for tags like "deleted" and "spam" that people
generally want to exclude from query results.  These exclusions will
be overridden if a tag is explicitly mentioned in a query.
---
 lib/notmuch-private.h |2 +-
 lib/notmuch.h |6 ++
 lib/query.cc  |   35 +++
 3 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 60a932f..7bf153e 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -458,7 +458,7 @@ typedef struct _notmuch_string_node {
 struct _notmuch_string_node *next;
 } notmuch_string_node_t;

-typedef struct _notmuch_string_list {
+typedef struct visible _notmuch_string_list {
 int length;
 notmuch_string_node_t *head;
 notmuch_string_node_t **tail;
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 9f23a10..7929fe7 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -457,6 +457,12 @@ notmuch_query_set_sort (notmuch_query_t *query, 
notmuch_sort_t sort);
 notmuch_sort_t
 notmuch_query_get_sort (notmuch_query_t *query);

+/* Add a tag that will be excluded from the query results by default.
+ * This exclusion will be overridden if this tag appears explicitly in
+ * the query. */
+void
+notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag);
+
 /* Execute a query for threads, returning a notmuch_threads_t object
  * which can be used to iterate over the results. The returned threads
  * object is owned by the query and as such, will only be valid until
diff --git a/lib/query.cc b/lib/query.cc
index b6c0f12..0b36602 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -27,6 +27,7 @@ struct _notmuch_query {
 notmuch_database_t *notmuch;
 const char *query_string;
 notmuch_sort_t sort;
+notmuch_string_list_t *exclude_terms;
 };

 typedef struct _notmuch_mset_messages {
@@ -76,6 +77,8 @@ notmuch_query_create (notmuch_database_t *notmuch,

 query->sort = NOTMUCH_SORT_NEWEST_FIRST;

+query->exclude_terms = _notmuch_string_list_create (query);
+
 return query;
 }

@@ -97,6 +100,13 @@ notmuch_query_get_sort (notmuch_query_t *query)
 return query->sort;
 }

+void
+notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
+{
+char *term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), tag);
+_notmuch_string_list_append (query->exclude_terms, term);
+}
+
 /* We end up having to call the destructors explicitly because we had
  * to use "placement new" in order to initialize C++ objects within a
  * block that we allocated with talloc. So C++ is making talloc
@@ -112,6 +122,27 @@ _notmuch_messages_destructor (notmuch_mset_messages_t 
*messages)
 return 0;
 }

+/* Return a query that does not match messages with the excluded tags
+ * registered with the query.  Any tags that explicitly appear in
+ * xquery will not be excluded. */
+static Xapian::Query
+_notmuch_exclude_tags (notmuch_query_t *query, Xapian::Query xquery)
+{
+for (notmuch_string_node_t *term = query->exclude_terms->head; term;
+term = term->next) {
+   Xapian::TermIterator it = xquery.get_terms_begin ();
+   Xapian::TermIterator end = xquery.get_terms_end ();
+   for (; it != end; it++) {
+   if ((*it).compare (term->string) == 0)
+   break;
+   }
+   if (it == end)
+   xquery = Xapian::Query (Xapian::Query::OP_AND_NOT,
+   xquery, Xapian::Query (term->string));
+}
+return xquery;
+}
+
 notmuch_messages_t *
 notmuch_query_search_messages (notmuch_query_t *query)
 {
@@ -157,6 +188,8 @@ notmuch_query_search_messages (notmuch_query_t *query)
 mail_query, string_query);
}

+   final_query = _notmuch_exclude_tags (query, final_query);
+
enquire.set_weighting_scheme (Xapian::BoolWeight());

switch (query->sort) {
@@ -436,6 +469,8 @@ notmuch_query_count_messages (notmuch_query_t *query)
 mail_query, string_query);
}

+   final_query = _notmuch_exclude_tags (query, final_query);
+
enquire.set_weighting_scheme(Xapian::BoolWeight());
enquire.set_docid_order(Xapian::Enquire::ASCENDING);

-- 
1.7.7.3



[PATCH v3 0/2] Automatic tag-based exclusion

2012-01-14 Thread Austin Clements
This fixes the symbol visibility warning Jamie pointed out.



[PATCH v2 3/3] search: Support automatic tag exclusions

2012-01-14 Thread Austin Clements
Quoth Jameson Graef Rollins on Jan 14 at  3:40 pm:
> This patch looks fine.  Philosophical UI discussion to follow:
> 
> On Fri, 13 Jan 2012 18:07:04 -0500, Austin Clements  
> wrote:
> > +if (notmuch_config_get_auto_exclude_tags (config, ) == NULL) {
> > +   const char *tags[] = { "deleted", "spam" };
> > +   notmuch_config_set_auto_exclude_tags (config, tags, 2);
> > +}
> 
> This creates the config section with the exclude list pre-set to
> "deleted;spam".  I personally have no problem with this, since I was
> going to be setting exactly that anyway.  However, assuming we decide to
> have this be the default in the CLI, should we therefore add support for
> it in the emacs UI?  I've been going back and forth on this (as readers
> are well aware), and have most recently rejected the idea that we should
> add delete support to the emacs UI.  However, if we are excluding
> "deleted" tags by default, then I'm going to go back and say that we
> should include the keybindings to "delete" messages.  Comments?

It's not that Emacs doesn't support the deleted tag.  You can always
+deleted, and this even seems like a pretty natural thing to do.
To me, the question is whether there should be a shortcut to do this.
I'm probably not one to answer this, since I don't plan to use the
deleted tag and hence using this binding would only ever be an
accident (though I will use the spam tag and I don't think I need a
binding for that; perhaps I would feel differently if my spam filters
were less effective).

> If people think we should exclude "deleted;spam" by default, and agree
> that we should also add delete support in the emacs UI, I'll go ahead
> and rework my keybinding patches.
> 
> jamie.


[PATCH v2 2/3] lib: Add support for automatically excluding tags from queries

2012-01-14 Thread Austin Clements
Quoth Jameson Graef Rollins on Jan 14 at  3:38 pm:
> It looks like something in this patch is causing the following build
> warning:
> 
> CXX -O2 lib/query.o
> lib/query.cc:26:8: warning: ?_notmuch_query? declared with greater visibility 
> than the type of its field
> ?_notmuch_query::exclude_terms? [-Wattributes]
> 
> However, I can't quite figure out what's causing it.

The problem is that notmuch_query_t is a "visible" symbol because the
type is declared (though not defined) in lib/notmuch.h.  The actual
definition is tucked away in lib/query.cc, but GCC doesn't seem to
care.  I added a field of type notmuch_string_list_t to the struct's
definition, but notmuch_string_list_t is declared between the "hidden"
pragmas in lib/notmuch-private.h because the type is private to the
library.  This field with a hidden type in a visible type is what GCC
is complaining about.

I'm rather confused by the whole type visibility thing since type
symbols aren't linkable anyway.  However, while puzzling over how you
could possibly use hidden types if they can only be used in other
hidden types, I discovered that Carl had solved this exact problem
last May in d5523ead by adding a visibility("default") attribute to
the offending hidden type.

> > +void
> > +notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
> > +{
> > +char *term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), 
> > tag);
> > +_notmuch_string_list_append (query->exclude_terms, term);
> > +}
> 
> This is really not an issue with this patch at all, and it should NOT
> prevent it from being applied, but this came up briefly on IRC and I'm
> curious, so I'll ask about it here.
> 
> Are terms ALWAYS lower cased?  If not, it seems to me it's possible to
> have an indexed term 'Kspam' that would get confused with the term
> 'spam' prefixed with the keyword prefix 'K' (which we use for tags).
> Maybe this degeneracy is broken by the query parser somehow (or maybe by
> the fact that tags are boolean terms?), but I wonder if it's not safer
> to use the built-in xapian prefix separator ':', ie:
> 
>   ... talloc_asprintf (query, "%s:%s", _find_prefix ("tag"), tag);
> 
> I guess fixing that globally would require a database rebuild...

We discussed this on IRC, but to summarize for the list, the tag
prefix is a single character, so Xapian's ':' rule doesn't apply.
There are several places where we *do* get this wrong and use a
multi-character term prefix with a term that may start with a capital
letter but they're all terms you can't search anyway and, unless I'm
mistaken, we're completely consistent about where we violate or do not
violate the ':' rule.

> Ok, that's totally just an aside, and should not be a blocker for this
> patch.
> 
> jamie.


[PATCH v5 1/5] cli: slightly refactor "notmuch reply" address scanning functions

2012-01-14 Thread Jani Nikula

For those not on IRC:

On Sat, 14 Jan 2012 11:31:16 -0400, David Bremner  wrote:
> This series definitely needs a NEWS item. 

id:"1326559168-29178-1-git-send-email-jani at nikula.org"

> Perhaps some kind soul could add a wiki entry explaining to people how
> to swap the bindings, just in case there people who don't like the
> "one-true-reply-bindings" (cough).

http://notmuchmail.org/emacstips/#index7h2


BR,
Jani.


[PATCH] Set fill column to 70 in .dir-locals.el and refactor other settings.

2012-01-14 Thread Xavier Maillard
On Fri, 13 Jan 2012 17:54:51 -0500, Austin Clements  wrote:
> Quoth Xavier Maillard on Jan 13 at 11:42 pm:
> > 
> > This controls where comments and other text wraps.  70 is the default
> > value, so this simply returns it to the default for people who have
> > overridden it.  Most notmuch code already adheres to this.
> > ---
> > SO here is the patch (still sorry if I did it wrongly and very badly).
> 
> You ammended it right, but actually the configuration is intentionally
> repeated for the various modes.

That's ok.

Maybe I will find an easier way to build and to send patches

/Xavier


[PATCH] NEWS: add news items for reply to sender

2012-01-14 Thread Jani Nikula
---
 NEWS |   23 +++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index bf21e64..1161c22 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,26 @@
+Notmuch 0.12 (2012-xx-xx)
+=
+
+Command-Line Interface
+--
+
+Reply to sender
+
+  "notmuch reply" has gained the ability to create a reply template
+  for replying just to the sender of the message, in addition to reply
+  to all. The feature is available through the new command line option
+  --reply-to=(all|sender).
+
+Emacs Interface
+---
+
+Reply to sender
+
+  The Emacs interface has, with the new CLI support, gained the
+  ability to reply to sender in addition to reply to all. In both show
+  and search modes, 'r' has been bound to reply to sender, replacing
+  reply to all, which now has key binding 'R'.
+
 Notmuch 0.11 (2012-01-13)
 =

-- 
1.7.5.4



[PATCH 2/2] cli: pick the user's address in a group list as from address

2012-01-14 Thread Austin Clements
Quoth Jani Nikula on Jan 14 at 11:49 pm:
> Messages received to a group list were not replied to using the from
> address in the list. Fix it.
> 
> Signed-off-by: Jani Nikula 

Both LGTM.


[PATCH v5 5/5] test: add tests for "notmuch reply" --reply-to=sender

2012-01-14 Thread Jani Nikula
From: Mark Walters 

---
 test/notmuch-test|1 +
 test/reply-to-sender |  209 ++
 2 files changed, 210 insertions(+), 0 deletions(-)
 create mode 100755 test/reply-to-sender

diff --git a/test/notmuch-test b/test/notmuch-test
index e40ef86..6a99ae3 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -33,6 +33,7 @@ TESTS="
   thread-naming
   raw
   reply
+  reply-to-sender
   dump-restore
   uuencode
   thread-order
diff --git a/test/reply-to-sender b/test/reply-to-sender
new file mode 100755
index 000..c7d15bb
--- /dev/null
+++ b/test/reply-to-sender
@@ -0,0 +1,209 @@
+#!/usr/bin/env bash
+test_description="\"notmuch reply --reply-to=sender\" in several variations"
+. ./test-lib.sh
+
+test_begin_subtest "Basic reply-to-sender"
+add_message '[from]="Sender "' \
+ [to]=test_suite at notmuchmail.org \
+ [subject]=notmuch-reply-test \
+'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
+'[body]="basic reply-to-sender test"'
+
+output=$(notmuch reply --reply-to=sender id:${gen_msg_id})
+test_expect_equal "$output" "From: Notmuch Test Suite 
+Subject: Re: notmuch-reply-test
+To: Sender 
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -, Sender  wrote:
+> basic reply-to-sender test"
+
+test_begin_subtest "From Us, Basic reply to message"
+add_message '[from]="Notmuch Test Suite "' \
+'[to]="Recipient "' \
+ [subject]=notmuch-reply-test \
+'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
+'[body]="basic reply-to-from-us test"'
+
+output=$(notmuch reply --reply-to=sender id:${gen_msg_id})
+test_expect_equal "$output" "From: Notmuch Test Suite 
+Subject: Re: notmuch-reply-test
+To: Recipient 
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -, Notmuch Test Suite  wrote:
+> basic reply-to-from-us test"
+
+test_begin_subtest "Multiple recipients"
+add_message '[from]="Sender "' \
+'[to]="test_suite at notmuchmail.org, Someone Else "' \
+ [subject]=notmuch-reply-test \
+'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
+'[body]="Multiple recipients"'
+
+output=$(notmuch reply  --reply-to=sender  id:${gen_msg_id})
+test_expect_equal "$output" "From: Notmuch Test Suite 
+Subject: Re: notmuch-reply-test
+To: Sender 
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -, Sender  wrote:
+> Multiple recipients"
+
+test_begin_subtest "From Us, Multiple TO recipients"
+add_message '[from]="Notmuch Test Suite "' \
+'[to]="Recipient , Someone Else "' \
+ [subject]=notmuch-reply-test \
+'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
+'[body]="From Us, Multiple TO recipients"'
+
+output=$(notmuch reply  --reply-to=sender  id:${gen_msg_id})
+test_expect_equal "$output" "From: Notmuch Test Suite 
+Subject: Re: notmuch-reply-test
+To: Recipient , Someone Else 
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -, Notmuch Test Suite  wrote:
+> From Us, Multiple TO recipients"
+
+test_begin_subtest "Reply with CC"
+add_message '[from]="Sender "' \
+ [to]=test_suite at notmuchmail.org \
+'[cc]="Other Parties "' \
+ [subject]=notmuch-reply-test \
+'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
+'[body]="reply with CC"'
+
+output=$(notmuch reply  --reply-to=sender id:${gen_msg_id})
+test_expect_equal "$output" "From: Notmuch Test Suite 
+Subject: Re: notmuch-reply-test
+To: Sender 
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -, Sender  wrote:
+> reply with CC"
+
+test_begin_subtest "From Us, Reply with CC"
+add_message '[from]="Notmuch Test Suite "' \
+'[to]="Recipient "' \
+'[cc]="Other Parties "' \
+ [subject]=notmuch-reply-test \
+'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
+'[body]="reply with CC"'
+
+output=$(notmuch reply  --reply-to=sender id:${gen_msg_id})
+test_expect_equal "$output" "From: Notmuch Test Suite 
+Subject: Re: notmuch-reply-test
+To: Recipient 
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -, Notmuch Test Suite  wrote:
+> reply with CC"
+
+test_begin_subtest "From Us, Reply no TO but with CC"
+add_message '[from]="Notmuch Test Suite "' \
+'[cc]="Other Parties "' \
+ [subject]=notmuch-reply-test \
+'[date]="Tue, 05 Jan 2010 15:43:56 -"' \
+'[body]="reply with CC"'
+
+output=$(notmuch reply  --reply-to=sender id:${gen_msg_id})
+test_expect_equal "$output" "From: Notmuch Test Suite 
+Subject: Re: notmuch-reply-test
+Cc: Other Parties 
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 

[PATCH v5 4/5] emacs: bind 'r' to reply-to-sender and 'R' to reply-to-all

2012-01-14 Thread Jani Nikula
Change the default reply key bindings, making 'r' reply-to-sender and 'R'
reply-to-all.

Signed-off-by: Jani Nikula 

---

There were mixed feelings about this. This as a separate patch so it's easy
to drop if needed.
---
 emacs/notmuch-show.el |4 ++--
 emacs/notmuch.el  |4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 9031b82..03c1f6b 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -933,8 +933,8 @@ thread id.  If a prefix is given, crypto processing is 
toggled."
(define-key map "s" 'notmuch-search)
(define-key map "m" 'notmuch-mua-new-mail)
(define-key map "f" 'notmuch-show-forward-message)
-   (define-key map "r" 'notmuch-show-reply)
-   (define-key map "R" 'notmuch-show-reply-sender)
+   (define-key map "r" 'notmuch-show-reply-sender)
+   (define-key map "R" 'notmuch-show-reply)
(define-key map "|" 'notmuch-show-pipe-message)
(define-key map "w" 'notmuch-show-save-attachments)
(define-key map "V" 'notmuch-show-view-raw-message)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 9ac2888..d952c41 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -213,8 +213,8 @@ For a mouse binding, return nil."
 (define-key map ">" 'notmuch-search-last-thread)
 (define-key map "p" 'notmuch-search-previous-thread)
 (define-key map "n" 'notmuch-search-next-thread)
-(define-key map "r" 'notmuch-search-reply-to-thread)
-(define-key map "R" 'notmuch-search-reply-to-thread-sender)
+(define-key map "r" 'notmuch-search-reply-to-thread-sender)
+(define-key map "R" 'notmuch-search-reply-to-thread)
 (define-key map "m" 'notmuch-mua-new-mail)
 (define-key map "s" 'notmuch-search)
 (define-key map "o" 'notmuch-search-toggle-order)
-- 
1.7.5.4



[PATCH v5 3/5] emacs: add support for replying just to the sender

2012-01-14 Thread Jani Nikula
Provide reply to sender counterparts to the search and show reply
functions. Add key binding 'R' to reply to sender, while keeping 'r' as
reply to all, both in search and show views.

Signed-off-by: Jani Nikula 
---
 emacs/notmuch-mua.el  |9 ++---
 emacs/notmuch-show.el |   10 --
 emacs/notmuch.el  |9 -
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 32e2e30..d8ab822 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -71,12 +71,15 @@ list."
(push header message-hidden-headers)))
notmuch-mua-hidden-headers))

-(defun notmuch-mua-reply (query-string  sender)
+(defun notmuch-mua-reply (query-string  sender reply-all)
   (let (headers
body
(args '("reply")))
 (if notmuch-show-process-crypto
(setq args (append args '("--decrypt"
+(if reply-all
+   (setq args (append args '("--reply-to=all")))
+  (setq args (append args '("--reply-to=sender"
 (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
@@ -218,13 +221,13 @@ the From: address first."
(notmuch-mua-forward-message))
 (notmuch-mua-forward-message)))

-(defun notmuch-mua-new-reply (query-string  prompt-for-sender)
+(defun notmuch-mua-new-reply (query-string  prompt-for-sender 
reply-all)
   "Invoke the notmuch reply window."
   (interactive "P")
   (let ((sender
 (when prompt-for-sender
   (notmuch-mua-prompt-for-sender
-(notmuch-mua-reply query-string sender)))
+(notmuch-mua-reply query-string sender reply-all)))

 (defun notmuch-mua-send-and-exit ( arg)
   (interactive "P")
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 034db87..9031b82 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -934,6 +934,7 @@ thread id.  If a prefix is given, crypto processing is 
toggled."
(define-key map "m" 'notmuch-mua-new-mail)
(define-key map "f" 'notmuch-show-forward-message)
(define-key map "r" 'notmuch-show-reply)
+   (define-key map "R" 'notmuch-show-reply-sender)
(define-key map "|" 'notmuch-show-pipe-message)
(define-key map "w" 'notmuch-show-save-attachments)
(define-key map "V" 'notmuch-show-view-raw-message)
@@ -1238,9 +1239,14 @@ any effects from previous calls to
   (notmuch-show-previous-message)

 (defun notmuch-show-reply ( prompt-for-sender)
-  "Reply to the current message."
+  "Reply to the sender and all recipients of the current message."
   (interactive "P")
-  (notmuch-mua-new-reply (notmuch-show-get-message-id) prompt-for-sender))
+  (notmuch-mua-new-reply (notmuch-show-get-message-id) prompt-for-sender t))
+
+(defun notmuch-show-reply-sender ( prompt-for-sender)
+  "Reply to the sender of the current message."
+  (interactive "P")
+  (notmuch-mua-new-reply (notmuch-show-get-message-id) prompt-for-sender nil))

 (defun notmuch-show-forward-message ( prompt-for-sender)
   "Forward the current message."
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 1e61775..9ac2888 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -214,6 +214,7 @@ For a mouse binding, return nil."
 (define-key map "p" 'notmuch-search-previous-thread)
 (define-key map "n" 'notmuch-search-next-thread)
 (define-key map "r" 'notmuch-search-reply-to-thread)
+(define-key map "R" 'notmuch-search-reply-to-thread-sender)
 (define-key map "m" 'notmuch-mua-new-mail)
 (define-key map "s" 'notmuch-search)
 (define-key map "o" 'notmuch-search-toggle-order)
@@ -448,10 +449,16 @@ Complete list of currently available key bindings:
   (message "End of search results."

 (defun notmuch-search-reply-to-thread ( prompt-for-sender)
+  "Begin composing a reply-all to the entire current thread in a new buffer."
+  (interactive "P")
+  (let ((message-id (notmuch-search-find-thread-id)))
+(notmuch-mua-new-reply message-id prompt-for-sender t)))
+
+(defun notmuch-search-reply-to-thread-sender ( prompt-for-sender)
   "Begin composing a reply to the entire current thread in a new buffer."
   (interactive "P")
   (let ((message-id (notmuch-search-find-thread-id)))
-(notmuch-mua-new-reply message-id prompt-for-sender)))
+(notmuch-mua-new-reply message-id prompt-for-sender nil)))

 (defun notmuch-call-notmuch-process ( args)
   "Synchronously invoke \"notmuch\" with the given list of arguments.
-- 
1.7.5.4



[PATCH v5 2/5] cli: add support for replying just to the sender in "notmuch reply"

2012-01-14 Thread Jani Nikula
Add new option --reply-to=(all|sender) to "notmuch reply" to select whether
to reply to all (sender and all recipients), or just sender. Reply to all
remains the default.

Credits to Mark Walters  for his similar earlier
work where I picked up the basic idea of handling reply-to-sender in
add_recipients_from_message(). All bugs are mine, though.

Signed-off-by: Jani Nikula 

---

Settled on --reply-to=(all|sender) per Carl's earlier suggestion
(id:87pqn5cg4g.fsf at yoom.home.cworth.org) and David's approval on IRC.
---
 man/man1/notmuch-reply.1 |   28 ++
 notmuch-reply.c  |   57 -
 2 files changed, 68 insertions(+), 17 deletions(-)

diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index db464d8..5160ece 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -14,11 +14,13 @@ Constructs a reply template for a set of messages.
 To make replying to email easier,
 .B notmuch reply
 takes an existing set of messages and constructs a suitable mail
-template. The Reply-to header (if any, otherwise From:) is used for
-the To: address. Vales from the To: and Cc: headers are copied, but
-not including any of the current user's email addresses (as configured
-in primary_mail or other_email in the .notmuch\-config file) in the
-recipient list
+template. The Reply-to: header (if any, otherwise From:) is used for
+the To: address. Unless
+.BR \-\-reply-to=sender
+is specified, values from the To: and Cc: headers are copied, but not
+including any of the current user's email addresses (as configured in
+primary_mail or other_email in the .notmuch\-config file) in the
+recipient list.

 It also builds a suitable new subject, including Re: at the front (if
 not already present), and adding the message IDs of the messages being
@@ -45,6 +47,22 @@ Includes subject and quoted message body.
 Only produces In\-Reply\-To, References, To, Cc, and Bcc headers.
 .RE
 .RE
+.RS
+.TP 4
+.BR \-\-reply\-to= ( all | sender )
+.RS
+.TP 4
+.BR all " (default)"
+Replies to all addresses.
+.TP 4
+.BR sender
+Replies only to the sender. If replying to user's own message
+(Reply-to: or From: header is one of the user's configured email
+addresses), try To:, Cc:, and Bcc: headers in this order, and copy
+values from the first that contains something other than only the
+user's addresses.
+.RE
+.RE

 See \fBnotmuch-search-terms\fR(7)
 for details of the supported syntax for .
diff --git a/notmuch-reply.c b/notmuch-reply.c
index a8d6a94..da3acce 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -291,15 +291,23 @@ reply_to_header_is_redundant (notmuch_message_t *message)
 return 0;
 }

-/* Augments the recipients of reply from the headers of message.
+/* Augment the recipients of 'reply' from the "Reply-to:", "From:",
+ * "To:", "Cc:", and "Bcc:" headers of 'message'.
  *
- * If any of the user's addresses were found in these headers, the first
- * of these returned, otherwise NULL is returned.
+ * If 'reply_all' is true, use sender and all recipients, otherwise
+ * scan the headers for the first that contains something other than
+ * the user's addresses and add the recipients from this header
+ * (typically this would be reply-to-sender, but also handles reply to
+ * user's own message in a sensible way).
+ *
+ * If any of the user's addresses were found in these headers, the
+ * first of these returned, otherwise NULL is returned.
  */
 static const char *
 add_recipients_from_message (GMimeMessage *reply,
 notmuch_config_t *config,
-notmuch_message_t *message)
+notmuch_message_t *message,
+notmuch_bool_t reply_all)
 {
 struct {
const char *header;
@@ -313,6 +321,7 @@ add_recipients_from_message (GMimeMessage *reply,
 };
 const char *from_addr = NULL;
 unsigned int i;
+unsigned int n = 0;

 /* Some mailing lists munge the Reply-To header despite it being A Bad
  * Thing, see http://www.unicom.com/pw/reply-to-harmful.html
@@ -339,8 +348,24 @@ add_recipients_from_message (GMimeMessage *reply,
recipients = notmuch_message_get_header (message,
 reply_to_map[i].fallback);

-   scan_address_string (recipients, config, reply,
-reply_to_map[i].recipient_type, _addr);
+   n += scan_address_string (recipients, config, reply,
+ reply_to_map[i].recipient_type, _addr);
+
+   if (!reply_all && n) {
+   /* Stop adding new recipients in reply-to-sender mode if
+* we have added some recipient(s) above.
+*
+* This also handles the case of user replying to his own
+* message, where reply-to/from is not a recipient. In
+* this case there may be more than one recipient even if
+* not replying to 

[PATCH v5 1/5] cli: slightly refactor "notmuch reply" address scanning functions

2012-01-14 Thread Jani Nikula
Slightly refactor "notmuch reply" recipient and user from address scanning
functions in preparation for reply-to-sender feature.

Add support for not adding recipients at all (just scan for user from
address), and returning the number of recipients added.

No externally visible functional changes.

Signed-off-by: Jani Nikula 
---
 notmuch-reply.c |   76 +--
 1 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 000f6da..a8d6a94 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -168,22 +168,29 @@ address_is_users (const char *address, notmuch_config_t 
*config)
 return 0;
 }

-/* For each address in 'list' that is not configured as one of the
- * user's addresses in 'config', add that address to 'message' as an
- * address of 'type'.
+/* Scan addresses in 'list'.
  *
- * The first address encountered that *is* the user's address will be
- * returned, (otherwise NULL is returned).
+ * If 'message' is non-NULL, then for each address in 'list' that is
+ * not configured as one of the user's addresses in 'config', add that
+ * address to 'message' as an address of 'type'.
+ *
+ * If 'user_from' is non-NULL and *user_from is NULL, *user_from will
+ * be set to the first address encountered in 'list' that is the
+ * user's address.
+ *
+ * Return the number of addresses added to 'message'. (If 'message' is
+ * NULL, the function returns 0 by definition.)
  */
-static const char *
-add_recipients_for_address_list (GMimeMessage *message,
-notmuch_config_t *config,
-GMimeRecipientType type,
-InternetAddressList *list)
+static unsigned int
+scan_address_list (InternetAddressList *list,
+  notmuch_config_t *config,
+  GMimeMessage *message,
+  GMimeRecipientType type,
+  const char **user_from)
 {
 InternetAddress *address;
 int i;
-const char *ret = NULL;
+unsigned int n = 0;

 for (i = 0; i < internet_address_list_length (list); i++) {
address = internet_address_list_get_address (list, i);
@@ -196,8 +203,7 @@ add_recipients_for_address_list (GMimeMessage *message,
if (group_list == NULL)
continue;

-   add_recipients_for_address_list (message, config,
-type, group_list);
+   n += scan_address_list (group_list, config, message, type, NULL);
} else {
InternetAddressMailbox *mailbox;
const char *name;
@@ -209,40 +215,41 @@ add_recipients_for_address_list (GMimeMessage *message,
addr = internet_address_mailbox_get_addr (mailbox);

if (address_is_users (addr, config)) {
-   if (ret == NULL)
-   ret = addr;
-   } else {
+   if (user_from && *user_from == NULL)
+   *user_from = addr;
+   } else if (message) {
g_mime_message_add_recipient (message, type, name, addr);
+   n++;
}
}
 }

-return ret;
+return n;
 }

-/* For each address in 'recipients' that is not configured as one of
- * the user's addresses in 'config', add that address to 'message' as
- * an address of 'type'.
+/* Scan addresses in 'recipients'.
  *
- * The first address encountered that *is* the user's address will be
- * returned, (otherwise NULL is returned).
+ * See the documentation of scan_address_list() above. This function
+ * does exactly the same, but converts 'recipients' to an
+ * InternetAddressList first.
  */
-static const char *
-add_recipients_for_string (GMimeMessage *message,
-  notmuch_config_t *config,
-  GMimeRecipientType type,
-  const char *recipients)
+static unsigned int
+scan_address_string (const char *recipients,
+notmuch_config_t *config,
+GMimeMessage *message,
+GMimeRecipientType type,
+const char **user_from)
 {
 InternetAddressList *list;

 if (recipients == NULL)
-   return NULL;
+   return 0;

 list = internet_address_list_parse_string (recipients);
 if (list == NULL)
-   return NULL;
+   return 0;

-return add_recipients_for_address_list (message, config, type, list);
+return scan_address_list (list, config, message, type, user_from);
 }

 /* Does the address in the Reply-To header of 'message' already appear
@@ -324,7 +331,7 @@ add_recipients_from_message (GMimeMessage *reply,
 }

 for (i = 0; i < ARRAY_SIZE (reply_to_map); i++) {
-   const char *addr, *recipients;
+   const char *recipients;

recipients = notmuch_message_get_header (message,
 reply_to_map[i].header);
@@ -332,11 +339,8 @@ 

[PATCH v5 0/5] reply to sender

2012-01-14 Thread Jani Nikula
Hi all, hopefully the final round with only the comment and commit
message fixes to issues in patches 1 and 2 spotted by Austin and
Mark. Thanks again for a thorough review!

BR,
Jani.

Jani Nikula (4):
  cli: slightly refactor "notmuch reply" address scanning functions
  cli: add support for replying just to the sender in "notmuch reply"
  emacs: add support for replying just to the sender
  emacs: bind 'r' to reply-to-sender and 'R' to reply-to-all

Mark Walters (1):
  test: add tests for "notmuch reply" --reply-to=sender

 emacs/notmuch-mua.el |9 ++-
 emacs/notmuch-show.el|   12 ++-
 emacs/notmuch.el |   11 ++-
 man/man1/notmuch-reply.1 |   28 +-
 notmuch-reply.c  |  129 ++--
 test/notmuch-test|1 +
 test/reply-to-sender |  209 ++
 7 files changed, 340 insertions(+), 59 deletions(-)
 create mode 100755 test/reply-to-sender

-- 
1.7.5.4



[PATCH v4 1/5] cli: slightly refactor "notmuch reply" address scanning functions

2012-01-14 Thread Jani Nikula
On Thu, 12 Jan 2012 16:59:05 -0500, Austin Clements  wrote:
> LGTM.  One thing you could fix below (and a few comments), but not
> enough alone to warrant a new version.
> 
> Quoth Jani Nikula on Jan 12 at 11:40 pm:
> > Slightly refactor "notmuch reply" recipient and user from address scanning
> > functions in preparation for reply-to-sender feature.
> > 
> > Add support for not adding messages at all (just scan for user from
> > address), and returning the number of messages added.
> > 
> > No externally visible functional changes.
> > 
> > Signed-off-by: Jani Nikula 
> > ---
> >  notmuch-reply.c |   74 
> > --
> >  1 files changed, 38 insertions(+), 36 deletions(-)
> > 
> > diff --git a/notmuch-reply.c b/notmuch-reply.c
> > index 000f6da..4fae66f 100644
> > --- a/notmuch-reply.c
> > +++ b/notmuch-reply.c
> > @@ -168,22 +168,28 @@ address_is_users (const char *address, 
> > notmuch_config_t *config)
> >  return 0;
> >  }
> >  
> > -/* For each address in 'list' that is not configured as one of the
> > - * user's addresses in 'config', add that address to 'message' as an
> > - * address of 'type'.
> > +/* Scan addresses in 'list'.
> >   *
> > - * The first address encountered that *is* the user's address will be
> > - * returned, (otherwise NULL is returned).
> > + * If 'message' is non-NULL, then for each address in 'list' that is not
> > + * configured as one of the user's addresses in 'config', add that address 
> > to
> > + * 'message' as an address of 'type'.
> > + *
> > + * If 'user_from' is non-NULL and *user_from is NULL, the first address
> > + * encountered in 'list' that *is* the user's address will be set to 
> > *user_from.
> > + *
> > + * Return the number of addresses added to 'message'. (If 'message' is 
> > NULL, the
> > + * function returns 0 by definition.)
> 
> Ah, I like the return value.  Better than adding an umpteenth argument
> like I was suggesting.
> 
> >   */
> > -static const char *
> > -add_recipients_for_address_list (GMimeMessage *message,
> > -notmuch_config_t *config,
> > -GMimeRecipientType type,
> > -InternetAddressList *list)
> > +static unsigned int
> > +scan_address_list (InternetAddressList *list,
> > +  notmuch_config_t *config,
> > +  GMimeMessage *message,
> > +  GMimeRecipientType type,
> > +  const char **user_from)
> >  {
> >  InternetAddress *address;
> >  int i;
> > -const char *ret = NULL;
> > +unsigned int n = 0;
> >  
> >  for (i = 0; i < internet_address_list_length (list); i++) {
> > address = internet_address_list_get_address (list, i);
> > @@ -196,8 +202,7 @@ add_recipients_for_address_list (GMimeMessage *message,
> > if (group_list == NULL)
> > continue;
> >  
> > -   add_recipients_for_address_list (message, config,
> > -type, group_list);
> > +   n += scan_address_list (group_list, config, message, type, NULL);
> 
> Should the NULL above be user_from?  You're being compatible with the
> original code, which is the right thing to do in this patch, but the
> new-found explicitness made me wonder if this is actually a bug.

I didn't even know what a group list was, and if [1] is accurate, I
don't think I've ever seen one either. This does smell like a bug, but
I'm doubtful if anyone would ever notice... Anyway, as you say, it
should be another patch, and independent of this series.

BR,
Jani.

[1] http://www.cs.tut.fi/~jkorpela/rfc/822addr.html

> 
> > } else {
> > InternetAddressMailbox *mailbox;
> > const char *name;
> > @@ -209,40 +214,40 @@ add_recipients_for_address_list (GMimeMessage 
> > *message,
> > addr = internet_address_mailbox_get_addr (mailbox);
> >  
> > if (address_is_users (addr, config)) {
> > -   if (ret == NULL)
> > -   ret = addr;
> > -   } else {
> > +   if (user_from && *user_from == NULL)
> > +   *user_from = addr;
> > +   } else if (message) {
> > g_mime_message_add_recipient (message, type, name, addr);
> > +   n++;
> > }
> > }
> >  }
> >  
> > -return ret;
> > +return n;
> >  }
> >  
> > -/* For each address in 'recipients' that is not configured as one of
> > - * the user's addresses in 'config', add that address to 'message' as
> > - * an address of 'type'.
> > +/* Scan addresses in 'recipients'.
> >   *
> > - * The first address encountered that *is* the user's address will be
> > - * returned, (otherwise NULL is returned).
> > + * See the documentation of scan_address_list() above. This function does
> > + * exactly the same, but converts 'recipients' to an InternetAddressList 
> > first.
> 
> Just bikeshedding, but comments in the notmuch codebase almost
> universally wrap at 72 columns, not 80 (based on 
> egrep '[ 

[PATCH v2 3/3] search: Support automatic tag exclusions

2012-01-14 Thread Jameson Graef Rollins
This patch looks fine.  Philosophical UI discussion to follow:

On Fri, 13 Jan 2012 18:07:04 -0500, Austin Clements  wrote:
> +if (notmuch_config_get_auto_exclude_tags (config, ) == NULL) {
> + const char *tags[] = { "deleted", "spam" };
> + notmuch_config_set_auto_exclude_tags (config, tags, 2);
> +}

This creates the config section with the exclude list pre-set to
"deleted;spam".  I personally have no problem with this, since I was
going to be setting exactly that anyway.  However, assuming we decide to
have this be the default in the CLI, should we therefore add support for
it in the emacs UI?  I've been going back and forth on this (as readers
are well aware), and have most recently rejected the idea that we should
add delete support to the emacs UI.  However, if we are excluding
"deleted" tags by default, then I'm going to go back and say that we
should include the keybindings to "delete" messages.  Comments?

If people think we should exclude "deleted;spam" by default, and agree
that we should also add delete support in the emacs UI, I'll go ahead
and rework my keybinding patches.

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/20120114/e0873110/attachment.pgp>


[PATCH v2 2/3] lib: Add support for automatically excluding tags from queries

2012-01-14 Thread Jameson Graef Rollins
It looks like something in this patch is causing the following build
warning:

CXX -O2 lib/query.o
lib/query.cc:26:8: warning: ?_notmuch_query? declared with greater visibility 
than the type of its field
?_notmuch_query::exclude_terms? [-Wattributes]

However, I can't quite figure out what's causing it.

> +void
> +notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
> +{
> +char *term = talloc_asprintf (query, "%s%s", _find_prefix ("tag"), tag);
> +_notmuch_string_list_append (query->exclude_terms, term);
> +}

This is really not an issue with this patch at all, and it should NOT
prevent it from being applied, but this came up briefly on IRC and I'm
curious, so I'll ask about it here.

Are terms ALWAYS lower cased?  If not, it seems to me it's possible to
have an indexed term 'Kspam' that would get confused with the term
'spam' prefixed with the keyword prefix 'K' (which we use for tags).
Maybe this degeneracy is broken by the query parser somehow (or maybe by
the fact that tags are boolean terms?), but I wonder if it's not safer
to use the built-in xapian prefix separator ':', ie:

  ... talloc_asprintf (query, "%s:%s", _find_prefix ("tag"), tag);

I guess fixing that globally would require a database rebuild...

Ok, that's totally just an aside, and should not be a blocker for this
patch.

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/20120114/acc758bf/attachment.pgp>


[PATCH v2 0/3]

2012-01-14 Thread Jameson Graef Rollins
On Fri, 13 Jan 2012 18:07:01 -0500, Austin Clements  wrote:
> This addresses Jani's comments and improves some of the text and code
> comments.

This is a really nice feature addition, Austin.  And it works like a
charm.  ++1.

A couple of small comments to follow.

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/20120114/b5536f12/attachment.pgp>


[PATCH] emacs: logically group def{custom,face}s

2012-01-14 Thread Austin Clements
Quoth Pieter Praet on Jan 14 at 10:04 am:
> To allow for expansion whilst keeping everything tidy and organized,
> move all defcustom/defface variables to the following subgroups,
> defined in notmuch-lib.el:
> 
> - Hello
> - Search
> - Show
> - Send
> - Crypto
> - Hooks
> - External Commands
> - Appearance
> 
> As an added benefit, defcustom keyword args are now consistently
> in order of appearance @ defcustom's docstring (OCD much?).

Thanks for doing this.  I recently went into customize-group notmuch
and was overwhelmed by the pile of options presented.

> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 0f856bf..f6f48e9 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -28,17 +28,58 @@
>"Notmuch mail reader for Emacs."
>:group 'mail)
>  

Group docstrings aren't generally of the form "Options concerning
..."; they just jump into what they concern.  E.g.,

Convenience : Convenience features for faster editing.

Calendar Hooks : Calendar hooks.

Also, all but one of the tags you give the groups would be
automatically derived by Emacs, so you can remove those.

> +(defgroup notmuch-hello nil
> +  "Options concerning `notmuch-hello-mode'."
> +  :tag "Notmuch Hello"
> +  :group 'notmuch)

Perhaps "Overview of saved searches, tags, etc."

> +
> +(defgroup notmuch-search nil
> +  "Options concerning `notmuch-search-mode'."
> +  :tag "Notmuch Search"
> +  :group 'notmuch)

"Searching and sorting mail"?

> +
> +(defgroup notmuch-show nil
> +  "Options concerning `notmuch-show-mode'."
> +  :tag "Notmuch Show"
> +  :group 'notmuch)

"Showing messages and threads"?

> +
> +(defgroup notmuch-send nil
> +  "Options concerning the sending of messages."
> +  :tag "Notmuch Send"
> +  :group 'notmuch)

"Sending messages from Notmuch"?

We should probably link to the 'message group, perhaps by adding
  :link '(custom-group-link message)
here or maybe to the notmuch group itself.  Unfortunately, I don't
think you can actually add a group to another group after it's been
defined (though I could be wrong).

> +
> +(defgroup notmuch-crypto nil
> +  "Options concerning the processing and fontification of
> +cryptographic MIME parts in `notmuch-show-mode'."
> +  :tag "Notmuch Crypto"
> +  :group 'notmuch)

"Processing and display of cryptographic MIME parts"?  (You also don't
want the docstring to be too long, given how it's displayed.)

> +
> +(defgroup notmuch-hooks nil
> +  "Run custom code on well-defined occasions."
> +  :tag "Notmuch Hooks"
> +  :group 'notmuch)
> +
> +(defgroup notmuch-external nil
> +  "Run more custom code on different well-defined occasions."
> +  :tag "Notmuch External Commands"
> +  :group 'notmuch)

Oof!  It's okay to be a little redundant in the docstring.  Core Emacs
options do it.  "External commands"?

> +
> +(defgroup notmuch-appearance nil
> +  "Options concerning how Notmuch looks."
> +  :tag "Notmuch Appearance"
> +  :group 'notmuch)

"How Notmuch looks"?

I worry that notmuch-appearance is a catch-all that most options
arguably fit in to.  In particular, some notmuch-show options are also
in this group and some aren't and it's not clear to me what the rule
is.

Perhaps this should be notmuch-faces and limited to just faces (and
maybe options that aren't technically faces but that affect face
selection)?  Then the grouping rule would be obvious, like it is for
all of the other groups.


[PATCH] emacs: logically group def{custom,face}s

2012-01-14 Thread Jameson Graef Rollins
I like all of Austin's description suggestions.

jamie.


[PATCH v5 1/5] cli: slightly refactor "notmuch reply" address scanning functions

2012-01-14 Thread David Bremner
On Sat, 14 Jan 2012 16:46:15 +0200, Jani Nikula  wrote:
> Slightly refactor "notmuch reply" recipient and user from address scanning
> functions in preparation for reply-to-sender feature.
> 

Pushed, bindings change and all.

This series definitely needs a NEWS item. 

Perhaps some kind soul could add a wiki entry explaining to people how
to swap the bindings, just in case there people who don't like the
"one-true-reply-bindings" (cough).

d





[PATCH] Fix build warning: "/*" within comment

2012-01-14 Thread David Bremner
On Fri, 13 Jan 2012 22:08:23 -0500, Austin Clements  wrote:
> ---
>  notmuch-show.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Pushed.


[PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."

2012-01-14 Thread Pieter Praet
On Sat, 14 Jan 2012 10:14:57 +0100, Pieter Praet  wrote:
> On Fri, 13 Jan 2012 08:23:55 +, David Edmondson  wrote:
> > On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet  
> > wrote:
> > > Less code, same results, without sacrificing readability.
> > 
> > +1, but why not replace non-branching `if' with `when' as well?
> 
> I was planning to do that when the `unless' patch was accepted,
^^
   submit

> but after reading Xavier and Tomi's replies, I've changed my mind.
   
   I hadn't, though...

> 
> Looking at "subr.el", it's actually more efficient to use `if'
> (implemented in C) instead of `when' (a macro, which essentially
> runs "if conf (progn ...)".
> 
> The amount of non-branching "(if COND (progn ..."  statements is very
> limited though, so if inclined to "fix" them nonetheless, a patch
> follows (relative to the previous one!).
> 
> 
> Peace
> 
> -- 
> Pieter

Typsos FTW!

-- 
Pieter


[PATCH v2] emacs: Cycle through notmuch buffers rather than jumping to the last.

2012-01-14 Thread Pieter Praet
On Tue, 10 Jan 2012 22:15:02 +0200, Jani Nikula  wrote:
> I don't feel qualified to review,

Allow me to disagree: your feeling and reality appear to be disjoint ATM :)


Peace

-- 
Pieter


[PATCH v2] emacs: Cycle through notmuch buffers rather than jumping to the last.

2012-01-14 Thread Pieter Praet
On Wed, 28 Dec 2011 08:29:58 +, David Edmondson  wrote:
> As suggested by j4ni in #notmuch, rename
> `notmuch-jump-to-recent-buffer' as `notmuch-cycle-notmuch-buffers' and
> have it behave accordingly.
> 
> Consider `message-mode' buffers to be of interest.
> ---
>  emacs/notmuch.el |   42 ++
>  1 files changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index c678c93..6a44d49 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -1055,21 +1055,39 @@ current search results AND that are tagged with the 
> given tag."
>(interactive)
>(notmuch-hello))
>  
> +(defun notmuch-interesting-buffer (b)
> +  "Is the current buffer of interest to a notmuch user?"
> +  (with-current-buffer b
> +(memq major-mode '(notmuch-show-mode
> +notmuch-search-mode
> +notmuch-hello-mode
> +message-mode
> +
>  ;;;###autoload
> -(defun notmuch-jump-to-recent-buffer ()
> -  "Jump to the most recent notmuch buffer (search, show or hello).
> +(defun notmuch-cycle-notmuch-buffers ()
> +  "Cycle through any existing notmuch buffers (search, show or hello).
>  
> -If no recent buffer is found, run `notmuch'."
> +If the current buffer is the only notmuch buffer, bury it. If no
> +notmuch buffers exist, run `notmuch'."
>(interactive)
> -  (let ((last
> -  (loop for buffer in (buffer-list)
> -if (with-current-buffer buffer
> - (memq major-mode '(notmuch-show-mode
> -notmuch-search-mode
> -notmuch-hello-mode)))
> -return buffer)))
> -(if last
> - (switch-to-buffer last)
> +
> +  (let (start first)
> +;; If the current buffer is a notmuch buffer, remember it and then
> +;; bury it.
> +(when (notmuch-interesting-buffer (current-buffer))
> +  (setq start (current-buffer))
> +  (bury-buffer))
> +
> +;; Find the first notmuch buffer.
> +(setq first (loop for buffer in (buffer-list)
> +  if (notmuch-interesting-buffer buffer)
> +  return buffer))
> +
> +(if first
> + ;; If the first one we found is any other than the starting
> + ;; buffer, switch to it.
> + (unless (eq first start)
> +   (switch-to-buffer first))
>(notmuch
>  
>  (setq mail-user-agent 'notmuch-user-agent)
> -- 
> 1.7.7.3

Signed-off-by: Pieter Praet   !


Might I ask, to what key(chord) have you bound this ?  Due to its
usefulness, I'm inclined to bind it to [SPC], but on second though,
that might be a bit on the intense side...


Peace

-- 
Pieter


[PATCH] Output unmodified Content-Type header value for JSON format.

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 12:28:40 -0500, Austin Clements  wrote:
> Quoth Pieter Praet on Jan 12 at  6:07 pm:
> > On Tue, 22 Nov 2011 22:40:21 -0500, Austin Clements  
> > wrote:
> > > Quoth Jameson Graef Rollins on Nov 20 at 12:10 pm:
> > > > The open question seems to be how we handle the content encoding
> > > > parameters.  My argument is that those should either be used by notmuch
> > > > to properly encode the content for the consumer.  If that's not
> > > > possible, then just those parameters needed by the consumer to decode
> > > > the content should be output.
> > > 
> > > If notmuch is going to include part content in the JSON output (which
> > > perhaps it shouldn't, as per recent IRC discussions), then it must
> > > handle content encodings because JSON must be Unicode and therefore
> > > the content strings in the JSON must be Unicode.
> > 
> > Having missed the IRC discussions: what is the rationale for not
> > including (specific types of?) part content in the JSON output ?
> > Eg. how about inline attached text/x-patch ?
> 
> Technically the IRC discussion was about not including *any* part
> content in the JSON output, and always using show --format=raw or
> similar to retrieve desired parts.  Currently, notmuch includes part
> content in the JSON only for text/*, *except* when it's text/html.  I
> assume non-text parts are omitted because binary data is hard to
> represent in JSON and text/html is omitted because some people don't
> need it.  However, this leads to some peculiar asymmetry in the Emacs
> code where sometimes it pulls part content out of the JSON and
> sometimes it retrieves it using show --format=raw.  This in turn leads
> to asymmetry in content encoding handling, since notmuch handles
> content encoding for parts included in the JSON (and there's no good
> way around that since JSON is Unicode), but not for parts retrieved as
> raw.
> 
> The idea discussed on IRC was to remove all part content from the JSON
> output and to always use show to retrieve it, possibly beefing up
> show's support for content decoding (and possibly introducing a way to
> retrieve multiple raw parts at once to avoid re-parsing).  This would
> get the JSON format out of the business of guessing what consumers
> need, simplify the Emacs code, and normalize content encoding
> handling.

Full ACK.

One concern though (IIUC): Due to the prevalence of retarded MUA's, not
outputting 'text/plain' and/or 'text/html' parts is unfortunately all
too often equivalent to not outputting anything at all, so wouldn't we,
in essence, be reducing `show --format=json' to an ever-so-slightly
augmented `search --format=json' ?


Peace

-- 
Pieter


[PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."

2012-01-14 Thread Pieter Praet
On Fri, 13 Jan 2012 16:06:17 +, David Edmondson  wrote:
> On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet  wrote:
> > Less code, same results, without sacrificing readability.
> 
> Does this change correctly re-indent the line following the if/unless?

It does...

Or so I thought... I appear to have forgotten to correct the indentation
@ `notmuch-hello-insert-tags'.


Unfortunately git doesn't provide a way to diff the whitespace itself;
  (eg. "git diff --word-diff=color --word-diff-regex='[ \t]*'")

Setting "color.diff.whitespace" (and perhaps expanding the scope of
"core.whitespace") only colorizes *erroneous* whitespace...


Does this really warrant a v2, or might we simply leave it as yet
another victim for Tomi's uncrustify-for-elisp [1] ?


Peace

-- 
Pieter

[1] id:"yf639bkexs3.fsf at taco2.nixu.fi"


notmuch release 0.11 now available

2012-01-14 Thread Xavier Maillard
Hi,

congratulations to all of the notmuch contributors.

Keep up the effort

/Xavier


[PATCH] emacs: globally replace non-branching "(if COND (progn ..." with "(when ..."

2012-01-14 Thread Pieter Praet
Less code, same results, without sacrificing readability.

---
 emacs/notmuch-show.el |   20 +---
 emacs/notmuch-wash.el |   47 +++
 emacs/notmuch.el  |   28 +---
 3 files changed, 45 insertions(+), 50 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 0cbf354..1e190ae 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1226,11 +1226,10 @@ any effects from previous calls to
   ;; If a small number of lines from the previous message are
   ;; visible, realign so that the top of the current message is at
   ;; the top of the screen.
-  (if (<= (count-screen-lines (window-start) start-of-message)
- next-screen-context-lines)
- (progn
-   (goto-char (notmuch-show-message-top))
-   (notmuch-show-message-adjust)))
+  (when (<= (count-screen-lines (window-start) start-of-message)
+   next-screen-context-lines)
+   (goto-char (notmuch-show-message-top))
+   (notmuch-show-message-adjust))
   ;; Move to the top left of the window.
   (goto-char (window-start)))
  (t
@@ -1423,12 +1422,11 @@ argument, hide all of the messages."
   ;; Move to the next item in the search results, if any.
   (let ((parent-buffer notmuch-show-parent-buffer))
 (notmuch-kill-this-buffer)
-(if parent-buffer
-   (progn
- (switch-to-buffer parent-buffer)
- (forward-line)
- (if show-next
- (notmuch-search-show-thread))
+(when parent-buffer
+  (switch-to-buffer parent-buffer)
+  (forward-line)
+  (if show-next
+ (notmuch-search-show-thread)

 (defun notmuch-show-archive-thread ()
   "Archive each message in thread, then show next thread from search.
diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 5c1e830..67143e5 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -336,30 +336,29 @@ patch and then guesses the extent of the patch, there is 
scope
 for error."

   (goto-char (point-min))
-  (if (re-search-forward diff-file-header-re nil t)
-  (progn
-   (beginning-of-line -1)
-   (let ((patch-start (point))
- (patch-end (point-max))
- part)
- (goto-char patch-start)
- (if (or
-  ;; Patch ends with signature.
-  (re-search-forward notmuch-wash-signature-regexp nil t)
-  ;; Patch ends with bugtraq comment.
-  (re-search-forward "^\\*\\*\\* " nil t))
- (setq patch-end (match-beginning 0)))
- (save-restriction
-   (narrow-to-region patch-start patch-end)
-   (setq part (plist-put part :content-type "inline-patch-fake-part"))
-   (setq part (plist-put part :content (buffer-string)))
-   (setq part (plist-put part :id -1))
-   (setq part (plist-put part :filename
- (notmuch-wash-subject-to-patch-filename
-  (plist-get
-   (plist-get msg :headers) :Subject
-   (delete-region (point-min) (point-max))
-   (notmuch-show-insert-bodypart nil part depth))
+  (when (re-search-forward diff-file-header-re nil t)
+(beginning-of-line -1)
+(let ((patch-start (point))
+ (patch-end (point-max))
+ part)
+  (goto-char patch-start)
+  (if (or
+  ;; Patch ends with signature.
+  (re-search-forward notmuch-wash-signature-regexp nil t)
+  ;; Patch ends with bugtraq comment.
+  (re-search-forward "^\\*\\*\\* " nil t))
+ (setq patch-end (match-beginning 0)))
+  (save-restriction
+   (narrow-to-region patch-start patch-end)
+   (setq part (plist-put part :content-type "inline-patch-fake-part"))
+   (setq part (plist-put part :content (buffer-string)))
+   (setq part (plist-put part :id -1))
+   (setq part (plist-put part :filename
+ (notmuch-wash-subject-to-patch-filename
+  (plist-get
+   (plist-get msg :headers) :Subject
+   (delete-region (point-min) (point-max))
+   (notmuch-show-insert-bodypart nil part depth)

 ;;

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index ba84494..0d37a74 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -631,17 +631,16 @@ This function advances the next thread when finished."
  (goto-char (point-max))
  (if (eq status 'signal)
  (insert "Incomplete search results (search process was 
killed).\n"))
- (if (eq status 'exit)
- (progn
-   (if notmuch-search-process-filter-data
-   (insert (concat "Error: Unexpected output from 
notmuch search:\n" notmuch-search-process-filter-data)))
-   (insert 

[PATCH] emacs: globally replace non-branching "(if (not ..." with "(unless ..."

2012-01-14 Thread Pieter Praet
On Fri, 13 Jan 2012 08:23:55 +, David Edmondson  wrote:
> On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet  wrote:
> > Less code, same results, without sacrificing readability.
> 
> +1, but why not replace non-branching `if' with `when' as well?

I was planning to do that when the `unless' patch was accepted,
but after reading Xavier and Tomi's replies, I've changed my mind.

Looking at "subr.el", it's actually more efficient to use `if'
(implemented in C) instead of `when' (a macro, which essentially
runs "if conf (progn ...)".

The amount of non-branching "(if COND (progn ..."  statements is very
limited though, so if inclined to "fix" them nonetheless, a patch
follows (relative to the previous one!).


Peace

-- 
Pieter


[PATCH] test: don't bail out of `run_emacs' too early when missing prereqs

2012-01-14 Thread Pieter Praet
When running the Emacs tests in verbose mode, only the first missing
prereq is reported because the `run_emacs' function is short-circuited
early:

  #+begin_example
emacs: Testing emacs interface
 missing prerequisites: [0]  emacs(1)
 skipping test: [0]  Basic notmuch-hello view in emacs
 SKIP   [0]  Basic notmuch-hello view in emacs
  #+end_example

This can lead to situations reminiscent of "dependency hell", so instead
of returning based on each individual `test_require_external_prereq's exit
status, we now do so only after checking all the prereqs:

  #+begin_example
emacs: Testing emacs interface
 missing prerequisites: [0]  dtach(1) emacs(1) emacsclient(1)
 skipping test: [0]  Basic notmuch-hello view in emacs
 SKIP   [0]  Basic notmuch-hello view in emacs
  #+end_example

Also added missing prereq for dtach(1).
---
 test/test-lib.sh |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 82767c0..d1fbc05 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -907,8 +907,11 @@ EOF

 test_emacs () {
# test dependencies beforehand to avoid the waiting loop below
-   test_require_external_prereq emacs || return
-   test_require_external_prereq emacsclient || return
+   missing_dependencies=
+   test_require_external_prereq dtach || missing_dependencies=1
+   test_require_external_prereq emacs || missing_dependencies=1
+   test_require_external_prereq emacsclient || missing_dependencies=1
+   test -z "$missing_dependencies" || return

if [ -z "$EMACS_SERVER" ]; then
server_name="notmuch-test-suite-$$"
-- 
1.7.8.1



[PATCH] test: don't bail out of `run_emacs' too early when missing prereqs

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 21:34:29 +0400, Dmitry Kurochkin  wrote:
> On Thu, 12 Jan 2012 18:16:59 +0100, Pieter Praet  wrote:
> > When running the Emacs tests in verbose mode, only the first missing
> > prereq is reported because the `run_emacs' function is short-circuited
> > early:
> > 
> >   #+begin_example
> > emacs: Testing emacs interface
> >  missing prerequisites: [0]  emacs(1)
> >  skipping test: [0]  Basic notmuch-hello view in emacs
> >  SKIP   [0]  Basic notmuch-hello view in emacs
> >   #+end_example
> > 
> > This can lead to situations reminiscent of "dependency hell", so instead
> > of returning based on each individual `test_require_external_prereq's exit
> > status, we now do so by checking $test_subtest_missing_external_prereqs_:
> > 
> >   #+begin_example
> > emacs: Testing emacs interface
> >  missing prerequisites: [0]  dtach(1) emacs(1) emacsclient(1)
> >  skipping test: [0]  Basic notmuch-hello view in emacs
> >  SKIP   [0]  Basic notmuch-hello view in emacs
> >   #+end_example
> > 
> > Also add missing prereq for dtach(1).
> > 
> > ---
> >  test/test-lib.sh |6 --
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > index 82767c0..6ec3882 100644
> > --- a/test/test-lib.sh
> > +++ b/test/test-lib.sh
> > @@ -907,8 +907,10 @@ EOF
> >  
> >  test_emacs () {
> > # test dependencies beforehand to avoid the waiting loop below
> > -   test_require_external_prereq emacs || return
> > -   test_require_external_prereq emacsclient || return
> > +   test_require_external_prereq dtach
> > +   test_require_external_prereq emacs
> > +   test_require_external_prereq emacsclient
> > +   test -z "$test_subtest_missing_external_prereqs_" || return
> 
> There may be other missing dependencies before test_emacs() is called
> and $test_subtest_missing_external_prereqs_ would not be blank.  [...]

True, hadn't though of that...

> [...] Also, I
> would like to keep the number of functions that use
> $test_subtest_missing_external_prereqs_ minimal.  [...]

Could you elaborate on that?

> [...] How about:
> 
>   missing_dependencies=
>   test_require_... dtach || missing_dependencies=1
>   test_require_... emacs || missing_dependencies=1
>   ...
>   test -z "$missing_dependencies" || return
> 

Agreed!  Patch follows.

> Regards,
>   Dmitry
> 
> >  
> > if [ -z "$EMACS_SERVER" ]; then
> > server_name="notmuch-test-suite-$$"
> > -- 
> > 1.7.8.1
> > 


Peace

-- 
Pieter


[PATCH v2] emacs: logically group def{custom,face}s

2012-01-14 Thread Pieter Praet
On Fri, 13 Jan 2012 16:15:59 +, David Edmondson  wrote:
> On Thu, 12 Jan 2012 23:31:34 -0400, David Bremner  
> wrote:
> > On Thu, 12 Jan 2012 18:12:16 +0100, Pieter Praet  
> > wrote:
> > > To allow for expansion whilst keeping everything tidy and organized,
> > > move all defcustom/defface variables to the following subgroups,
> > > defined in notmuch-lib.el:
> > > 
> > > - Hello
> > > - Search
> > > - Show
> > > - Send
> > > - Crypto
> > > - Hooks
> > > - Appearance
> > > - External Commands
> > 
> > I didn't investigate too closely, but I noticed when I customize-group
> > emacs, each subgroup has some explanatory text beside it. Did you omit
> > that on purpose?
> 
> That should be a requirement, I think.

I agree;  But for ~every rule, there are exceptions  :)


Peace

-- 
Pieter


[PATCH] emacs: logically group def{custom,face}s

2012-01-14 Thread Pieter Praet
To allow for expansion whilst keeping everything tidy and organized,
move all defcustom/defface variables to the following subgroups,
defined in notmuch-lib.el:

- Hello
- Search
- Show
- Send
- Crypto
- Hooks
- External Commands
- Appearance

As an added benefit, defcustom keyword args are now consistently
in order of appearance @ defcustom's docstring (OCD much?).

---
 emacs/notmuch-address.el |3 +-
 emacs/notmuch-crypto.el  |   22 +---
 emacs/notmuch-hello.el   |   34 +--
 emacs/notmuch-lib.el |   45 -
 emacs/notmuch-maildir-fcc.el |6 ++--
 emacs/notmuch-message.el |2 +-
 emacs/notmuch-mua.el |   21 ++-
 emacs/notmuch-show.el|   33 +++--
 emacs/notmuch.el |   35 
 9 files changed, 132 insertions(+), 69 deletions(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index 8eba7a0..2e8b840 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -28,7 +28,8 @@
 single argument and output a list of possible matches, one per
 line."
   :type 'string
-  :group 'notmuch)
+  :group 'notmuch-send
+  :group 'notmuch-external)

 (defvar notmuch-address-message-alist-member
   
'("^\\(Resent-\\)?\\(To\\|B?Cc\\|Reply-To\\|From\\|Mail-Followup-To\\|Mail-Copies-To\\):"
diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index ac30098..232c1a0 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -34,38 +34,44 @@ The effect of setting this variable can be seen temporarily 
by
 providing a prefix when viewing a signed or encrypted message, or
 by providing a prefix when reloading the message in notmuch-show
 mode."
-  :group 'notmuch
-  :type 'boolean)
+  :type 'boolean
+  :group 'notmuch-crypto)

 (defface notmuch-crypto-part-header
   '((t (:foreground "blue")))
   "Face used for crypto parts headers."
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)

 (defface notmuch-crypto-signature-good
   '((t (:background "green" :foreground "black")))
   "Face used for good signatures."
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)

 (defface notmuch-crypto-signature-good-key
   '((t (:background "orange" :foreground "black")))
   "Face used for good signatures."
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)

 (defface notmuch-crypto-signature-bad
   '((t (:background "red" :foreground "black")))
   "Face used for bad signatures."
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)

 (defface notmuch-crypto-signature-unknown
   '((t (:background "red" :foreground "black")))
   "Face used for signatures of unknown status."
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)

 (defface notmuch-crypto-decryption
   '((t (:background "purple" :foreground "black")))
   "Face used for encryption/decryption status messages."
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)

 (define-button-type 'notmuch-crypto-status-button-type
   'action (lambda (button) (message (button-get button 'help-echo)))
diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 02017ce..2493a26 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -35,12 +35,12 @@
 (defcustom notmuch-recent-searches-max 10
   "The number of recent searches to store and display."
   :type 'integer
-  :group 'notmuch)
+  :group 'notmuch-hello)

 (defcustom notmuch-show-empty-saved-searches nil
   "Should saved searches with no messages be listed?"
   :type 'boolean
-  :group 'notmuch)
+  :group 'notmuch-hello)

 (defun notmuch-sort-saved-searches (alist)
   "Generate an alphabetically sorted saved searches alist."
@@ -60,7 +60,7 @@ alist to be used."
 (const :tag "Sort alphabetically" notmuch-sort-saved-searches)
 (function :tag "Custom sort function"
   :value notmuch-sort-saved-searches))
-  :group 'notmuch)
+  :group 'notmuch-hello)

 (defvar notmuch-hello-indent 4
   "How much to indent non-headers.")
@@ -68,12 +68,13 @@ alist to be used."
 (defcustom notmuch-show-logo t
   "Should the notmuch logo be shown?"
   :type 'boolean
-  :group 'notmuch)
+  :group 'notmuch-hello
+  :group 'notmuch-appearance)

 (defcustom notmuch-show-all-tags-list nil
   "Should all tags be shown in the notmuch-hello view?"
   :type 'boolean
-  :group 'notmuch)
+  :group 'notmuch-hello)

 (defcustom notmuch-hello-tag-list-make-query nil
   "Function or string to generate queries for the all tags list.
@@ -89,12 +90,12 @@ should return a filter for that tag, or nil to hide the 
tag."
 (string :tag "Custom filter"
 :value "tag:unread")
 (function :tag "Custom filter function"))
-  :group 'notmuch)
+  :group 'notmuch-hello)

 

[PATCH v2] emacs: logically group def{custom,face}s

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 23:31:34 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 18:12:16 +0100, Pieter Praet  wrote:
> > To allow for expansion whilst keeping everything tidy and organized,
> > move all defcustom/defface variables to the following subgroups,
> > defined in notmuch-lib.el:
> > 
> > - Hello
> > - Search
> > - Show
> > - Send
> > - Crypto
> > - Hooks
> > - Appearance
> > - External Commands
> 
> I didn't investigate too closely, but I noticed when I customize-group
> emacs, each subgroup has some explanatory text beside it. Did you omit
> that on purpose?
> 

That is correct.  Coming up with decent docstrings is hard :)

Also, the group names are fairly descriptive in and of themselves IMO.

Anyways, I've given it a shot, but (I hope!) people will come up with
much better descriptions, so this probably isn't the end of it...

Patch follows.

> d


Peace

-- 
Pieter


[PATCH 2/2] NEWS: fix some old typos and trailing whitespace

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 23:11:35 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 18:06:14 +0100, Pieter Praet  wrote:
> > That would indeed have been the most sensible thing to do.
> > 
> > Could someone (who -unlike me- has already submitted his/her public SSH
> > key for commit access) add something along those lines to the Patch
> > Formatting guidelines [1] ?  Perhaps just quote Carl's statement [2] ?
> > 
> 
> Hi Pieter;
> 
> It's a wiki, anyone can push.
> 

Oh, I wasn't aware of this. (changed recently?)

http://notmuchmail.org/patchformatting/ has been amended!

> d


Peace

-- 
Pieter


[PATCH v2] Output unmodified Content-Type header value for JSON format.

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 23:16:29 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 18:08:08 +0100, Pieter Praet  wrote:
> > > > - The previous point is a bit of a counterargument to this, but in
> 
> > > I couls separate it.  I made is a single patch to avoid having a
> > > revision with broken emacs UI (and tests).
> > > 
> 
> > I'd like to propose to always apply patch series on a *topic* branch
> > which would then be merged back into 'master', thus avoiding this issue
> > altogether whilst making it more obvious which patches belong together
> > (eg. for easier cross-referencing with the ML).
> 
> Hi Pieter;
> 
> Sorry, I must have lost the thread somewhere.  What application are you
> thinking about, and what issue do you think that topic branches avoid?
> 

No specific application;

In the case of patch series, it may occur that an individual commit
leaves the code in a broken state.  One might argue that devs need to
ensure that the project builds cleanly at every commit, but this can be
prohibitively time-consuming or even simply impossible due to the need
to keep changes atomic and area-specific (eg. changes to the binary and
the Emacs UI should be separate commits irrespective of their relation).

So it would make sense to consistently apply (certain, or perhaps all?)
patch series on an ad-hoc branch which would then be merged back into
'master', instead of being directly applied on 'master'.

Aside from preventing broken builds (as was Dmitry's reason to consolidate
a series touching on multiple distinct "spheres" of the code into a single
patch), series of related commits would also be clearly delineated/clustered,
facilitating source archaeology. (and all without making the commit log exude
the odor of entropy incarnate)

> d
> 


Peace

-- 
Pieter


[PATCH] test: cli: getting/setting/removing config values

2012-01-14 Thread Pieter Praet
Full test coverage for getting, setting and removing options in
notmuch(1)'s config file ($NOTMUCH_CONFIG or $HOME/.notmuch-config).

---
Please *do* take note of the FIXME in the last test!

 test/config   |   88 +
 test/notmuch-test |1 +
 2 files changed, 89 insertions(+), 0 deletions(-)
 create mode 100755 test/config

diff --git a/test/config b/test/config
new file mode 100755
index 000..ee3126c
--- /dev/null
+++ b/test/config
@@ -0,0 +1,88 @@
+#!/usr/bin/env bash
+test_description='notmuch config'
+. ./test-lib.sh
+
+
+config_options=(
+"database.path"
+"user.name"
+"user.primary_email"
+"user.other_email"
+"new.tags"
+"maildir.synchronize_flags"
+)
+
+
+test_begin_subtest 'getting config: "config get ."'
+echo -n "" > OUTPUT
+for i in ${config_options[*]} ; do
+notmuch config get "${i}"
+done >> OUTPUT
+cat >EXPECTED <. [values ...]"'
+notmuch config set database.path /path/to/maildir
+notmuch config set user.name "User Name"
+notmuch config set user.primary_email primary_email at notmuchmail.org
+notmuch config set user.other_email alt1 at notmuchmail.org alt2 at 
notmuchmail.org
+notmuch config set new.tags tag1 tag2 tag3
+notmuch config set maildir.synchronize_flags false
+echo -n "" > OUTPUT
+for i in ${config_options[*]} ; do
+notmuch config get "${i}"
+done >> OUTPUT
+cat >EXPECTED <."'
+notmuch config set database.path
+notmuch config set user.name
+notmuch config set user.primary_email
+notmuch config set user.other_email
+notmuch config set new.tags
+notmuch config set maildir.synchronize_flags
+echo -n "" > OUTPUT
+for i in ${config_options[*]} ; do
+notmuch config get "${i}"
+done >> OUTPUT
+
+# FIXME: Not the most robust nor portable solution here...
+# Especially `hostname --domain' may have unwanted effects on
+# some platforms, e.g. setting your hostname to "--domain" ;)
+fallback_name="$(grep $(id -un) /etc/passwd | cut -d ":" -f 5 | cut -d "," -f 
1)"
+fallback_email="$(id -un)@$(hostname).$(hostname --domain)"
+
+cat >EXPECTED 

[PATCH] test: cli: getting/setting/removing config values

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 23:42:04 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 18:30:01 +0100, Pieter Praet  wrote:
> > Should have come before commit 1df71b55
> 
> This doesn't seem like an especially helpful commit
> message. Editorializing is ok, I guess, but maybe keep it below the
> '---' ?
> 

You're right.  The message I was trying to convey (quite tersely),
was that these tests were written in response to commit 1df71b55
(and should probably be retroactively ran on 1df71b55~1).

I realize now that it could potentially be wrongly interpreted as
criticism of sorts, which of course wasn't my intention.

Anyways, I ran the tests on both 1df71b55~1 and 1df71b55, and
-as expected- everything checks out fine.

Amended patch follows.

> d


Peace

-- 
Pieter


revised patch for gmime init, with test.

2012-01-14 Thread Pieter Praet
On Fri, 13 Jan 2012 05:05:35 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner  
> wrote:
> > On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet  
> > wrote:
> > > On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  
> > > wrote:
> > 
> > > with differing hashes), this has the potential of causing confusion
> > > and/or quite some extra work when debugging using git-bisect(1), so
> > > I'd like to propose that bugfixes for (to-be-)released code are only
> > > applied on the 'maint' branch ('release' in the case of Notmuch),
> > > and then immediately merged back into 'master'.  In fact, this would
> > > preferrably happen after *every* (series of) commit(s) on the 'maint'
> > > branch, to prevent issues like [1].
> > 
> > There is some merit it to this. On the other hand, it makes the history
> > messier.  [1] would have also been prevented by making the patch against
> > the right branch.
> 
> I thought about this a bit more, and I agree that at least the release
> candidates (basically anything tagged on branch release) ought to be
> merged back to master. Since any series of bugfix patches seems to be
> cause for a new release candidate, this should avoid the need to have
> doubly applied patches.
> 

Thanks!

> I'm less convinced about the need to merge every little doc change and
> debian packaging change back to master right away. This might be a
> purely aesthetic objection; [...]

See my previous reply [1].

> [...] I'm not sure if the extra merge commits
> cause any problems for e.g. bisection.
> 

Infrequent merging increases the possibility of bugs due to unforeseen
interactions between commits on different branches, which is likely to
require one to do a multitude of fake merges (`git merge --no-commit')
in order to properly track down the offending commit, so... frequent
merging would actually *prevent* issues when bisecting.


> d


Peace

-- 
Pieter

[1] id:"878vlar7ka.fsf at praet.org"


revised patch for gmime init, with test.

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner  wrote:
> On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet  wrote:
> > On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner  
> > wrote:
> 
> > with differing hashes), this has the potential of causing confusion
> > and/or quite some extra work when debugging using git-bisect(1), so
> > I'd like to propose that bugfixes for (to-be-)released code are only
> > applied on the 'maint' branch ('release' in the case of Notmuch),
> > and then immediately merged back into 'master'.  In fact, this would
> > preferrably happen after *every* (series of) commit(s) on the 'maint'
> > branch, to prevent issues like [1].
> 
> There is some merit it to this. On the other hand, it makes the history
> messier.  [...]

This *can* get rather messy when interlacing the 'master'/'maint' merges
with non-rebased changesets pulled from other repos (most often aren't
rebased in advance since they've already been published), which is a
common occurence in the magit [1] repo.

But take Org-mode [2] for example (which is an *extremely* active project),
where non-rebased changesets are rare(ish), and where the 'maint' branch
is constantly being merged back into 'master', resulting in a very clean
ladder-like commit log, eg. :

  --******---**---*---*--- master
 \ / / \
  *---*---*-*---*--- maint
 0.11   bugfix   NEWSbugfix   0.12~rc1


> [...] [1] would have also been prevented by making the patch against
> the right branch.
> 

True, but if we can obviate the need to check if we're on the right
branch altogether, without causing any unwanted side-effects, wouldn't
it be counterproductive not to?


Peace

-- 
Pieter

[1] git://github.com/magit/magit.git
[2] git://orgmode.org/org-mode.git


[RFC] vim plugin rewrite II

2012-01-14 Thread an...@khirnov.net

Hi,
this is a followup to my mail from spring where i presented a partial
rewrite of the vim plugin using python. There weren't many comments back
then, so I hope there will be more now.

I've changed the filenames so my version now coexists with the current
vim plugin. You can find it in

git://git.khirnov.net/git/notmuch

branch vim. Simply copy vim/plugin/{nm_vim.py,notmuch-vimpy.vim} to the
vim plugins dir and vim/syntax/{nm_vimpy*} to the vim syntax dir and run
:NMVimpy() in vim. You'll need vim with python support and
python-notmuch bindings.

The advantages over current vim client are still the following:
* sending and displaying/saving attachments
* much better unicode support
* tag name and search commands completion
* proper representation of the thread structure
* easier to extend thanks to python's massive standard library

Please comment.

-- 
Anton Khirnov


[PATCH] test: cli: getting/setting/removing config values

2012-01-14 Thread David Bremner
On Sat, 14 Jan 2012 09:57:56 +0100, Pieter Praet  wrote:
> Full test coverage for getting, setting and removing options in
> notmuch(1)'s config file ($NOTMUCH_CONFIG or $HOME/.notmuch-config).
> 
> ---
> +
> +# FIXME: Not the most robust nor portable solution here...
> +# Especially `hostname --domain' may have unwanted effects on
> +# some platforms, e.g. setting your hostname to "--domain" ;)
> +fallback_name="$(grep $(id -un) /etc/passwd | cut -d ":" -f 5 | cut -d "," 
> -f 1)"
> +fallback_email="$(id -un)@$(hostname).$(hostname --domain)"

I'm not sure how portable it is, but maybe dnsdomainname would at least
have better failure modes.

I also wondered about using getent instead of grep.

d


Please Update Current GLib Requirements For notmuch: - (glib-2.0 >= 2.14) + (glib-2.0 >= 2.22)

2012-01-14 Thread datap...@gmail.com
I've just tried to compile notmuch, after having downloaded it a few
hours back, via: `git clone git://notmuchmail.org/git/notmuch`.
Unfortunately, even though I seem to have all needed dependencies in
place, during the build I ran into the following error:
lib/query.cc: In function 'int
_notmuch_threads_destructor(notmuch_threads_t*)':
lib/query.cc:311: error: 'g_array_unref' was not declared in this scope

Looking into this 'g_array_unref' function, I found that it should be
provided by glib, but it seems that it isn't even defined in any of the
header files installed by glib on this system (v2.20.3), although I did
find it within /usr/include/glib-2.0/glib/garray.h on a system with a
newer version (v2.24.2).  Looking into its documentation, I found the
following:
http://developer.gnome.org/glib/2.31/glib-Arrays.html#g-array-unref
And specifically note:
"g_array_ref ()
...
Since 2.22"

Now, unless I'm misinterpreting this, it seems to me that the
documentation is stating that the function has only been available since
glib v2.22...


--
Sub


---
DO NOT REPLY TO THIS MESSAGE, THIS ADDRESS IS NOT MONITORED


[RFC] vim plugin rewrite II

2012-01-14 Thread anton

Hi,
this is a followup to my mail from spring where i presented a partial
rewrite of the vim plugin using python. There weren't many comments back
then, so I hope there will be more now.

I've changed the filenames so my version now coexists with the current
vim plugin. You can find it in

git://git.khirnov.net/git/notmuch

branch vim. Simply copy vim/plugin/{nm_vim.py,notmuch-vimpy.vim} to the
vim plugins dir and vim/syntax/{nm_vimpy*} to the vim syntax dir and run
:NMVimpy() in vim. You'll need vim with python support and
python-notmuch bindings.

The advantages over current vim client are still the following:
* sending and displaying/saving attachments
* much better unicode support
* tag name and search commands completion
* proper representation of the thread structure
* easier to extend thanks to python's massive standard library

Please comment.

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


Re: revised patch for gmime init, with test.

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner da...@tethera.net wrote:
 On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet pie...@praet.org wrote:
  On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner da...@tethera.net wrote:
 
  with differing hashes), this has the potential of causing confusion
  and/or quite some extra work when debugging using git-bisect(1), so
  I'd like to propose that bugfixes for (to-be-)released code are only
  applied on the 'maint' branch ('release' in the case of Notmuch),
  and then immediately merged back into 'master'.  In fact, this would
  preferrably happen after *every* (series of) commit(s) on the 'maint'
  branch, to prevent issues like [1].
 
 There is some merit it to this. On the other hand, it makes the history
 messier.  [...]

This *can* get rather messy when interlacing the 'master'/'maint' merges
with non-rebased changesets pulled from other repos (most often aren't
rebased in advance since they've already been published), which is a
common occurence in the magit [1] repo.

But take Org-mode [2] for example (which is an *extremely* active project),
where non-rebased changesets are rare(ish), and where the 'maint' branch
is constantly being merged back into 'master', resulting in a very clean
ladder-like commit log, eg. :

  --******---**---*---*--- master
 \ / / \
  *---*---*-*---*--- maint
 0.11   bugfix   NEWSbugfix   0.12~rc1


 [...] [1] would have also been prevented by making the patch against
 the right branch.
 

True, but if we can obviate the need to check if we're on the right
branch altogether, without causing any unwanted side-effects, wouldn't
it be counterproductive not to?


Peace

-- 
Pieter

[1] git://github.com/magit/magit.git
[2] git://orgmode.org/org-mode.git
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: revised patch for gmime init, with test.

2012-01-14 Thread Pieter Praet
On Fri, 13 Jan 2012 05:05:35 -0400, David Bremner da...@tethera.net wrote:
 On Thu, 12 Jan 2012 23:46:46 -0400, David Bremner da...@tethera.net wrote:
  On Thu, 12 Jan 2012 18:25:38 +0100, Pieter Praet pie...@praet.org wrote:
   On Sat, 31 Dec 2011 23:22:46 -0400, David Bremner da...@tethera.net 
   wrote:
  
   with differing hashes), this has the potential of causing confusion
   and/or quite some extra work when debugging using git-bisect(1), so
   I'd like to propose that bugfixes for (to-be-)released code are only
   applied on the 'maint' branch ('release' in the case of Notmuch),
   and then immediately merged back into 'master'.  In fact, this would
   preferrably happen after *every* (series of) commit(s) on the 'maint'
   branch, to prevent issues like [1].
  
  There is some merit it to this. On the other hand, it makes the history
  messier.  [1] would have also been prevented by making the patch against
  the right branch.
 
 I thought about this a bit more, and I agree that at least the release
 candidates (basically anything tagged on branch release) ought to be
 merged back to master. Since any series of bugfix patches seems to be
 cause for a new release candidate, this should avoid the need to have
 doubly applied patches.
 

Thanks!

 I'm less convinced about the need to merge every little doc change and
 debian packaging change back to master right away. This might be a
 purely aesthetic objection; [...]

See my previous reply [1].

 [...] I'm not sure if the extra merge commits
 cause any problems for e.g. bisection.
 

Infrequent merging increases the possibility of bugs due to unforeseen
interactions between commits on different branches, which is likely to
require one to do a multitude of fake merges (`git merge --no-commit')
in order to properly track down the offending commit, so... frequent
merging would actually *prevent* issues when bisecting.


 d


Peace

-- 
Pieter

[1] id:878vlar7ka@praet.org
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] test: cli: getting/setting/removing config values

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 23:42:04 -0400, David Bremner da...@tethera.net wrote:
 On Thu, 12 Jan 2012 18:30:01 +0100, Pieter Praet pie...@praet.org wrote:
  Should have come before commit 1df71b55
 
 This doesn't seem like an especially helpful commit
 message. Editorializing is ok, I guess, but maybe keep it below the
 '---' ?
 

You're right.  The message I was trying to convey (quite tersely),
was that these tests were written in response to commit 1df71b55
(and should probably be retroactively ran on 1df71b55~1).

I realize now that it could potentially be wrongly interpreted as
criticism of sorts, which of course wasn't my intention.

Anyways, I ran the tests on both 1df71b55~1 and 1df71b55, and
-as expected- everything checks out fine.

Amended patch follows.

 d


Peace

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


Re: [PATCH 2/2] NEWS: fix some old typos and trailing whitespace

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 23:11:35 -0400, David Bremner da...@tethera.net wrote:
 On Thu, 12 Jan 2012 18:06:14 +0100, Pieter Praet pie...@praet.org wrote:
  That would indeed have been the most sensible thing to do.
  
  Could someone (who -unlike me- has already submitted his/her public SSH
  key for commit access) add something along those lines to the Patch
  Formatting guidelines [1] ?  Perhaps just quote Carl's statement [2] ?
  
 
 Hi Pieter;
 
 It's a wiki, anyone can push.
 

Oh, I wasn't aware of this. (changed recently?)

http://notmuchmail.org/patchformatting/ has been amended!

 d


Peace

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


[PATCH] emacs: logically group def{custom,face}s

2012-01-14 Thread Pieter Praet
To allow for expansion whilst keeping everything tidy and organized,
move all defcustom/defface variables to the following subgroups,
defined in notmuch-lib.el:

- Hello
- Search
- Show
- Send
- Crypto
- Hooks
- External Commands
- Appearance

As an added benefit, defcustom keyword args are now consistently
in order of appearance @ defcustom's docstring (OCD much?).

---
 emacs/notmuch-address.el |3 +-
 emacs/notmuch-crypto.el  |   22 +---
 emacs/notmuch-hello.el   |   34 +--
 emacs/notmuch-lib.el |   45 -
 emacs/notmuch-maildir-fcc.el |6 ++--
 emacs/notmuch-message.el |2 +-
 emacs/notmuch-mua.el |   21 ++-
 emacs/notmuch-show.el|   33 +++--
 emacs/notmuch.el |   35 
 9 files changed, 132 insertions(+), 69 deletions(-)

diff --git a/emacs/notmuch-address.el b/emacs/notmuch-address.el
index 8eba7a0..2e8b840 100644
--- a/emacs/notmuch-address.el
+++ b/emacs/notmuch-address.el
@@ -28,7 +28,8 @@
 single argument and output a list of possible matches, one per
 line.
   :type 'string
-  :group 'notmuch)
+  :group 'notmuch-send
+  :group 'notmuch-external)
 
 (defvar notmuch-address-message-alist-member
   
'(^\\(Resent-\\)?\\(To\\|B?Cc\\|Reply-To\\|From\\|Mail-Followup-To\\|Mail-Copies-To\\):
diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index ac30098..232c1a0 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -34,38 +34,44 @@ The effect of setting this variable can be seen temporarily 
by
 providing a prefix when viewing a signed or encrypted message, or
 by providing a prefix when reloading the message in notmuch-show
 mode.
-  :group 'notmuch
-  :type 'boolean)
+  :type 'boolean
+  :group 'notmuch-crypto)
 
 (defface notmuch-crypto-part-header
   '((t (:foreground blue)))
   Face used for crypto parts headers.
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)
 
 (defface notmuch-crypto-signature-good
   '((t (:background green :foreground black)))
   Face used for good signatures.
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)
 
 (defface notmuch-crypto-signature-good-key
   '((t (:background orange :foreground black)))
   Face used for good signatures.
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)
 
 (defface notmuch-crypto-signature-bad
   '((t (:background red :foreground black)))
   Face used for bad signatures.
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)
 
 (defface notmuch-crypto-signature-unknown
   '((t (:background red :foreground black)))
   Face used for signatures of unknown status.
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)
 
 (defface notmuch-crypto-decryption
   '((t (:background purple :foreground black)))
   Face used for encryption/decryption status messages.
-  :group 'notmuch)
+  :group 'notmuch-crypto
+  :group 'notmuch-appearance)
 
 (define-button-type 'notmuch-crypto-status-button-type
   'action (lambda (button) (message (button-get button 'help-echo)))
diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
index 02017ce..2493a26 100644
--- a/emacs/notmuch-hello.el
+++ b/emacs/notmuch-hello.el
@@ -35,12 +35,12 @@
 (defcustom notmuch-recent-searches-max 10
   The number of recent searches to store and display.
   :type 'integer
-  :group 'notmuch)
+  :group 'notmuch-hello)
 
 (defcustom notmuch-show-empty-saved-searches nil
   Should saved searches with no messages be listed?
   :type 'boolean
-  :group 'notmuch)
+  :group 'notmuch-hello)
 
 (defun notmuch-sort-saved-searches (alist)
   Generate an alphabetically sorted saved searches alist.
@@ -60,7 +60,7 @@ alist to be used.
 (const :tag Sort alphabetically notmuch-sort-saved-searches)
 (function :tag Custom sort function
   :value notmuch-sort-saved-searches))
-  :group 'notmuch)
+  :group 'notmuch-hello)
 
 (defvar notmuch-hello-indent 4
   How much to indent non-headers.)
@@ -68,12 +68,13 @@ alist to be used.
 (defcustom notmuch-show-logo t
   Should the notmuch logo be shown?
   :type 'boolean
-  :group 'notmuch)
+  :group 'notmuch-hello
+  :group 'notmuch-appearance)
 
 (defcustom notmuch-show-all-tags-list nil
   Should all tags be shown in the notmuch-hello view?
   :type 'boolean
-  :group 'notmuch)
+  :group 'notmuch-hello)
 
 (defcustom notmuch-hello-tag-list-make-query nil
   Function or string to generate queries for the all tags list.
@@ -89,12 +90,12 @@ should return a filter for that tag, or nil to hide the 
tag.
 (string :tag Custom filter
 :value tag:unread)
 (function :tag Custom filter function))
-  :group 'notmuch)
+  :group 'notmuch-hello)
 
 (defcustom notmuch-hello-hide-tags nil
   List of 

Re: [PATCH v2] emacs: logically group def{custom,face}s

2012-01-14 Thread Pieter Praet
On Fri, 13 Jan 2012 16:15:59 +, David Edmondson d...@dme.org wrote:
 On Thu, 12 Jan 2012 23:31:34 -0400, David Bremner da...@tethera.net wrote:
  On Thu, 12 Jan 2012 18:12:16 +0100, Pieter Praet pie...@praet.org wrote:
   To allow for expansion whilst keeping everything tidy and organized,
   move all defcustom/defface variables to the following subgroups,
   defined in notmuch-lib.el:
   
   - Hello
   - Search
   - Show
   - Send
   - Crypto
   - Hooks
   - Appearance
   - External Commands
  
  I didn't investigate too closely, but I noticed when I customize-group
  emacs, each subgroup has some explanatory text beside it. Did you omit
  that on purpose?
 
 That should be a requirement, I think.

I agree;  But for ~every rule, there are exceptions  :)


Peace

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


[PATCH] test: don't bail out of `run_emacs' too early when missing prereqs

2012-01-14 Thread Pieter Praet
When running the Emacs tests in verbose mode, only the first missing
prereq is reported because the `run_emacs' function is short-circuited
early:

  #+begin_example
emacs: Testing emacs interface
 missing prerequisites: [0]  emacs(1)
 skipping test: [0]  Basic notmuch-hello view in emacs
 SKIP   [0]  Basic notmuch-hello view in emacs
  #+end_example

This can lead to situations reminiscent of dependency hell, so instead
of returning based on each individual `test_require_external_prereq's exit
status, we now do so only after checking all the prereqs:

  #+begin_example
emacs: Testing emacs interface
 missing prerequisites: [0]  dtach(1) emacs(1) emacsclient(1)
 skipping test: [0]  Basic notmuch-hello view in emacs
 SKIP   [0]  Basic notmuch-hello view in emacs
  #+end_example

Also added missing prereq for dtach(1).
---
 test/test-lib.sh |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 82767c0..d1fbc05 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -907,8 +907,11 @@ EOF
 
 test_emacs () {
# test dependencies beforehand to avoid the waiting loop below
-   test_require_external_prereq emacs || return
-   test_require_external_prereq emacsclient || return
+   missing_dependencies=
+   test_require_external_prereq dtach || missing_dependencies=1
+   test_require_external_prereq emacs || missing_dependencies=1
+   test_require_external_prereq emacsclient || missing_dependencies=1
+   test -z $missing_dependencies || return
 
if [ -z $EMACS_SERVER ]; then
server_name=notmuch-test-suite-$$
-- 
1.7.8.1

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


Re: [PATCH] emacs: globally replace non-branching (if (not ... with (unless ...

2012-01-14 Thread Pieter Praet
On Fri, 13 Jan 2012 08:23:55 +, David Edmondson d...@dme.org wrote:
 On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet pie...@praet.org wrote:
  Less code, same results, without sacrificing readability.
 
 +1, but why not replace non-branching `if' with `when' as well?

I was planning to do that when the `unless' patch was accepted,
but after reading Xavier and Tomi's replies, I've changed my mind.

Looking at subr.el, it's actually more efficient to use `if'
(implemented in C) instead of `when' (a macro, which essentially
runs if conf (progn ...).

The amount of non-branching (if COND (progn ...  statements is very
limited though, so if inclined to fix them nonetheless, a patch
follows (relative to the previous one!).


Peace

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


[PATCH] emacs: globally replace non-branching (if COND (progn ... with (when ...

2012-01-14 Thread Pieter Praet
Less code, same results, without sacrificing readability.

---
 emacs/notmuch-show.el |   20 +---
 emacs/notmuch-wash.el |   47 +++
 emacs/notmuch.el  |   28 +---
 3 files changed, 45 insertions(+), 50 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 0cbf354..1e190ae 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1226,11 +1226,10 @@ any effects from previous calls to
   ;; If a small number of lines from the previous message are
   ;; visible, realign so that the top of the current message is at
   ;; the top of the screen.
-  (if (= (count-screen-lines (window-start) start-of-message)
- next-screen-context-lines)
- (progn
-   (goto-char (notmuch-show-message-top))
-   (notmuch-show-message-adjust)))
+  (when (= (count-screen-lines (window-start) start-of-message)
+   next-screen-context-lines)
+   (goto-char (notmuch-show-message-top))
+   (notmuch-show-message-adjust))
   ;; Move to the top left of the window.
   (goto-char (window-start)))
  (t
@@ -1423,12 +1422,11 @@ argument, hide all of the messages.
   ;; Move to the next item in the search results, if any.
   (let ((parent-buffer notmuch-show-parent-buffer))
 (notmuch-kill-this-buffer)
-(if parent-buffer
-   (progn
- (switch-to-buffer parent-buffer)
- (forward-line)
- (if show-next
- (notmuch-search-show-thread))
+(when parent-buffer
+  (switch-to-buffer parent-buffer)
+  (forward-line)
+  (if show-next
+ (notmuch-search-show-thread)
 
 (defun notmuch-show-archive-thread ()
   Archive each message in thread, then show next thread from search.
diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 5c1e830..67143e5 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -336,30 +336,29 @@ patch and then guesses the extent of the patch, there is 
scope
 for error.
 
   (goto-char (point-min))
-  (if (re-search-forward diff-file-header-re nil t)
-  (progn
-   (beginning-of-line -1)
-   (let ((patch-start (point))
- (patch-end (point-max))
- part)
- (goto-char patch-start)
- (if (or
-  ;; Patch ends with signature.
-  (re-search-forward notmuch-wash-signature-regexp nil t)
-  ;; Patch ends with bugtraq comment.
-  (re-search-forward ^\\*\\*\\*  nil t))
- (setq patch-end (match-beginning 0)))
- (save-restriction
-   (narrow-to-region patch-start patch-end)
-   (setq part (plist-put part :content-type inline-patch-fake-part))
-   (setq part (plist-put part :content (buffer-string)))
-   (setq part (plist-put part :id -1))
-   (setq part (plist-put part :filename
- (notmuch-wash-subject-to-patch-filename
-  (plist-get
-   (plist-get msg :headers) :Subject
-   (delete-region (point-min) (point-max))
-   (notmuch-show-insert-bodypart nil part depth))
+  (when (re-search-forward diff-file-header-re nil t)
+(beginning-of-line -1)
+(let ((patch-start (point))
+ (patch-end (point-max))
+ part)
+  (goto-char patch-start)
+  (if (or
+  ;; Patch ends with signature.
+  (re-search-forward notmuch-wash-signature-regexp nil t)
+  ;; Patch ends with bugtraq comment.
+  (re-search-forward ^\\*\\*\\*  nil t))
+ (setq patch-end (match-beginning 0)))
+  (save-restriction
+   (narrow-to-region patch-start patch-end)
+   (setq part (plist-put part :content-type inline-patch-fake-part))
+   (setq part (plist-put part :content (buffer-string)))
+   (setq part (plist-put part :id -1))
+   (setq part (plist-put part :filename
+ (notmuch-wash-subject-to-patch-filename
+  (plist-get
+   (plist-get msg :headers) :Subject
+   (delete-region (point-min) (point-max))
+   (notmuch-show-insert-bodypart nil part depth)
 
 ;;
 
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index ba84494..0d37a74 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -631,17 +631,16 @@ This function advances the next thread when finished.
  (goto-char (point-max))
  (if (eq status 'signal)
  (insert Incomplete search results (search process was 
killed).\n))
- (if (eq status 'exit)
- (progn
-   (if notmuch-search-process-filter-data
-   (insert (concat Error: Unexpected output from 
notmuch search:\n notmuch-search-process-filter-data)))
-   (insert End of search 

Re: [PATCH] emacs: globally replace non-branching (if (not ... with (unless ...

2012-01-14 Thread Pieter Praet
On Fri, 13 Jan 2012 16:06:17 +, David Edmondson d...@dme.org wrote:
 On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet pie...@praet.org wrote:
  Less code, same results, without sacrificing readability.
 
 Does this change correctly re-indent the line following the if/unless?

It does...

Or so I thought... I appear to have forgotten to correct the indentation
@ `notmuch-hello-insert-tags'.


Unfortunately git doesn't provide a way to diff the whitespace itself;
  (eg. git diff --word-diff=color --word-diff-regex='[ \t]*')

Setting color.diff.whitespace (and perhaps expanding the scope of
core.whitespace) only colorizes *erroneous* whitespace...


Does this really warrant a v2, or might we simply leave it as yet
another victim for Tomi's uncrustify-for-elisp [1] ?


Peace

-- 
Pieter

[1] id:yf639bkexs3@taco2.nixu.fi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] Output unmodified Content-Type header value for JSON format.

2012-01-14 Thread Pieter Praet
On Thu, 12 Jan 2012 12:28:40 -0500, Austin Clements amdra...@mit.edu wrote:
 Quoth Pieter Praet on Jan 12 at  6:07 pm:
  On Tue, 22 Nov 2011 22:40:21 -0500, Austin Clements amdra...@mit.edu 
  wrote:
   Quoth Jameson Graef Rollins on Nov 20 at 12:10 pm:
The open question seems to be how we handle the content encoding
parameters.  My argument is that those should either be used by notmuch
to properly encode the content for the consumer.  If that's not
possible, then just those parameters needed by the consumer to decode
the content should be output.
   
   If notmuch is going to include part content in the JSON output (which
   perhaps it shouldn't, as per recent IRC discussions), then it must
   handle content encodings because JSON must be Unicode and therefore
   the content strings in the JSON must be Unicode.
  
  Having missed the IRC discussions: what is the rationale for not
  including (specific types of?) part content in the JSON output ?
  Eg. how about inline attached text/x-patch ?
 
 Technically the IRC discussion was about not including *any* part
 content in the JSON output, and always using show --format=raw or
 similar to retrieve desired parts.  Currently, notmuch includes part
 content in the JSON only for text/*, *except* when it's text/html.  I
 assume non-text parts are omitted because binary data is hard to
 represent in JSON and text/html is omitted because some people don't
 need it.  However, this leads to some peculiar asymmetry in the Emacs
 code where sometimes it pulls part content out of the JSON and
 sometimes it retrieves it using show --format=raw.  This in turn leads
 to asymmetry in content encoding handling, since notmuch handles
 content encoding for parts included in the JSON (and there's no good
 way around that since JSON is Unicode), but not for parts retrieved as
 raw.
 
 The idea discussed on IRC was to remove all part content from the JSON
 output and to always use show to retrieve it, possibly beefing up
 show's support for content decoding (and possibly introducing a way to
 retrieve multiple raw parts at once to avoid re-parsing).  This would
 get the JSON format out of the business of guessing what consumers
 need, simplify the Emacs code, and normalize content encoding
 handling.

Full ACK.

One concern though (IIUC): Due to the prevalence of retarded MUA's, not
outputting 'text/plain' and/or 'text/html' parts is unfortunately all
too often equivalent to not outputting anything at all, so wouldn't we,
in essence, be reducing `show --format=json' to an ever-so-slightly
augmented `search --format=json' ?


Peace

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


Re: [PATCH v2] emacs: Cycle through notmuch buffers rather than jumping to the last.

2012-01-14 Thread Pieter Praet
On Wed, 28 Dec 2011 08:29:58 +, David Edmondson d...@dme.org wrote:
 As suggested by j4ni in #notmuch, rename
 `notmuch-jump-to-recent-buffer' as `notmuch-cycle-notmuch-buffers' and
 have it behave accordingly.
 
 Consider `message-mode' buffers to be of interest.
 ---
  emacs/notmuch.el |   42 ++
  1 files changed, 30 insertions(+), 12 deletions(-)
 
 diff --git a/emacs/notmuch.el b/emacs/notmuch.el
 index c678c93..6a44d49 100644
 --- a/emacs/notmuch.el
 +++ b/emacs/notmuch.el
 @@ -1055,21 +1055,39 @@ current search results AND that are tagged with the 
 given tag.
(interactive)
(notmuch-hello))
  
 +(defun notmuch-interesting-buffer (b)
 +  Is the current buffer of interest to a notmuch user?
 +  (with-current-buffer b
 +(memq major-mode '(notmuch-show-mode
 +notmuch-search-mode
 +notmuch-hello-mode
 +message-mode
 +
  ;;;###autoload
 -(defun notmuch-jump-to-recent-buffer ()
 -  Jump to the most recent notmuch buffer (search, show or hello).
 +(defun notmuch-cycle-notmuch-buffers ()
 +  Cycle through any existing notmuch buffers (search, show or hello).
  
 -If no recent buffer is found, run `notmuch'.
 +If the current buffer is the only notmuch buffer, bury it. If no
 +notmuch buffers exist, run `notmuch'.
(interactive)
 -  (let ((last
 -  (loop for buffer in (buffer-list)
 -if (with-current-buffer buffer
 - (memq major-mode '(notmuch-show-mode
 -notmuch-search-mode
 -notmuch-hello-mode)))
 -return buffer)))
 -(if last
 - (switch-to-buffer last)
 +
 +  (let (start first)
 +;; If the current buffer is a notmuch buffer, remember it and then
 +;; bury it.
 +(when (notmuch-interesting-buffer (current-buffer))
 +  (setq start (current-buffer))
 +  (bury-buffer))
 +
 +;; Find the first notmuch buffer.
 +(setq first (loop for buffer in (buffer-list)
 +  if (notmuch-interesting-buffer buffer)
 +  return buffer))
 +
 +(if first
 + ;; If the first one we found is any other than the starting
 + ;; buffer, switch to it.
 + (unless (eq first start)
 +   (switch-to-buffer first))
(notmuch
  
  (setq mail-user-agent 'notmuch-user-agent)
 -- 
 1.7.7.3

Signed-off-by: Pieter Praet pie...@praet.org  !


Might I ask, to what key(chord) have you bound this ?  Due to its
usefulness, I'm inclined to bind it to [SPC], but on second though,
that might be a bit on the intense side...


Peace

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


Re: [PATCH v2] emacs: Cycle through notmuch buffers rather than jumping to the last.

2012-01-14 Thread Pieter Praet
On Tue, 10 Jan 2012 22:15:02 +0200, Jani Nikula j...@nikula.org wrote:
 I don't feel qualified to review,

Allow me to disagree: your feeling and reality appear to be disjoint ATM :)


Peace

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


Re: [PATCH] emacs: globally replace non-branching (if (not ... with (unless ...

2012-01-14 Thread Pieter Praet
On Sat, 14 Jan 2012 10:14:57 +0100, Pieter Praet pie...@praet.org wrote:
 On Fri, 13 Jan 2012 08:23:55 +, David Edmondson d...@dme.org wrote:
  On Thu, 12 Jan 2012 18:33:06 +0100, Pieter Praet pie...@praet.org wrote:
   Less code, same results, without sacrificing readability.
  
  +1, but why not replace non-branching `if' with `when' as well?
 
 I was planning to do that when the `unless' patch was accepted,
^^
   submit

 but after reading Xavier and Tomi's replies, I've changed my mind.
   
   I hadn't, though...

 
 Looking at subr.el, it's actually more efficient to use `if'
 (implemented in C) instead of `when' (a macro, which essentially
 runs if conf (progn ...).
 
 The amount of non-branching (if COND (progn ...  statements is very
 limited though, so if inclined to fix them nonetheless, a patch
 follows (relative to the previous one!).
 
 
 Peace
 
 -- 
 Pieter

Typsos FTW!

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


Re: [PATCH] test: cli: getting/setting/removing config values

2012-01-14 Thread David Bremner
On Sat, 14 Jan 2012 09:57:56 +0100, Pieter Praet pie...@praet.org wrote:
 Full test coverage for getting, setting and removing options in
 notmuch(1)'s config file ($NOTMUCH_CONFIG or $HOME/.notmuch-config).
 
 ---
 +
 +# FIXME: Not the most robust nor portable solution here...
 +# Especially `hostname --domain' may have unwanted effects on
 +# some platforms, e.g. setting your hostname to --domain ;)
 +fallback_name=$(grep $(id -un) /etc/passwd | cut -d : -f 5 | cut -d , 
 -f 1)
 +fallback_email=$(id -un)@$(hostname).$(hostname --domain)

I'm not sure how portable it is, but maybe dnsdomainname would at least
have better failure modes.

I also wondered about using getent instead of grep.

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


[PATCH v5 0/5] reply to sender

2012-01-14 Thread Jani Nikula
Hi all, hopefully the final round with only the comment and commit
message fixes to issues in patches 1 and 2 spotted by Austin and
Mark. Thanks again for a thorough review!

BR,
Jani.

Jani Nikula (4):
  cli: slightly refactor notmuch reply address scanning functions
  cli: add support for replying just to the sender in notmuch reply
  emacs: add support for replying just to the sender
  emacs: bind 'r' to reply-to-sender and 'R' to reply-to-all

Mark Walters (1):
  test: add tests for notmuch reply --reply-to=sender

 emacs/notmuch-mua.el |9 ++-
 emacs/notmuch-show.el|   12 ++-
 emacs/notmuch.el |   11 ++-
 man/man1/notmuch-reply.1 |   28 +-
 notmuch-reply.c  |  129 ++--
 test/notmuch-test|1 +
 test/reply-to-sender |  209 ++
 7 files changed, 340 insertions(+), 59 deletions(-)
 create mode 100755 test/reply-to-sender

-- 
1.7.5.4

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


[PATCH v5 1/5] cli: slightly refactor notmuch reply address scanning functions

2012-01-14 Thread Jani Nikula
Slightly refactor notmuch reply recipient and user from address scanning
functions in preparation for reply-to-sender feature.

Add support for not adding recipients at all (just scan for user from
address), and returning the number of recipients added.

No externally visible functional changes.

Signed-off-by: Jani Nikula j...@nikula.org
---
 notmuch-reply.c |   76 +--
 1 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 000f6da..a8d6a94 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -168,22 +168,29 @@ address_is_users (const char *address, notmuch_config_t 
*config)
 return 0;
 }
 
-/* For each address in 'list' that is not configured as one of the
- * user's addresses in 'config', add that address to 'message' as an
- * address of 'type'.
+/* Scan addresses in 'list'.
  *
- * The first address encountered that *is* the user's address will be
- * returned, (otherwise NULL is returned).
+ * If 'message' is non-NULL, then for each address in 'list' that is
+ * not configured as one of the user's addresses in 'config', add that
+ * address to 'message' as an address of 'type'.
+ *
+ * If 'user_from' is non-NULL and *user_from is NULL, *user_from will
+ * be set to the first address encountered in 'list' that is the
+ * user's address.
+ *
+ * Return the number of addresses added to 'message'. (If 'message' is
+ * NULL, the function returns 0 by definition.)
  */
-static const char *
-add_recipients_for_address_list (GMimeMessage *message,
-notmuch_config_t *config,
-GMimeRecipientType type,
-InternetAddressList *list)
+static unsigned int
+scan_address_list (InternetAddressList *list,
+  notmuch_config_t *config,
+  GMimeMessage *message,
+  GMimeRecipientType type,
+  const char **user_from)
 {
 InternetAddress *address;
 int i;
-const char *ret = NULL;
+unsigned int n = 0;
 
 for (i = 0; i  internet_address_list_length (list); i++) {
address = internet_address_list_get_address (list, i);
@@ -196,8 +203,7 @@ add_recipients_for_address_list (GMimeMessage *message,
if (group_list == NULL)
continue;
 
-   add_recipients_for_address_list (message, config,
-type, group_list);
+   n += scan_address_list (group_list, config, message, type, NULL);
} else {
InternetAddressMailbox *mailbox;
const char *name;
@@ -209,40 +215,41 @@ add_recipients_for_address_list (GMimeMessage *message,
addr = internet_address_mailbox_get_addr (mailbox);
 
if (address_is_users (addr, config)) {
-   if (ret == NULL)
-   ret = addr;
-   } else {
+   if (user_from  *user_from == NULL)
+   *user_from = addr;
+   } else if (message) {
g_mime_message_add_recipient (message, type, name, addr);
+   n++;
}
}
 }
 
-return ret;
+return n;
 }
 
-/* For each address in 'recipients' that is not configured as one of
- * the user's addresses in 'config', add that address to 'message' as
- * an address of 'type'.
+/* Scan addresses in 'recipients'.
  *
- * The first address encountered that *is* the user's address will be
- * returned, (otherwise NULL is returned).
+ * See the documentation of scan_address_list() above. This function
+ * does exactly the same, but converts 'recipients' to an
+ * InternetAddressList first.
  */
-static const char *
-add_recipients_for_string (GMimeMessage *message,
-  notmuch_config_t *config,
-  GMimeRecipientType type,
-  const char *recipients)
+static unsigned int
+scan_address_string (const char *recipients,
+notmuch_config_t *config,
+GMimeMessage *message,
+GMimeRecipientType type,
+const char **user_from)
 {
 InternetAddressList *list;
 
 if (recipients == NULL)
-   return NULL;
+   return 0;
 
 list = internet_address_list_parse_string (recipients);
 if (list == NULL)
-   return NULL;
+   return 0;
 
-return add_recipients_for_address_list (message, config, type, list);
+return scan_address_list (list, config, message, type, user_from);
 }
 
 /* Does the address in the Reply-To header of 'message' already appear
@@ -324,7 +331,7 @@ add_recipients_from_message (GMimeMessage *reply,
 }
 
 for (i = 0; i  ARRAY_SIZE (reply_to_map); i++) {
-   const char *addr, *recipients;
+   const char *recipients;
 
recipients = notmuch_message_get_header (message,
 reply_to_map[i].header);
@@ 

[PATCH v5 2/5] cli: add support for replying just to the sender in notmuch reply

2012-01-14 Thread Jani Nikula
Add new option --reply-to=(all|sender) to notmuch reply to select whether
to reply to all (sender and all recipients), or just sender. Reply to all
remains the default.

Credits to Mark Walters markwalters1...@gmail.com for his similar earlier
work where I picked up the basic idea of handling reply-to-sender in
add_recipients_from_message(). All bugs are mine, though.

Signed-off-by: Jani Nikula j...@nikula.org

---

Settled on --reply-to=(all|sender) per Carl's earlier suggestion
(id:87pqn5cg4g@yoom.home.cworth.org) and David's approval on IRC.
---
 man/man1/notmuch-reply.1 |   28 ++
 notmuch-reply.c  |   57 -
 2 files changed, 68 insertions(+), 17 deletions(-)

diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index db464d8..5160ece 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -14,11 +14,13 @@ Constructs a reply template for a set of messages.
 To make replying to email easier,
 .B notmuch reply
 takes an existing set of messages and constructs a suitable mail
-template. The Reply-to header (if any, otherwise From:) is used for
-the To: address. Vales from the To: and Cc: headers are copied, but
-not including any of the current user's email addresses (as configured
-in primary_mail or other_email in the .notmuch\-config file) in the
-recipient list
+template. The Reply-to: header (if any, otherwise From:) is used for
+the To: address. Unless
+.BR \-\-reply-to=sender
+is specified, values from the To: and Cc: headers are copied, but not
+including any of the current user's email addresses (as configured in
+primary_mail or other_email in the .notmuch\-config file) in the
+recipient list.
 
 It also builds a suitable new subject, including Re: at the front (if
 not already present), and adding the message IDs of the messages being
@@ -45,6 +47,22 @@ Includes subject and quoted message body.
 Only produces In\-Reply\-To, References, To, Cc, and Bcc headers.
 .RE
 .RE
+.RS
+.TP 4
+.BR \-\-reply\-to= ( all | sender )
+.RS
+.TP 4
+.BR all  (default)
+Replies to all addresses.
+.TP 4
+.BR sender
+Replies only to the sender. If replying to user's own message
+(Reply-to: or From: header is one of the user's configured email
+addresses), try To:, Cc:, and Bcc: headers in this order, and copy
+values from the first that contains something other than only the
+user's addresses.
+.RE
+.RE
 
 See \fBnotmuch-search-terms\fR(7)
 for details of the supported syntax for search-terms.
diff --git a/notmuch-reply.c b/notmuch-reply.c
index a8d6a94..da3acce 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -291,15 +291,23 @@ reply_to_header_is_redundant (notmuch_message_t *message)
 return 0;
 }
 
-/* Augments the recipients of reply from the headers of message.
+/* Augment the recipients of 'reply' from the Reply-to:, From:,
+ * To:, Cc:, and Bcc: headers of 'message'.
  *
- * If any of the user's addresses were found in these headers, the first
- * of these returned, otherwise NULL is returned.
+ * If 'reply_all' is true, use sender and all recipients, otherwise
+ * scan the headers for the first that contains something other than
+ * the user's addresses and add the recipients from this header
+ * (typically this would be reply-to-sender, but also handles reply to
+ * user's own message in a sensible way).
+ *
+ * If any of the user's addresses were found in these headers, the
+ * first of these returned, otherwise NULL is returned.
  */
 static const char *
 add_recipients_from_message (GMimeMessage *reply,
 notmuch_config_t *config,
-notmuch_message_t *message)
+notmuch_message_t *message,
+notmuch_bool_t reply_all)
 {
 struct {
const char *header;
@@ -313,6 +321,7 @@ add_recipients_from_message (GMimeMessage *reply,
 };
 const char *from_addr = NULL;
 unsigned int i;
+unsigned int n = 0;
 
 /* Some mailing lists munge the Reply-To header despite it being A Bad
  * Thing, see http://www.unicom.com/pw/reply-to-harmful.html
@@ -339,8 +348,24 @@ add_recipients_from_message (GMimeMessage *reply,
recipients = notmuch_message_get_header (message,
 reply_to_map[i].fallback);
 
-   scan_address_string (recipients, config, reply,
-reply_to_map[i].recipient_type, from_addr);
+   n += scan_address_string (recipients, config, reply,
+ reply_to_map[i].recipient_type, from_addr);
+
+   if (!reply_all  n) {
+   /* Stop adding new recipients in reply-to-sender mode if
+* we have added some recipient(s) above.
+*
+* This also handles the case of user replying to his own
+* message, where reply-to/from is not a recipient. In
+* this case there may be more than one 

[PATCH v5 3/5] emacs: add support for replying just to the sender

2012-01-14 Thread Jani Nikula
Provide reply to sender counterparts to the search and show reply
functions. Add key binding 'R' to reply to sender, while keeping 'r' as
reply to all, both in search and show views.

Signed-off-by: Jani Nikula j...@nikula.org
---
 emacs/notmuch-mua.el  |9 ++---
 emacs/notmuch-show.el |   10 --
 emacs/notmuch.el  |9 -
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 32e2e30..d8ab822 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -71,12 +71,15 @@ list.
(push header message-hidden-headers)))
notmuch-mua-hidden-headers))
 
-(defun notmuch-mua-reply (query-string optional sender)
+(defun notmuch-mua-reply (query-string optional sender reply-all)
   (let (headers
body
(args '(reply)))
 (if notmuch-show-process-crypto
(setq args (append args '(--decrypt
+(if reply-all
+   (setq args (append args '(--reply-to=all)))
+  (setq args (append args '(--reply-to=sender
 (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
@@ -218,13 +221,13 @@ the From: address first.
(notmuch-mua-forward-message))
 (notmuch-mua-forward-message)))
 
-(defun notmuch-mua-new-reply (query-string optional prompt-for-sender)
+(defun notmuch-mua-new-reply (query-string optional prompt-for-sender 
reply-all)
   Invoke the notmuch reply window.
   (interactive P)
   (let ((sender
 (when prompt-for-sender
   (notmuch-mua-prompt-for-sender
-(notmuch-mua-reply query-string sender)))
+(notmuch-mua-reply query-string sender reply-all)))
 
 (defun notmuch-mua-send-and-exit (optional arg)
   (interactive P)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 034db87..9031b82 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -934,6 +934,7 @@ thread id.  If a prefix is given, crypto processing is 
toggled.
(define-key map m 'notmuch-mua-new-mail)
(define-key map f 'notmuch-show-forward-message)
(define-key map r 'notmuch-show-reply)
+   (define-key map R 'notmuch-show-reply-sender)
(define-key map | 'notmuch-show-pipe-message)
(define-key map w 'notmuch-show-save-attachments)
(define-key map V 'notmuch-show-view-raw-message)
@@ -1238,9 +1239,14 @@ any effects from previous calls to
   (notmuch-show-previous-message)
 
 (defun notmuch-show-reply (optional prompt-for-sender)
-  Reply to the current message.
+  Reply to the sender and all recipients of the current message.
   (interactive P)
-  (notmuch-mua-new-reply (notmuch-show-get-message-id) prompt-for-sender))
+  (notmuch-mua-new-reply (notmuch-show-get-message-id) prompt-for-sender t))
+
+(defun notmuch-show-reply-sender (optional prompt-for-sender)
+  Reply to the sender of the current message.
+  (interactive P)
+  (notmuch-mua-new-reply (notmuch-show-get-message-id) prompt-for-sender nil))
 
 (defun notmuch-show-forward-message (optional prompt-for-sender)
   Forward the current message.
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 1e61775..9ac2888 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -214,6 +214,7 @@ For a mouse binding, return nil.
 (define-key map p 'notmuch-search-previous-thread)
 (define-key map n 'notmuch-search-next-thread)
 (define-key map r 'notmuch-search-reply-to-thread)
+(define-key map R 'notmuch-search-reply-to-thread-sender)
 (define-key map m 'notmuch-mua-new-mail)
 (define-key map s 'notmuch-search)
 (define-key map o 'notmuch-search-toggle-order)
@@ -448,10 +449,16 @@ Complete list of currently available key bindings:
   (message End of search results.
 
 (defun notmuch-search-reply-to-thread (optional prompt-for-sender)
+  Begin composing a reply-all to the entire current thread in a new buffer.
+  (interactive P)
+  (let ((message-id (notmuch-search-find-thread-id)))
+(notmuch-mua-new-reply message-id prompt-for-sender t)))
+
+(defun notmuch-search-reply-to-thread-sender (optional prompt-for-sender)
   Begin composing a reply to the entire current thread in a new buffer.
   (interactive P)
   (let ((message-id (notmuch-search-find-thread-id)))
-(notmuch-mua-new-reply message-id prompt-for-sender)))
+(notmuch-mua-new-reply message-id prompt-for-sender nil)))
 
 (defun notmuch-call-notmuch-process (rest args)
   Synchronously invoke \notmuch\ with the given list of arguments.
-- 
1.7.5.4

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


[PATCH v5 4/5] emacs: bind 'r' to reply-to-sender and 'R' to reply-to-all

2012-01-14 Thread Jani Nikula
Change the default reply key bindings, making 'r' reply-to-sender and 'R'
reply-to-all.

Signed-off-by: Jani Nikula j...@nikula.org

---

There were mixed feelings about this. This as a separate patch so it's easy
to drop if needed.
---
 emacs/notmuch-show.el |4 ++--
 emacs/notmuch.el  |4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 9031b82..03c1f6b 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -933,8 +933,8 @@ thread id.  If a prefix is given, crypto processing is 
toggled.
(define-key map s 'notmuch-search)
(define-key map m 'notmuch-mua-new-mail)
(define-key map f 'notmuch-show-forward-message)
-   (define-key map r 'notmuch-show-reply)
-   (define-key map R 'notmuch-show-reply-sender)
+   (define-key map r 'notmuch-show-reply-sender)
+   (define-key map R 'notmuch-show-reply)
(define-key map | 'notmuch-show-pipe-message)
(define-key map w 'notmuch-show-save-attachments)
(define-key map V 'notmuch-show-view-raw-message)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 9ac2888..d952c41 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -213,8 +213,8 @@ For a mouse binding, return nil.
 (define-key map  'notmuch-search-last-thread)
 (define-key map p 'notmuch-search-previous-thread)
 (define-key map n 'notmuch-search-next-thread)
-(define-key map r 'notmuch-search-reply-to-thread)
-(define-key map R 'notmuch-search-reply-to-thread-sender)
+(define-key map r 'notmuch-search-reply-to-thread-sender)
+(define-key map R 'notmuch-search-reply-to-thread)
 (define-key map m 'notmuch-mua-new-mail)
 (define-key map s 'notmuch-search)
 (define-key map o 'notmuch-search-toggle-order)
-- 
1.7.5.4

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


[PATCH v5 5/5] test: add tests for notmuch reply --reply-to=sender

2012-01-14 Thread Jani Nikula
From: Mark Walters markwalters1...@gmail.com

---
 test/notmuch-test|1 +
 test/reply-to-sender |  209 ++
 2 files changed, 210 insertions(+), 0 deletions(-)
 create mode 100755 test/reply-to-sender

diff --git a/test/notmuch-test b/test/notmuch-test
index e40ef86..6a99ae3 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -33,6 +33,7 @@ TESTS=
   thread-naming
   raw
   reply
+  reply-to-sender
   dump-restore
   uuencode
   thread-order
diff --git a/test/reply-to-sender b/test/reply-to-sender
new file mode 100755
index 000..c7d15bb
--- /dev/null
+++ b/test/reply-to-sender
@@ -0,0 +1,209 @@
+#!/usr/bin/env bash
+test_description=\notmuch reply --reply-to=sender\ in several variations
+. ./test-lib.sh
+
+test_begin_subtest Basic reply-to-sender
+add_message '[from]=Sender sen...@example.com' \
+ [to]=test_su...@notmuchmail.org \
+ [subject]=notmuch-reply-test \
+'[date]=Tue, 05 Jan 2010 15:43:56 -' \
+'[body]=basic reply-to-sender test'
+
+output=$(notmuch reply --reply-to=sender id:${gen_msg_id})
+test_expect_equal $output From: Notmuch Test Suite 
test_su...@notmuchmail.org
+Subject: Re: notmuch-reply-test
+To: Sender sen...@example.com
+In-Reply-To: ${gen_msg_id}
+References: ${gen_msg_id}
+
+On Tue, 05 Jan 2010 15:43:56 -, Sender sen...@example.com wrote:
+ basic reply-to-sender test
+
+test_begin_subtest From Us, Basic reply to message
+add_message '[from]=Notmuch Test Suite test_su...@notmuchmail.org' \
+'[to]=Recipient recipi...@example.com' \
+ [subject]=notmuch-reply-test \
+'[date]=Tue, 05 Jan 2010 15:43:56 -' \
+'[body]=basic reply-to-from-us test'
+
+output=$(notmuch reply --reply-to=sender id:${gen_msg_id})
+test_expect_equal $output From: Notmuch Test Suite 
test_su...@notmuchmail.org
+Subject: Re: notmuch-reply-test
+To: Recipient recipi...@example.com
+In-Reply-To: ${gen_msg_id}
+References: ${gen_msg_id}
+
+On Tue, 05 Jan 2010 15:43:56 -, Notmuch Test Suite 
test_su...@notmuchmail.org wrote:
+ basic reply-to-from-us test
+
+test_begin_subtest Multiple recipients
+add_message '[from]=Sender sen...@example.com' \
+'[to]=test_su...@notmuchmail.org, Someone Else 
some...@example.com' \
+ [subject]=notmuch-reply-test \
+'[date]=Tue, 05 Jan 2010 15:43:56 -' \
+'[body]=Multiple recipients'
+
+output=$(notmuch reply  --reply-to=sender  id:${gen_msg_id})
+test_expect_equal $output From: Notmuch Test Suite 
test_su...@notmuchmail.org
+Subject: Re: notmuch-reply-test
+To: Sender sen...@example.com
+In-Reply-To: ${gen_msg_id}
+References: ${gen_msg_id}
+
+On Tue, 05 Jan 2010 15:43:56 -, Sender sen...@example.com wrote:
+ Multiple recipients
+
+test_begin_subtest From Us, Multiple TO recipients
+add_message '[from]=Notmuch Test Suite test_su...@notmuchmail.org' \
+'[to]=Recipient recipi...@example.com, Someone Else 
some...@example.com' \
+ [subject]=notmuch-reply-test \
+'[date]=Tue, 05 Jan 2010 15:43:56 -' \
+'[body]=From Us, Multiple TO recipients'
+
+output=$(notmuch reply  --reply-to=sender  id:${gen_msg_id})
+test_expect_equal $output From: Notmuch Test Suite 
test_su...@notmuchmail.org
+Subject: Re: notmuch-reply-test
+To: Recipient recipi...@example.com, Someone Else some...@example.com
+In-Reply-To: ${gen_msg_id}
+References: ${gen_msg_id}
+
+On Tue, 05 Jan 2010 15:43:56 -, Notmuch Test Suite 
test_su...@notmuchmail.org wrote:
+ From Us, Multiple TO recipients
+
+test_begin_subtest Reply with CC
+add_message '[from]=Sender sen...@example.com' \
+ [to]=test_su...@notmuchmail.org \
+'[cc]=Other Parties c...@example.com' \
+ [subject]=notmuch-reply-test \
+'[date]=Tue, 05 Jan 2010 15:43:56 -' \
+'[body]=reply with CC'
+
+output=$(notmuch reply  --reply-to=sender id:${gen_msg_id})
+test_expect_equal $output From: Notmuch Test Suite 
test_su...@notmuchmail.org
+Subject: Re: notmuch-reply-test
+To: Sender sen...@example.com
+In-Reply-To: ${gen_msg_id}
+References: ${gen_msg_id}
+
+On Tue, 05 Jan 2010 15:43:56 -, Sender sen...@example.com wrote:
+ reply with CC
+
+test_begin_subtest From Us, Reply with CC
+add_message '[from]=Notmuch Test Suite test_su...@notmuchmail.org' \
+'[to]=Recipient recipi...@example.com' \
+'[cc]=Other Parties c...@example.com' \
+ [subject]=notmuch-reply-test \
+'[date]=Tue, 05 Jan 2010 15:43:56 -' \
+'[body]=reply with CC'
+
+output=$(notmuch reply  --reply-to=sender id:${gen_msg_id})
+test_expect_equal $output From: Notmuch Test Suite 
test_su...@notmuchmail.org
+Subject: Re: notmuch-reply-test
+To: Recipient recipi...@example.com
+In-Reply-To: ${gen_msg_id}
+References: ${gen_msg_id}
+
+On Tue, 05 Jan 2010 15:43:56 -, Notmuch Test Suite 

Re: [PATCH v5 1/5] cli: slightly refactor notmuch reply address scanning functions

2012-01-14 Thread David Bremner
On Sat, 14 Jan 2012 16:46:15 +0200, Jani Nikula j...@nikula.org wrote:
 Slightly refactor notmuch reply recipient and user from address scanning
 functions in preparation for reply-to-sender feature.
 

Pushed, bindings change and all.

This series definitely needs a NEWS item. 

Perhaps some kind soul could add a wiki entry explaining to people how
to swap the bindings, just in case there people who don't like the
one-true-reply-bindings (cough).

d



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


[PATCH] NEWS: add news items for reply to sender

2012-01-14 Thread Jani Nikula
---
 NEWS |   23 +++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index bf21e64..1161c22 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,26 @@
+Notmuch 0.12 (2012-xx-xx)
+=
+
+Command-Line Interface
+--
+
+Reply to sender
+
+  notmuch reply has gained the ability to create a reply template
+  for replying just to the sender of the message, in addition to reply
+  to all. The feature is available through the new command line option
+  --reply-to=(all|sender).
+
+Emacs Interface
+---
+
+Reply to sender
+
+  The Emacs interface has, with the new CLI support, gained the
+  ability to reply to sender in addition to reply to all. In both show
+  and search modes, 'r' has been bound to reply to sender, replacing
+  reply to all, which now has key binding 'R'.
+
 Notmuch 0.11 (2012-01-13)
 =
 
-- 
1.7.5.4

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


Re: [PATCH v5 1/5] cli: slightly refactor notmuch reply address scanning functions

2012-01-14 Thread Jani Nikula

For those not on IRC:

On Sat, 14 Jan 2012 11:31:16 -0400, David Bremner da...@tethera.net wrote:
 This series definitely needs a NEWS item. 

id:1326559168-29178-1-git-send-email-j...@nikula.org

 Perhaps some kind soul could add a wiki entry explaining to people how
 to swap the bindings, just in case there people who don't like the
 one-true-reply-bindings (cough).

http://notmuchmail.org/emacstips/#index7h2


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


Re: [PATCH] emacs: logically group def{custom,face}s

2012-01-14 Thread Austin Clements
Quoth Pieter Praet on Jan 14 at 10:04 am:
 To allow for expansion whilst keeping everything tidy and organized,
 move all defcustom/defface variables to the following subgroups,
 defined in notmuch-lib.el:
 
 - Hello
 - Search
 - Show
 - Send
 - Crypto
 - Hooks
 - External Commands
 - Appearance
 
 As an added benefit, defcustom keyword args are now consistently
 in order of appearance @ defcustom's docstring (OCD much?).

Thanks for doing this.  I recently went into customize-group notmuch
and was overwhelmed by the pile of options presented.

 diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
 index 0f856bf..f6f48e9 100644
 --- a/emacs/notmuch-lib.el
 +++ b/emacs/notmuch-lib.el
 @@ -28,17 +28,58 @@
Notmuch mail reader for Emacs.
:group 'mail)
  

Group docstrings aren't generally of the form Options concerning
...; they just jump into what they concern.  E.g.,

Convenience : Convenience features for faster editing.

Calendar Hooks : Calendar hooks.

Also, all but one of the tags you give the groups would be
automatically derived by Emacs, so you can remove those.

 +(defgroup notmuch-hello nil
 +  Options concerning `notmuch-hello-mode'.
 +  :tag Notmuch Hello
 +  :group 'notmuch)

Perhaps Overview of saved searches, tags, etc.

 +
 +(defgroup notmuch-search nil
 +  Options concerning `notmuch-search-mode'.
 +  :tag Notmuch Search
 +  :group 'notmuch)

Searching and sorting mail?

 +
 +(defgroup notmuch-show nil
 +  Options concerning `notmuch-show-mode'.
 +  :tag Notmuch Show
 +  :group 'notmuch)

Showing messages and threads?

 +
 +(defgroup notmuch-send nil
 +  Options concerning the sending of messages.
 +  :tag Notmuch Send
 +  :group 'notmuch)

Sending messages from Notmuch?

We should probably link to the 'message group, perhaps by adding
  :link '(custom-group-link message)
here or maybe to the notmuch group itself.  Unfortunately, I don't
think you can actually add a group to another group after it's been
defined (though I could be wrong).

 +
 +(defgroup notmuch-crypto nil
 +  Options concerning the processing and fontification of
 +cryptographic MIME parts in `notmuch-show-mode'.
 +  :tag Notmuch Crypto
 +  :group 'notmuch)

Processing and display of cryptographic MIME parts?  (You also don't
want the docstring to be too long, given how it's displayed.)

 +
 +(defgroup notmuch-hooks nil
 +  Run custom code on well-defined occasions.
 +  :tag Notmuch Hooks
 +  :group 'notmuch)
 +
 +(defgroup notmuch-external nil
 +  Run more custom code on different well-defined occasions.
 +  :tag Notmuch External Commands
 +  :group 'notmuch)

Oof!  It's okay to be a little redundant in the docstring.  Core Emacs
options do it.  External commands?

 +
 +(defgroup notmuch-appearance nil
 +  Options concerning how Notmuch looks.
 +  :tag Notmuch Appearance
 +  :group 'notmuch)

How Notmuch looks?

I worry that notmuch-appearance is a catch-all that most options
arguably fit in to.  In particular, some notmuch-show options are also
in this group and some aren't and it's not clear to me what the rule
is.

Perhaps this should be notmuch-faces and limited to just faces (and
maybe options that aren't technically faces but that affect face
selection)?  Then the grouping rule would be obvious, like it is for
all of the other groups.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] emacs: logically group def{custom,face}s

2012-01-14 Thread Jameson Graef Rollins
I like all of Austin's description suggestions.

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


Re: [PATCH 2/4] reply: Add a JSON reply format.

2012-01-14 Thread Jani Nikula
On Sun,  8 Jan 2012 00:52:40 -0700, Adam Wolfe Gordon awg+notm...@xvx.ca 
wrote:
 From: Adam Wolfe Gordon a...@xvx.ca
 
 This new JSON format for replies includes headers generated for a reply
 message as well as the headers and all text parts of the original message.
 Using this data, a client can intelligently create a reply. For example,
 the emacs client will be able to create replies with quoted HTML parts by
 parsing the HTML parts using w3m.

Hi Adam, this is a drive-by-review on some things I spotted, but can't
say I would've thought the whole thing through. I'm pretty ignorant
about MIME parts etc. Please find comments inline.

The reply-to-sender set was just merged. You'll have conflicts both here
and in emacs code, but they should be trivial to sort out.


BR,
Jani.


 ---
  notmuch-reply.c |  269 
 +++
  1 files changed, 230 insertions(+), 39 deletions(-)
 
 diff --git a/notmuch-reply.c b/notmuch-reply.c
 index f8d5f64..82df396 100644
 --- a/notmuch-reply.c
 +++ b/notmuch-reply.c
 @@ -30,6 +30,15 @@ reply_headers_message_part (GMimeMessage *message);
  static void
  reply_part_content (GMimeObject *part);
  
 +static void
 +reply_part_start_json (GMimeObject *part, int *part_count);
 +
 +static void
 +reply_part_content_json (GMimeObject *part);
 +
 +static void
 +reply_part_end_json (GMimeObject *part);
 +

I know there are existing forward declarations like this, but would it
be much trouble to arrange the code so that they were not needed at all?

  static const notmuch_show_format_t format_reply = {
  ,
   , NULL,
 @@ -46,6 +55,22 @@ static const notmuch_show_format_t format_reply = {
  
  };
  
 +static const notmuch_show_format_t format_json = {
 +,
 + , NULL,
 + , NULL, NULL, ,
 + ,
 + reply_part_start_json,
 + NULL,
 + NULL,
 + reply_part_content_json,
 + reply_part_end_json,
 + ,
 + ,
 + , ,
 +
 +};
 +
  static void
  show_reply_headers (GMimeMessage *message)
  {
 @@ -147,6 +172,78 @@ reply_part_content (GMimeObject *part)
  }
  }
  
 +static void
 +reply_part_start_json (GMimeObject *part, unused(int *part_count))
 +{
 +GMimeContentType *content_type = g_mime_object_get_content_type 
 (GMIME_OBJECT (part));
 +GMimeContentDisposition *disposition = 
 g_mime_object_get_content_disposition (part);
 +
 +if (g_mime_content_type_is_type (content_type, text, *) 
 + (!disposition ||
 +  strcmp (disposition-disposition, GMIME_DISPOSITION_INLINE) == 0))
 +{
 + printf({ );
 +}
 +}
 +
 +static void
 +reply_part_end_json (GMimeObject *part)
 +{
 +GMimeContentType *content_type = g_mime_object_get_content_type 
 (GMIME_OBJECT (part));
 +GMimeContentDisposition *disposition = 
 g_mime_object_get_content_disposition (part);
 +
 +if (g_mime_content_type_is_type (content_type, text, *) 
 + (!disposition ||
 +  strcmp (disposition-disposition, GMIME_DISPOSITION_INLINE) == 0))
 + printf (}, );
 +}

The two functions above, while small, are almost identical. Please move
the common stuff to a common helper, and you can have something like
this:

if (the_common_function (part))
printf (}, );

 +
 +static void
 +reply_part_content_json (GMimeObject *part)
 +{
 +GMimeContentType *content_type = g_mime_object_get_content_type 
 (GMIME_OBJECT (part));
 +GMimeContentDisposition *disposition = 
 g_mime_object_get_content_disposition (part);
 +
 +void *ctx = talloc_new (NULL);
 +
 +/* We only care about inline text parts for reply purposes */
 +if (g_mime_content_type_is_type (content_type, text, *) 
 + (!disposition ||
 +  strcmp (disposition-disposition, GMIME_DISPOSITION_INLINE) == 0))

Oh, you can use the common helper here too.

 +{
 + GMimeStream *stream_memory = NULL, *stream_filter = NULL;

No need to initialize stream_memory here.

 + GMimeDataWrapper *wrapper;
 + GByteArray *part_content;
 + const char *charset;
 +
 + printf(\content-type\: %s, \content\: ,
 +json_quote_str(ctx, 
 g_mime_content_type_to_string(content_type)));

The style in notmuch is to have a space before the opening brace in
function calls. Check elsewhere also. I always forget that too. :)

 +
 + charset = g_mime_object_get_content_type_parameter (part, charset);

AFAICT charset declaration and the above call could be moved inside if
(stream_memory) below.

 + stream_memory = g_mime_stream_mem_new ();
 + if (stream_memory) {
 + stream_filter = g_mime_stream_filter_new(stream_memory);
 + if (charset) {
 + g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
 +  g_mime_filter_charset_new(charset, 
 UTF-8));
 + }
 + }
 + wrapper = g_mime_part_get_content_object (GMIME_PART (part));
 + if (wrapper  stream_filter)
 

[PATCH 1/2] test: add known broken test for reply from address in named group list

2012-01-14 Thread Jani Nikula
If a message was received to the user's address that was in a named
group list, notmuch reply does not use that address for picking the
from address.

Groups lists are of the form: foo:b...@example.com,b...@example.com;

Signed-off-by: Jani Nikula j...@nikula.org
---
 test/reply |   19 +++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/test/reply b/test/reply
index c0b8e26..196535a 100755
--- a/test/reply
+++ b/test/reply
@@ -72,6 +72,25 @@ References: ${gen_msg_id}
 On Tue, 05 Jan 2010 15:43:56 -, Sender sen...@example.com wrote:
  reply from alternate address
 
+test_begin_subtest Reply from address in named group list
+test_subtest_known_broken
+add_message '[from]=Sender sen...@example.com' \
+'[to]=group:test_su...@notmuchmail.org,some...@example.com\;' \
+ [cc]=test_suite_ot...@notmuchmail.org \
+ [subject]=notmuch-reply-test \
+'[date]=Tue, 05 Jan 2010 15:43:56 -' \
+'[body]=Reply from address in named group list'
+
+output=$(notmuch reply id:${gen_msg_id})
+test_expect_equal $output From: Notmuch Test Suite 
test_su...@notmuchmail.org
+Subject: Re: notmuch-reply-test
+To: Sender sen...@example.com, some...@example.com
+In-Reply-To: ${gen_msg_id}
+References: ${gen_msg_id}
+
+On Tue, 05 Jan 2010 15:43:56 -, Sender sen...@example.com wrote:
+ Reply from address in named group list
+
 test_begin_subtest Support for Reply-To
 add_message '[from]=Sender sen...@example.com' \
  [to]=test_su...@notmuchmail.org \
-- 
1.7.5.4

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


[PATCH 2/2] cli: pick the user's address in a group list as from address

2012-01-14 Thread Jani Nikula
Messages received to a group list were not replied to using the from
address in the list. Fix it.

Signed-off-by: Jani Nikula j...@nikula.org
---
 notmuch-reply.c |2 +-
 test/reply  |1 -
 2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index da3acce..0f682db 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -203,7 +203,7 @@ scan_address_list (InternetAddressList *list,
if (group_list == NULL)
continue;
 
-   n += scan_address_list (group_list, config, message, type, NULL);
+   n += scan_address_list (group_list, config, message, type, 
user_from);
} else {
InternetAddressMailbox *mailbox;
const char *name;
diff --git a/test/reply b/test/reply
index 196535a..e4e16eb 100755
--- a/test/reply
+++ b/test/reply
@@ -73,7 +73,6 @@ On Tue, 05 Jan 2010 15:43:56 -, Sender 
sen...@example.com wrote:
  reply from alternate address
 
 test_begin_subtest Reply from address in named group list
-test_subtest_known_broken
 add_message '[from]=Sender sen...@example.com' \
 '[to]=group:test_su...@notmuchmail.org,some...@example.com\;' \
  [cc]=test_suite_ot...@notmuchmail.org \
-- 
1.7.5.4

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


Re: [PATCH 2/2] cli: pick the user's address in a group list as from address

2012-01-14 Thread Austin Clements
Quoth Jani Nikula on Jan 14 at 11:49 pm:
 Messages received to a group list were not replied to using the from
 address in the list. Fix it.
 
 Signed-off-by: Jani Nikula j...@nikula.org

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


Re: [PATCH v2 2/3] lib: Add support for automatically excluding tags from queries

2012-01-14 Thread Jameson Graef Rollins
It looks like something in this patch is causing the following build
warning:

CXX -O2 lib/query.o
lib/query.cc:26:8: warning: ‘_notmuch_query’ declared with greater visibility 
than the type of its field
‘_notmuch_query::exclude_terms’ [-Wattributes]

However, I can't quite figure out what's causing it.

 +void
 +notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
 +{
 +char *term = talloc_asprintf (query, %s%s, _find_prefix (tag), tag);
 +_notmuch_string_list_append (query-exclude_terms, term);
 +}

This is really not an issue with this patch at all, and it should NOT
prevent it from being applied, but this came up briefly on IRC and I'm
curious, so I'll ask about it here.

Are terms ALWAYS lower cased?  If not, it seems to me it's possible to
have an indexed term 'Kspam' that would get confused with the term
'spam' prefixed with the keyword prefix 'K' (which we use for tags).
Maybe this degeneracy is broken by the query parser somehow (or maybe by
the fact that tags are boolean terms?), but I wonder if it's not safer
to use the built-in xapian prefix separator ':', ie:

  ... talloc_asprintf (query, %s:%s, _find_prefix (tag), tag);

I guess fixing that globally would require a database rebuild...

Ok, that's totally just an aside, and should not be a blocker for this
patch.

jamie.


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


Re: [PATCH v2 3/3] search: Support automatic tag exclusions

2012-01-14 Thread Jameson Graef Rollins
This patch looks fine.  Philosophical UI discussion to follow:

On Fri, 13 Jan 2012 18:07:04 -0500, Austin Clements amdra...@mit.edu wrote:
 +if (notmuch_config_get_auto_exclude_tags (config, tmp) == NULL) {
 + const char *tags[] = { deleted, spam };
 + notmuch_config_set_auto_exclude_tags (config, tags, 2);
 +}

This creates the config section with the exclude list pre-set to
deleted;spam.  I personally have no problem with this, since I was
going to be setting exactly that anyway.  However, assuming we decide to
have this be the default in the CLI, should we therefore add support for
it in the emacs UI?  I've been going back and forth on this (as readers
are well aware), and have most recently rejected the idea that we should
add delete support to the emacs UI.  However, if we are excluding
deleted tags by default, then I'm going to go back and say that we
should include the keybindings to delete messages.  Comments?

If people think we should exclude deleted;spam by default, and agree
that we should also add delete support in the emacs UI, I'll go ahead
and rework my keybinding patches.

jamie.


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


Re: [PATCH v2 2/3] lib: Add support for automatically excluding tags from queries

2012-01-14 Thread Austin Clements
Quoth Jameson Graef Rollins on Jan 14 at  3:38 pm:
 It looks like something in this patch is causing the following build
 warning:
 
 CXX -O2 lib/query.o
 lib/query.cc:26:8: warning: ‘_notmuch_query’ declared with greater visibility 
 than the type of its field
 ‘_notmuch_query::exclude_terms’ [-Wattributes]
 
 However, I can't quite figure out what's causing it.

The problem is that notmuch_query_t is a visible symbol because the
type is declared (though not defined) in lib/notmuch.h.  The actual
definition is tucked away in lib/query.cc, but GCC doesn't seem to
care.  I added a field of type notmuch_string_list_t to the struct's
definition, but notmuch_string_list_t is declared between the hidden
pragmas in lib/notmuch-private.h because the type is private to the
library.  This field with a hidden type in a visible type is what GCC
is complaining about.

I'm rather confused by the whole type visibility thing since type
symbols aren't linkable anyway.  However, while puzzling over how you
could possibly use hidden types if they can only be used in other
hidden types, I discovered that Carl had solved this exact problem
last May in d5523ead by adding a visibility(default) attribute to
the offending hidden type.

  +void
  +notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
  +{
  +char *term = talloc_asprintf (query, %s%s, _find_prefix (tag), 
  tag);
  +_notmuch_string_list_append (query-exclude_terms, term);
  +}
 
 This is really not an issue with this patch at all, and it should NOT
 prevent it from being applied, but this came up briefly on IRC and I'm
 curious, so I'll ask about it here.
 
 Are terms ALWAYS lower cased?  If not, it seems to me it's possible to
 have an indexed term 'Kspam' that would get confused with the term
 'spam' prefixed with the keyword prefix 'K' (which we use for tags).
 Maybe this degeneracy is broken by the query parser somehow (or maybe by
 the fact that tags are boolean terms?), but I wonder if it's not safer
 to use the built-in xapian prefix separator ':', ie:
 
   ... talloc_asprintf (query, %s:%s, _find_prefix (tag), tag);
 
 I guess fixing that globally would require a database rebuild...

We discussed this on IRC, but to summarize for the list, the tag
prefix is a single character, so Xapian's ':' rule doesn't apply.
There are several places where we *do* get this wrong and use a
multi-character term prefix with a term that may start with a capital
letter but they're all terms you can't search anyway and, unless I'm
mistaken, we're completely consistent about where we violate or do not
violate the ':' rule.

 Ok, that's totally just an aside, and should not be a blocker for this
 patch.
 
 jamie.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH v2 3/3] search: Support automatic tag exclusions

2012-01-14 Thread Austin Clements
Quoth Jameson Graef Rollins on Jan 14 at  3:40 pm:
 This patch looks fine.  Philosophical UI discussion to follow:
 
 On Fri, 13 Jan 2012 18:07:04 -0500, Austin Clements amdra...@mit.edu wrote:
  +if (notmuch_config_get_auto_exclude_tags (config, tmp) == NULL) {
  +   const char *tags[] = { deleted, spam };
  +   notmuch_config_set_auto_exclude_tags (config, tags, 2);
  +}
 
 This creates the config section with the exclude list pre-set to
 deleted;spam.  I personally have no problem with this, since I was
 going to be setting exactly that anyway.  However, assuming we decide to
 have this be the default in the CLI, should we therefore add support for
 it in the emacs UI?  I've been going back and forth on this (as readers
 are well aware), and have most recently rejected the idea that we should
 add delete support to the emacs UI.  However, if we are excluding
 deleted tags by default, then I'm going to go back and say that we
 should include the keybindings to delete messages.  Comments?

It's not that Emacs doesn't support the deleted tag.  You can always
+deletedRET, and this even seems like a pretty natural thing to do.
To me, the question is whether there should be a shortcut to do this.
I'm probably not one to answer this, since I don't plan to use the
deleted tag and hence using this binding would only ever be an
accident (though I will use the spam tag and I don't think I need a
binding for that; perhaps I would feel differently if my spam filters
were less effective).

 If people think we should exclude deleted;spam by default, and agree
 that we should also add delete support in the emacs UI, I'll go ahead
 and rework my keybinding patches.
 
 jamie.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH v3 0/2] Automatic tag-based exclusion

2012-01-14 Thread Austin Clements
This fixes the symbol visibility warning Jamie pointed out.

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


[PATCH v3 1/2] lib: Add support for automatically excluding tags from queries

2012-01-14 Thread Austin Clements
This is useful for tags like deleted and spam that people
generally want to exclude from query results.  These exclusions will
be overridden if a tag is explicitly mentioned in a query.
---
 lib/notmuch-private.h |2 +-
 lib/notmuch.h |6 ++
 lib/query.cc  |   35 +++
 3 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 60a932f..7bf153e 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -458,7 +458,7 @@ typedef struct _notmuch_string_node {
 struct _notmuch_string_node *next;
 } notmuch_string_node_t;
 
-typedef struct _notmuch_string_list {
+typedef struct visible _notmuch_string_list {
 int length;
 notmuch_string_node_t *head;
 notmuch_string_node_t **tail;
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 9f23a10..7929fe7 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -457,6 +457,12 @@ notmuch_query_set_sort (notmuch_query_t *query, 
notmuch_sort_t sort);
 notmuch_sort_t
 notmuch_query_get_sort (notmuch_query_t *query);
 
+/* Add a tag that will be excluded from the query results by default.
+ * This exclusion will be overridden if this tag appears explicitly in
+ * the query. */
+void
+notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag);
+
 /* Execute a query for threads, returning a notmuch_threads_t object
  * which can be used to iterate over the results. The returned threads
  * object is owned by the query and as such, will only be valid until
diff --git a/lib/query.cc b/lib/query.cc
index b6c0f12..0b36602 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -27,6 +27,7 @@ struct _notmuch_query {
 notmuch_database_t *notmuch;
 const char *query_string;
 notmuch_sort_t sort;
+notmuch_string_list_t *exclude_terms;
 };
 
 typedef struct _notmuch_mset_messages {
@@ -76,6 +77,8 @@ notmuch_query_create (notmuch_database_t *notmuch,
 
 query-sort = NOTMUCH_SORT_NEWEST_FIRST;
 
+query-exclude_terms = _notmuch_string_list_create (query);
+
 return query;
 }
 
@@ -97,6 +100,13 @@ notmuch_query_get_sort (notmuch_query_t *query)
 return query-sort;
 }
 
+void
+notmuch_query_add_tag_exclude (notmuch_query_t *query, const char *tag)
+{
+char *term = talloc_asprintf (query, %s%s, _find_prefix (tag), tag);
+_notmuch_string_list_append (query-exclude_terms, term);
+}
+
 /* We end up having to call the destructors explicitly because we had
  * to use placement new in order to initialize C++ objects within a
  * block that we allocated with talloc. So C++ is making talloc
@@ -112,6 +122,27 @@ _notmuch_messages_destructor (notmuch_mset_messages_t 
*messages)
 return 0;
 }
 
+/* Return a query that does not match messages with the excluded tags
+ * registered with the query.  Any tags that explicitly appear in
+ * xquery will not be excluded. */
+static Xapian::Query
+_notmuch_exclude_tags (notmuch_query_t *query, Xapian::Query xquery)
+{
+for (notmuch_string_node_t *term = query-exclude_terms-head; term;
+term = term-next) {
+   Xapian::TermIterator it = xquery.get_terms_begin ();
+   Xapian::TermIterator end = xquery.get_terms_end ();
+   for (; it != end; it++) {
+   if ((*it).compare (term-string) == 0)
+   break;
+   }
+   if (it == end)
+   xquery = Xapian::Query (Xapian::Query::OP_AND_NOT,
+   xquery, Xapian::Query (term-string));
+}
+return xquery;
+}
+
 notmuch_messages_t *
 notmuch_query_search_messages (notmuch_query_t *query)
 {
@@ -157,6 +188,8 @@ notmuch_query_search_messages (notmuch_query_t *query)
 mail_query, string_query);
}
 
+   final_query = _notmuch_exclude_tags (query, final_query);
+
enquire.set_weighting_scheme (Xapian::BoolWeight());
 
switch (query-sort) {
@@ -436,6 +469,8 @@ notmuch_query_count_messages (notmuch_query_t *query)
 mail_query, string_query);
}
 
+   final_query = _notmuch_exclude_tags (query, final_query);
+
enquire.set_weighting_scheme(Xapian::BoolWeight());
enquire.set_docid_order(Xapian::Enquire::ASCENDING);
 
-- 
1.7.7.3

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


[PATCH v3 2/2] search: Support automatic tag exclusions

2012-01-14 Thread Austin Clements
This adds a search section to the config file and an
auto_tag_exclusions setting in that section.  The search and count
commands pass tag tags from the configuration to the library.
---
 notmuch-client.h |8 
 notmuch-config.c |   42 ++
 notmuch-count.c  |8 
 notmuch-search.c |8 
 test/search  |   18 ++
 5 files changed, 84 insertions(+), 0 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 517c010..62ede28 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -235,6 +235,14 @@ void
 notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config,
  notmuch_bool_t synchronize_flags);
 
+const char **
+notmuch_config_get_auto_exclude_tags (notmuch_config_t *config, size_t 
*length);
+
+void
+notmuch_config_set_auto_exclude_tags (notmuch_config_t *config,
+ const char *list[],
+ size_t length);
+
 int
 notmuch_run_hook (const char *db_path, const char *hook);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index d697138..3d4d5b9 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -84,6 +84,15 @@ static const char maildir_config_comment[] =
 \tand update tags, while the \notmuch tag\ and \notmuch restore\\n
 \tcommands will notice tag changes and update flags in filenames\n;
 
+static const char search_config_comment[] =
+ Search configuration\n
+\n
+ The following option is supported here:\n
+\n
+\tauto_exclude_tags  A ;-separated list of tags that will be\n
+\t excluded from search results by default.  Using an excluded tag\n
+\t in a query will override that exclusion.\n;
+
 struct _notmuch_config {
 char *filename;
 GKeyFile *key_file;
@@ -96,6 +105,8 @@ struct _notmuch_config {
 const char **new_tags;
 size_t new_tags_length;
 notmuch_bool_t maildir_synchronize_flags;
+const char **auto_exclude_tags;
+size_t auto_exclude_tags_length;
 };
 
 static int
@@ -221,6 +232,7 @@ notmuch_config_open (void *ctx,
 int file_had_new_group;
 int file_had_user_group;
 int file_had_maildir_group;
+int file_had_search_group;
 
 if (is_new_ret)
*is_new_ret = 0;
@@ -252,6 +264,8 @@ notmuch_config_open (void *ctx,
 config-new_tags = NULL;
 config-new_tags_length = 0;
 config-maildir_synchronize_flags = TRUE;
+config-auto_exclude_tags = NULL;
+config-auto_exclude_tags_length = 0;
 
 if (! g_key_file_load_from_file (config-key_file,
 config-filename,
@@ -295,6 +309,7 @@ notmuch_config_open (void *ctx,
 file_had_new_group = g_key_file_has_group (config-key_file, new);
 file_had_user_group = g_key_file_has_group (config-key_file, user);
 file_had_maildir_group = g_key_file_has_group (config-key_file, 
maildir);
+file_had_search_group = g_key_file_has_group (config-key_file, search);
 
 
 if (notmuch_config_get_database_path (config) == NULL) {
@@ -345,6 +360,11 @@ notmuch_config_open (void *ctx,
notmuch_config_set_new_tags (config, tags, 2);
 }
 
+if (notmuch_config_get_auto_exclude_tags (config, tmp) == NULL) {
+   const char *tags[] = { deleted, spam };
+   notmuch_config_set_auto_exclude_tags (config, tags, 2);
+}
+
 error = NULL;
 config-maildir_synchronize_flags =
g_key_file_get_boolean (config-key_file,
@@ -387,6 +407,11 @@ notmuch_config_open (void *ctx,
maildir_config_comment, NULL);
 }
 
+if (! file_had_search_group) {
+   g_key_file_set_comment (config-key_file, search, NULL,
+   search_config_comment, NULL);
+}
+
 if (is_new_ret)
*is_new_ret = is_new;
 
@@ -597,6 +622,23 @@ notmuch_config_set_new_tags (notmuch_config_t *config,
 (config-new_tags));
 }
 
+const char **
+notmuch_config_get_auto_exclude_tags (notmuch_config_t *config, size_t *length)
+{
+return _config_get_list (config, search, auto_exclude_tags,
+(config-auto_exclude_tags),
+(config-auto_exclude_tags_length), length);
+}
+
+void
+notmuch_config_set_auto_exclude_tags (notmuch_config_t *config,
+ const char *list[],
+ size_t length)
+{
+_config_set_list (config, search, auto_exclude_tags, list, length,
+ (config-auto_exclude_tags));
+}
+
 /* Given a configuration item of the form group.key return the
  * component group and key. If any error occurs, print a message on
  * stderr and return 1. Otherwise, return 0.
diff --git a/notmuch-count.c b/notmuch-count.c
index 0982f99..f77861e 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -35,6 +35,9 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
 char *query_str;
 int 

The overloading of show (was Re: [PATCH] Output unmodified Content-Type header value for JSON format.)

2012-01-14 Thread Austin Clements
(was in reply to id:87ehv2proa@praet.org, but I wanted to start a
new top-level thread)

Quoth Pieter Praet on Jan 14 at 10:19 am:
 On Thu, 12 Jan 2012 12:28:40 -0500, Austin Clements amdra...@mit.edu wrote:
  Quoth Pieter Praet on Jan 12 at  6:07 pm:
   On Tue, 22 Nov 2011 22:40:21 -0500, Austin Clements amdra...@mit.edu 
   wrote:
Quoth Jameson Graef Rollins on Nov 20 at 12:10 pm:
 The open question seems to be how we handle the content encoding
 parameters.  My argument is that those should either be used by 
 notmuch
 to properly encode the content for the consumer.  If that's not
 possible, then just those parameters needed by the consumer to decode
 the content should be output.

If notmuch is going to include part content in the JSON output (which
perhaps it shouldn't, as per recent IRC discussions), then it must
handle content encodings because JSON must be Unicode and therefore
the content strings in the JSON must be Unicode.
   
   Having missed the IRC discussions: what is the rationale for not
   including (specific types of?) part content in the JSON output ?
   Eg. how about inline attached text/x-patch ?
  
  Technically the IRC discussion was about not including *any* part
  content in the JSON output, and always using show --format=raw or
  similar to retrieve desired parts.  Currently, notmuch includes part
  content in the JSON only for text/*, *except* when it's text/html.  I
  assume non-text parts are omitted because binary data is hard to
  represent in JSON and text/html is omitted because some people don't
  need it.  However, this leads to some peculiar asymmetry in the Emacs
  code where sometimes it pulls part content out of the JSON and
  sometimes it retrieves it using show --format=raw.  This in turn leads
  to asymmetry in content encoding handling, since notmuch handles
  content encoding for parts included in the JSON (and there's no good
  way around that since JSON is Unicode), but not for parts retrieved as
  raw.
  
  The idea discussed on IRC was to remove all part content from the JSON
  output and to always use show to retrieve it, possibly beefing up
  show's support for content decoding (and possibly introducing a way to
  retrieve multiple raw parts at once to avoid re-parsing).  This would
  get the JSON format out of the business of guessing what consumers
  need, simplify the Emacs code, and normalize content encoding
  handling.
 
 Full ACK.
 
 One concern though (IIUC): Due to the prevalence of retarded MUA's, not
 outputting 'text/plain' and/or 'text/html' parts is unfortunately all
 too often equivalent to not outputting anything at all, so wouldn't we,
 in essence, be reducing `show --format=json' to an ever-so-slightly
 augmented `search --format=json' ?

I'm not sure I fully understand what you're saying, but there are
several levels of structure here:

1. Threads (query results)
2. Thread structure
3. Message structure (MIME)
4. Part content

Currently, search returns 1; show --format=json returns 2, 3, and
sometimes 4 (but sometimes not); and show --format=raw returns 4.
Notably, 1 does not require opening message files (neither does 2),
which I consider an important distinction between search and show.

Some of the discussion has been about putting 4 squarely in the realm
of show --format=raw.  One counterargument (which has grown on me
since this discussion) is that the part content included in
--format=json can be thought of as pre-fetching content that clients
are likely to need in order to avoid re-parsing the message in most
circumstances.  I believe this is not the *intent* of the current
code, though without a specification of the JSON format it's hard to
tell.

Other discussion (more interesting, in my mind) has been about
separating retrieving thread structure, 2, from retrieving message
structure, 3.  To me, splitting these feels much more natural than
what we do now, which seems to be inflexibly bound to the specific way
the Emacs show mode currently works.  The thread structure is readily
available from the database, so I think separating these would open up
some new UI opportunities, particularly easy and fast thread outlining
and navigation.  I believe it would also simplify the code and address
some irritating asymmetries in the way notmuch show handles the --part
argument.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


  1   2   >