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

[Dmitry Kurochkin]

On Sun, 5 Feb 2012 12:42:12 -0700, Adam Wolfe Gordon  
wrote:
> Thanks for the review. The style nits are things I missed in my
> previous cleanup, so thanks for pointing them out. I should probably
> run uncrustify and see if it complains about anything else.
> 
> The other points are definitely up for discussion, and some are areas
> where I was unsure to start with. Discussion inline:
> 
> On Sun, Feb 5, 2012 at 04:50, Mark Walters  
> wrote:
> >> + ? ?/* We only care about inline text parts for reply purposes */
> >> + ? ?if (reply_check_part_type (part, "text", "*", 
> >> GMIME_DISPOSITION_INLINE)) {
> >
> > This seems to be different from the logic in the text output: I think
> > that inlines all text/* regardless of disposition. I think the JSON
> > output should include at least as much as the text output as it is easy
> > for the caller to discard parts.
> 
> Indeed, the text output includes all text/* parts except for
> text/html, regardless of disposition. My thought was that it doesn't
> really make sense to quote an attachment, or at least it's not the
> behavior I would expect. But, perhaps it makes more sense to include
> all the text parts, with their dispositions, and let the MUA decide
> what it wants to quote. If anyone has thoughts on this I'm happy to
> hear them.
> 
> > Does wrapper need to a free/unref somewhere?
> 
> The text format doesn't free or unref wrapper, so I followed its
> example. But, I'm not a gmime expert, and I agree intuitively that it
> should be freed somehow. Can anyone enlighten me?
> 
> > If replying to multiple messages (such as a whole thread) you get
> > multiple sets of "new headers". I think that probably is not what is
> > wanted but its still better than the weird things the text version
> > does. Might be worth putting a comment. [What I think should happen is
> > that a union of all the headers from all these is taken throwing away
> > duplicate addresses but that is obviously not part of this patch set]
> 
> I've never been sure about what the intended behavior is when replying
> to multiple messages in the CLI. My thought was that it should create
> a reply to each message, so an MUA could iterate over them allowing
> you to compose replies to multiple messages. But, I've never wanted or
> used such a feature, so I'm agnostic on whether it's right. The emacs
> MUA (at least with my patch) ignores all but the first reply object in
> the array, my assumption being that reply only operates on multiple
> messages by accident.
> 

In some cases, notmuch CLI insists that the search query matches exactly
one message (e.g. notmuch show for parts).  IMO the same behavior makes
sense for notmuch reply.

Regards,
  Dmitry

> Does anyone use reply with multiple messages? If so, what semantics do
> you expect?
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] STYLE: Initial draft of coding style document

[Austin Clements]

Quoth David Bremner on Feb 03 at  8:50 pm:
> On Mon, 30 Jan 2012 18:01:15 +0200, Jani Nikula  wrote:
> > On Jan 30, 2012 4:38 PM, "Tomi Ollila"  wrote:
> > >
> > > > +
> > > > +* Indent is 4 spaces with mixed tabs/spaces and a tab width of 8.
> > > > +  Tabs should be only at the beginning of the line.
> > >
> > > So, after initial indentation (with tabs) there should not be further
> > > tabs? We'll be using the former instead of the latter in these 2 below?
> 
> Ah, I only meant to explain what whitespace-mode complains about, namely
> 
> x=1;
> 
> as opposed to 
> 
>  
> I'm not sure it's that important, but apparently if it stays in the
> style guide it needs rewording.

Any indentation style this difficult to explain can't be a good idea.
How about,

* Indent is 4 spaces with mixed tab/spaces and a tab width of 8.
  (Specifically, a line should begin with zero or more tabs followed
  by fewer than eight spaces.)

[PATCH] tag: remove unused attribute from notmuch_tag_command() arguments

[Dmitry Kurochkin]

On Sat, 04 Feb 2012 07:37:45 -0500, David Bremner  wrote:
> On Sat, 28 Jan 2012 12:02:33 +0400, Dmitry Kurochkin  gmail.com> wrote:
> > Argc and argv arguments are used in notmuch_tag_command() function.
> > So unused attribute is not appropriate for them.
> 
> pushed, 
> 

Are you sure?  I do not see it in master.

Regards,
  Dmitry

> d

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

[Adam Wolfe Gordon]

On Sun, Feb 5, 2012 at 20:44, Austin Clements  wrote:
> Sorry for coming late to the party. ?I really like this idea, but it
> seems like your implementation is duplicating a lot of the work of
> notmuch show. ?This makes me wonder if it would be better to return
> reply header information in the JSON (which is definitely the way to
> go) but to fetch the part body from the UI via show (and maybe reuse
> some of the show-mode logic, if it makes sense to do so). ?If this has
> already been discussed, just point me at the thread and I'll catch
> myself up.

Thanks for taking a look. Dmitry noted on IRC that inlining the HTML
in JSON could cause issues with non-UTF8 character sets. Right now I'm
working on essentially what you've suggested - having the CLI produce
only headers, and then using show to get the quotable body.

Something else that was mentioned on IRC is using some of the notmuch
show logic to produce the show JSON format as part of reply. I looked
into this, but it would take some serious refactoring (to make the
show JSON stuff accessible to reply), and since emacs will need to end
up calling show anyway, I'm not sure it's worth it. I do like the idea
of different CLI commands being able to produce standardized formats
through some shared interface, I'm just not sure it's necessary here,
and not sure what the interface should look like.

NEWS: add entries for the changes in the python bindings

[Tomi Ollila]

On Sun, 29 Jan 2012 18:08:50 +0100, Justus Winter <4winter at 
informatik.uni-hamburg.de> wrote:
> This patch series adds a section for the python binding changes to the
> NEWS file.

I marked patches 2/3 and 3/3 stale as git-am doesn't accept those anymore
on top of 2c6710e3ba22f5af6e5813dad8bee732e6c5d02c.

> 
> Cheers,
> Justus

Tomi

[PATCH 1/2] cli: convert "notmuch show" to use the new argument parser

[Austin Clements]

Yikes.  I don't envy you this patch.  Two minor nits, otherwise this
and the next patch LGTM.

Quoth Jani Nikula on Feb 04 at 12:41 am:
> Use the new notmuch argument parser to handle arguments in "notmuch
> show". There are two corner case functional changes:
> 
> 1) Also set params.raw = 1 when defaulting to raw format when part is
>requested but format is not specified.

Huh.  So "notmuch show --part=0 " was broken before.

> 2) Do not set params.decrypt if crypto context creation fails.

Technically this also behaves differently if given multiple --format
arguments, but I'll let that slide.

> 
> Signed-off-by: Jani Nikula 
> ---
>  notmuch-show.c |  153 
> +---
>  1 files changed, 79 insertions(+), 74 deletions(-)
> 
> diff --git a/notmuch-show.c b/notmuch-show.c
> index dec799c..f93e121 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -1049,6 +1049,14 @@ do_show (void *ctx,
>  return 0;
>  }
>  
> +enum {
> +NOTMUCH_FORMAT_NOT_SPECIFIED,
> +NOTMUCH_FORMAT_JSON,
> +NOTMUCH_FORMAT_TEXT,
> +NOTMUCH_FORMAT_MBOX,
> +NOTMUCH_FORMAT_RAW
> +};
> +
>  int
>  notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
>  {
> @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), 
> unused (char *argv[]))
>  notmuch_database_t *notmuch;
>  notmuch_query_t *query;
>  char *query_string;
> -char *opt;
> +int opt_index;
>  const notmuch_show_format_t *format = _text;
> -notmuch_show_params_t params;
> -int mbox = 0;
> -int format_specified = 0;
> -int i;
> +notmuch_show_params_t params = { .part = -1 };
> +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
> +notmuch_bool_t decrypt = FALSE;
> +notmuch_bool_t verify = FALSE;
> +notmuch_bool_t entire_thread = FALSE;
> +
> +notmuch_opt_desc_t options[] = {
> + { NOTMUCH_OPT_KEYWORD, _sel, "format", 'f',
> +   (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON },
> +   { "text", NOTMUCH_FORMAT_TEXT },
> +   { "mbox", NOTMUCH_FORMAT_MBOX },
> +   { "raw", NOTMUCH_FORMAT_RAW },
> +   { 0, 0 } } },
> + { NOTMUCH_OPT_INT, , "part", 'p', 0 },
> + { NOTMUCH_OPT_BOOLEAN, _thread, "entire-thread", 't', 0 },
> + { NOTMUCH_OPT_BOOLEAN, , "decrypt", 'd', 0 },
> + { NOTMUCH_OPT_BOOLEAN, , "verify", 'v', 0 },
> + { 0, 0, 0, 0, 0 }
> +};
> +
> +opt_index = parse_arguments (argc, argv, options, 1);
> +if (opt_index < 0) {
> + /* diagnostics already printed */
> + return 1;
> +}
>  
> -params.entire_thread = 0;
> -params.raw = 0;
> -params.part = -1;
> -params.cryptoctx = NULL;
> -params.decrypt = 0;
> +params.entire_thread = entire_thread;

If you make params.entire_thread a notmuch_bool_t (instead of an int),
you could pass _thread in the notmuch_opt_desc_t and get
rid of the local variable.

>  
> -argc--; argv++; /* skip subcommand argument */
> +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) {
> + /* if part was requested and format was not specified, use format=raw */
> + if (params.part >= 0)
> + format_sel = NOTMUCH_FORMAT_RAW;
> + else
> + format_sel = NOTMUCH_FORMAT_TEXT;
> +}
>  
> -for (i = 0; i < argc && argv[i][0] == '-'; i++) {
> - if (strcmp (argv[i], "--") == 0) {
> - i++;
> - break;
> +switch (format_sel) {
> +case NOTMUCH_FORMAT_JSON:
> + format = _json;
> + params.entire_thread = 1;
> + break;
> +case NOTMUCH_FORMAT_TEXT:
> + format = _text;
> + break;
> +case NOTMUCH_FORMAT_MBOX:
> + if (params.part > 0) {
> + fprintf (stderr, "Error: specifying parts is incompatible with mbox 
> output format.\n");
> + return 1;
>   }
> - if (STRNCMP_LITERAL (argv[i], "--format=") == 0) {
> - opt = argv[i] + sizeof ("--format=") - 1;
> - if (strcmp (opt, "text") == 0) {
> - format = _text;
> - } else if (strcmp (opt, "json") == 0) {
> - format = _json;
> - params.entire_thread = 1;
> - } else if (strcmp (opt, "mbox") == 0) {
> - format = _mbox;
> - mbox = 1;
> - } else if (strcmp (opt, "raw") == 0) {
> - format = _raw;
> - params.raw = 1;
> - } else {
> - fprintf (stderr, "Invalid value for --format: %s\n", opt);
> - return 1;
> - }
> - format_specified = 1;
> - } else if (STRNCMP_LITERAL (argv[i], "--part=") == 0) {
> - params.part = atoi(argv[i] + sizeof ("--part=") - 1);
> - } else if (STRNCMP_LITERAL (argv[i], "--entire-thread") == 0) {
> - params.entire_thread = 1;
> - } else if ((STRNCMP_LITERAL (argv[i], "--verify") == 0) ||
> -(STRNCMP_LITERAL (argv[i], 

[PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[Jameson Graef Rollins]

Sorry to be so late on this, but I'm not a big fan of this new feature.
I would prefer to always see the subject (or any other field for that
matter) as is.

As a principle I would prefer there not be text replacements unless it's
very clear that text has been replaced.  Buttons work because it's clear
they're buttons.  This is not the case here, though, since this text
replacement could actually be confused with real text.

It's also not clear to me why this feature would be needed.  I have
never found blank subjects confusing.  The field is always clearly
delineated, at least in search and show mode.  If it's not clear
elsewhere, maybe we can make the delineation of the subject field
clearer, but leave the actual subject text string alone.

If some feel this feature is really needed we should at least have a
customization variable.  notmuch-unblank-subject?  I don't have any good
name suggestions, though.

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/20120205/92eba810/attachment.pgp>

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

[Austin Clements]

Quoth Adam Wolfe Gordon on Jan 19 at 10:46 am:
> 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.

Sorry for coming late to the party.  I really like this idea, but it
seems like your implementation is duplicating a lot of the work of
notmuch show.  This makes me wonder if it would be better to return
reply header information in the JSON (which is definitely the way to
go) but to fetch the part body from the UI via show (and maybe reuse
some of the show-mode logic, if it makes sense to do so).  If this has
already been discussed, just point me at the thread and I'll catch
myself up.

[PATCH v2 0/2] Rewrite text show format

[Dmitry Kurochkin]

On Sat,  4 Feb 2012 16:24:24 -0500, Austin Clements  wrote:
> v2
> 
> * Remove unnecessary braces (id:87lioooj7m.fsf at gmail.com)
> 
> * Trivial rebase against master
> 

Based on the change log and IRC discussion, I think this series does not
need another round of reviews.  So it looks like ready for master and I
removed needs-review tag.

Regards,
  Dmitry

[Patch v2 0/4] Control selection of From: header when replying

[Jani Nikula]

On Sun, 05 Feb 2012 10:58:04 +0400, Dmitry Kurochkin  wrote:
> Hi Mark.
> 
> I am not sure I like this solution.  My concerns are:
> 
> * New option looks too complex, too specific.

*shrug* The --from parameter is simple to implement, simple to test, and
simple to use.

> * There are more aspects of notmuch reply behavior which users would
>   like to change (e.g. which part to quote).  If we add an option for
>   each, we complicate both nomtuch show UI and code.

This I agree is more of an issue.

> The problem is that notmuch show output format is too limiting.  Instead
> of providing myriad of options for tweaking notmuch show text format
> behavior, we should add JSON format for notmuch reply similar to nomtuch
> show.  That would allow notmuch reply to produce structured output with
> required additional information, which should be enough for users to
> construct whatever reply they want.

Heh, when I told Mark on IRC to just send the patches and not discuss
and worry about it too much, I added "...and then someone will come up
with an approach we failed to think of, and scrap the whole
thing". Thanks Dmitry! ;)

> In this particular case, notmuch reply JSON format could have
> "from-source" attribute that would indicate how it was guessed.

My first thought is that it's offloading things that are trivial in the
cli to the users of the cli where it might be slightly more complicated,
but...

> Now the best part.  Not so long ago, Adam (in Cc) provided a patch for
> improving nomtuch reply for HTML-only emails.  At first he added an
> option for notmuch reply, like you did for from-guessing.  I suggested
> him to implement it based on the JSON format instead and he did.  AFAIK
> the latest version of his patches is [1].  I did not look at the code
> though.  It seems that it is waiting for more review.
> 
> So, instead of adding more nomtuch show options, I think a better
> solution is to work with Adam to get the notmuch reply JSON format to
> master and then fix the from-guessing issue by adding an attribute to
> notmuch reply JSON format.

...no matter what the solution will be in the end, I agree it's best to
get Adam's work merged first, and see how easily this can be handled
using JSON vs. the parameter.

Thanks for your insights.


BR,
Jani.


> Regards,
>   Dmitry
> 
> [1] id:"1326995217-27423-1-git-send-email-awg+notmuch at xvx.ca"
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Installation on Mac OS X Lion

[Saurabh Nanda]

Hi,

I'm trying to install notmuchmail on my Mac box, but have (obviously) 
run into dependancy issues (Xapian, GMime, Glib, talloc). Has anyone 
managed to install notmuchmail on Mac OS X? How painful is it? Is it 
possible to have a binary package for this?

Thanks,
Saurabh.
-- 
http://www.saurabhnanda.com

[Patch v2 0/4] Control selection of From: header when replying

[Mark Walters]

On Sun, 05 Feb 2012 10:58:04 +0400, Dmitry Kurochkin  wrote:
> Hi Mark.
> 
> I am not sure I like this solution.  My concerns are:
> 
> * New option looks too complex, too specific.
> 
> * There are more aspects of notmuch reply behavior which users would
>   like to change (e.g. which part to quote).  If we add an option for
>   each, we complicate both nomtuch show UI and code.
> 
> The problem is that notmuch show output format is too limiting.  Instead
> of providing myriad of options for tweaking notmuch show text format
> behavior, we should add JSON format for notmuch reply similar to nomtuch
> show.  That would allow notmuch reply to produce structured output with
> required additional information, which should be enough for users to
> construct whatever reply they want.
> 
> In this particular case, notmuch reply JSON format could have
> "from-source" attribute that would indicate how it was guessed.
> 
> Now the best part.  Not so long ago, Adam (in Cc) provided a patch for
> improving nomtuch reply for HTML-only emails.  At first he added an
> option for notmuch reply, like you did for from-guessing.  I suggested
> him to implement it based on the JSON format instead and he did.  AFAIK
> the latest version of his patches is [1].  I did not look at the code
> though.  It seems that it is waiting for more review.
> 
> So, instead of adding more nomtuch show options, I think a better
> solution is to work with Adam to get the notmuch reply JSON format to
> master and then fix the from-guessing issue by adding an attribute to
> notmuch reply JSON format.

Yes I think you are quite right: this would be a better solution
(particularly as Adam has already done all the hard work!)

Many thanks

Mark

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

[Mark Walters]

On Sun, 05 Feb 2012 11:50:12 +, Mark Walters  
wrote:
> On Thu, 19 Jan 2012 10:46:54 -0700, Adam Wolfe Gordon  
> wrote:
> > 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 this is only a preliminary look so far as I read the code. Note this
> is the first time I have tried reviewing a substantial chunk of code so
> sorry for any stupidities on my part!

After Austin's show modifications (commit 7430a42) I needed the
following patch which is probably trivial but I was only guessing based
on the other change to notmuch-reply Austin made at the time.

Best wishes

Mark

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 9aefce8..1c62b54 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -56,7 +56,7 @@ static const notmuch_show_format_t format_reply = {
 };

 static const notmuch_show_format_t format_json = {
-"",
+"", NULL,
"", NULL,
"", NULL, NULL, "",
"",

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

[Adam Wolfe Gordon]

Thanks for the review. The style nits are things I missed in my
previous cleanup, so thanks for pointing them out. I should probably
run uncrustify and see if it complains about anything else.

The other points are definitely up for discussion, and some are areas
where I was unsure to start with. Discussion inline:

On Sun, Feb 5, 2012 at 04:50, Mark Walters  wrote:
>> + ? ?/* We only care about inline text parts for reply purposes */
>> + ? ?if (reply_check_part_type (part, "text", "*", 
>> GMIME_DISPOSITION_INLINE)) {
>
> This seems to be different from the logic in the text output: I think
> that inlines all text/* regardless of disposition. I think the JSON
> output should include at least as much as the text output as it is easy
> for the caller to discard parts.

Indeed, the text output includes all text/* parts except for
text/html, regardless of disposition. My thought was that it doesn't
really make sense to quote an attachment, or at least it's not the
behavior I would expect. But, perhaps it makes more sense to include
all the text parts, with their dispositions, and let the MUA decide
what it wants to quote. If anyone has thoughts on this I'm happy to
hear them.

> Does wrapper need to a free/unref somewhere?

The text format doesn't free or unref wrapper, so I followed its
example. But, I'm not a gmime expert, and I agree intuitively that it
should be freed somehow. Can anyone enlighten me?

> If replying to multiple messages (such as a whole thread) you get
> multiple sets of "new headers". I think that probably is not what is
> wanted but its still better than the weird things the text version
> does. Might be worth putting a comment. [What I think should happen is
> that a union of all the headers from all these is taken throwing away
> duplicate addresses but that is obviously not part of this patch set]

I've never been sure about what the intended behavior is when replying
to multiple messages in the CLI. My thought was that it should create
a reply to each message, so an MUA could iterate over them allowing
you to compose replies to multiple messages. But, I've never wanted or
used such a feature, so I'm agnostic on whether it's right. The emacs
MUA (at least with my patch) ignores all but the first reply object in
the array, my assumption being that reply only operates on multiple
messages by accident.

Does anyone use reply with multiple messages? If so, what semantics do
you expect?

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

[Mark Walters]

On Thu, 19 Jan 2012 10:46:56 -0700, Adam Wolfe Gordon  
wrote:
> Using the new JSON reply format allows emacs to quote HTML
> parts nicely by using mm-display-part to turn them into displayable
> text, then quoting them. This is very useful for users who
> regularly receive HTML-only email.
> 
> The behavior for messages that contain plain text parts should be
> unchanged.

Hi I have tried using this patch and it did give an identical emacs
buffer when replying to text/plain or multipart/alternative messages
except for two things:

the old one put a line in the reply for each attachment not being
included in the reply (eg "Attachment: file.pdf (application/pdf)" ) and
the new one does not.

and your version does not include text/plain attachments with
disposition attachment (obviously as the cli part does not).

[Note I am not saying whether either of these correct or not, just that
they are changes.]

It also worked nicely on the html only messages I tried it on.

I have not reviewed the lisp yet; I will try and have a look at it but I
am a lisp beginner.

Best wishes

Mark

PS Sorry I sent this from my unsubscribed address first: since I am
using your patch-set my recent reply From: modification wasn't there!

newbie questions

[albin stjerna]

On Fri, 3 Feb 2012 12:59:34 +0100, Tamas Papp  wrote:
> Hi,

Hi Tamas, and (I suppose) welcome to the Notmuch mailing list!

> I just started using notmuch. It is fascinating, but I still need to
> figure out a few things:
> 
> 1. How can I restrict searches (eg of my inbox) to the last few
> messages (eg 50-100) or some date (eg last 2 weeks)? I am using the
> Emacs interface.

Well, the query syntax is ... I
know there's been talk on the list about some way to use it more easily
in the queries, and it wouldn't be hard to write a function that
generated a query using a more human-friendly notation of time. So far I
don't know about anyone actually having written such a function though,
but I wouldn't be surprised if someone has.

> 2. Could someone point me to some guides on workflow with notmuch in
> Emacs? So far I have been using mutt, and I guess I need to rethink a
> few things.

Possibly. I've only used mutt briefly and a very long time ago, so I
don't really remember, but as you've probably already seen notmuch
relies very heavily on tags, as opposed to just about any other email
client out there (except Gmail, which I've found to be very similar). So
?inbox?, ?archived?, ?read? and so on isn't really ?boxes? the way one
is used to, rather than separate labels you can set (and unset) on each
thread.

My workflow is something like this: first I fetch mail from IMAP using
offlineimap, then I run a script that tags incoming mail as personal,
list, notmuch, rss and so on (notmuch is also my RSS reader). I've set
the saved searches in Emacs to show fairly standard ?tag: AND
tag:inbox? searches for most of my tags. Then each morning, I begin by
processing ?personal? from the top to the bottom, because the ?personal?
tag is the one most likely to contain important and/or time-critical
mail (in effect it is the catch-all tag for all mail that isn't directed
to a mailing list or otherwise filtered out as probably uninteresting).

If you're interested, I can send you my mail tagging script.

When I'm done with each mail I archive it (using the standard keybinding
?a?). Most mails are handled more or less immediately, but some (in
particular posts from RSS) take longer time to read. Those I tag with
?read/review? using a custom keybinding ?T?. I can then process those
when I have the time.

> 3. If I have multiple accounts, how can I change my e-mail address
> (From:) when I am writing messages (within emacs, using the m key).

I'm mostly changing this by just typing my other email address (and
tab-completing it of course). I actually haven't thought about there
being any other way, but now that you mention it there probably is. :)

Hope this helps! Albin

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

[Mark Walters]

On Thu, 19 Jan 2012 10:46:54 -0700, Adam Wolfe Gordon  
wrote:
> 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 this is only a preliminary look so far as I read the code. Note this
is the first time I have tried reviewing a substantial chunk of code so
sorry for any stupidities on my part!

Best wishes

Mark

>  notmuch-reply.c |  271 
> +++
>  1 files changed, 231 insertions(+), 40 deletions(-)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 0f682db..b4c2426 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);
> +
>  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)
>  {
> @@ -86,6 +111,17 @@ reply_headers_message_part (GMimeMessage *message)
>  printf ("> Date: %s\n", g_mime_message_get_date_as_string (message));
>  }
>  
> +static notmuch_bool_t
> +reply_check_part_type (GMimeObject *part, const char *type, const char 
> *subtype,
> +const char *disposition)
> +{
> +GMimeContentType *content_type = g_mime_object_get_content_type 
> (GMIME_OBJECT (part));
> +GMimeContentDisposition *part_disposition = 
> g_mime_object_get_content_disposition (part);
> +
> +return (g_mime_content_type_is_type (content_type, type, subtype) &&
> + (!part_disposition ||
> +  strcmp (part_disposition->disposition, disposition) == 0));
> +}
>  
>  static void
>  reply_part_content (GMimeObject *part)
> @@ -147,6 +183,63 @@ reply_part_content (GMimeObject *part)
>  }
>  }
>  
> +static void
> +reply_part_start_json (GMimeObject *part, unused (int *part_count))
> +{
> +if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE))
> + printf ("{ ");
> +}
> +
> +static void
> +reply_part_end_json (GMimeObject *part)
> +{
> +if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE))
> + printf ("}, ");
> +}
> +
> +static void
> +reply_part_content_json (GMimeObject *part)
> +{
> +GMimeContentType *content_type = g_mime_object_get_content_type 
> (GMIME_OBJECT (part));
> +void *ctx = talloc_new (NULL);
> +
> +/* We only care about inline text parts for reply purposes */
> +if (reply_check_part_type (part, "text", "*", GMIME_DISPOSITION_INLINE)) 
> {

This seems to be different from the logic in the text output: I think
that inlines all text/* regardless of disposition. I think the JSON
output should include at least as much as the text output as it is easy
for the caller to discard parts.

> + GMimeDataWrapper *wrapper;
> + GByteArray *part_content;
> +
> + printf ("\"content-type\": %s, \"content\": ",
> +json_quote_str (ctx, g_mime_content_type_to_string 
> (content_type)));
> +
> + wrapper = g_mime_part_get_content_object (GMIME_PART (part));
> + if (wrapper) {
> + const char *charset = g_mime_object_get_content_type_parameter 
> (part, "charset");
> + GMimeStream *stream_memory = g_mime_stream_mem_new ();
> + if (stream_memory) {
> + GMimeStream *stream_filter = NULL;
> + 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"));
> + }
> +
> + if (stream_filter) {

should the if (charset) block be inside the if (stream_filter) block?

> + g_mime_data_wrapper_write_to_stream (wrapper, 
> stream_filter);
> + part_content = g_mime_stream_mem_get_byte_array 
> (GMIME_STREAM_MEM (stream_memory));
> +
> + printf ("%s", json_quote_chararray (ctx, (char *) 
> part_content->data, part_content->len));
> +

[PATCH v5 12/12] NEWS: document Emacs UI tagging operations changes

[Dmitry Kurochkin]

---
 NEWS |   18 ++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index 5c5b645..f449fba 100644
--- a/NEWS
+++ b/NEWS
@@ -39,6 +39,24 @@ Reply to sender
   and search modes, 'r' has been bound to reply to sender, replacing
   reply to all, which now has key binding 'R'.

+More flexible and consistent tagging operations
+
+  All tagging operations ("+", "-", "*") now accept multiple tags with
+  "+" or "-" prefix, like "*" operation in notmuch-search view before.
+
+  "*" operation (`notmuch-show-tag-all') is now available in
+  notmuch-show view.
+
+  `Notmuch-show-{add,remove}-tag' functions no longer accept tag
+  argument, `notmuch-show-tag-message' should be used instead.  Custom
+  bindings using these functions should be updated, e.g.:
+
+(notmuch-show-remove-tag "unread")
+
+  should be changed to:
+
+(notmuch-show-tag-message "-unread")
+
 Library changes
 ---

-- 
1.7.9

[PATCH v5 11/12] emacs: s/tags/tag-changes/ for arguments of tagging functions

[Dmitry Kurochkin]

This makes the argument names more consistent and clear.  The
following functions changed: `notmuch-tag',
`notmuch-search-tag-thread', `notmuch-search-tag-region' and
`notmuch-search-tag-all'.
---
 emacs/notmuch.el |   33 +
 1 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 0ffdf9c..8250961 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -537,25 +537,26 @@ and will also appear in a buffer named \"*Notmuch 
errors*\"."
(error (buffer-substring beg end))
))

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

-TAGS should be a list of strings of the form \"+TAG\" or \"-TAG\" and
-QUERY should be a string containing the search-query.
+TAG-CHANGES should be a list of strings of the form \"+tag\" or
+\"-tag\" and QUERY should be a string containing the
+search-query.

 Note: Other code should always use this function alter tags of
 messages instead of running (notmuch-call-notmuch-process \"tag\" ..)
 directly, so that hooks specified in notmuch-before-tag-hook and
 notmuch-after-tag-hook will be run."
   ;; Perform some validation
-  (mapc (lambda (tag)
- (unless (string-match-p "^[-+]\\S-+$" tag)
+  (mapc (lambda (tag-change)
+ (unless (string-match-p "^[-+]\\S-+$" tag-change)
(error "Tag must be of the form `+this_tag' or `-that_tag'")))
-   tags)
-  (unless (null tags)
+   tag-changes)
+  (unless (null tag-changes)
 (run-hooks 'notmuch-before-tag-hook)
 (apply 'notmuch-call-notmuch-process "tag"
-  (append tags (list "--" query)))
+  (append tag-changes (list "--" query)))
 (run-hooks 'notmuch-after-tag-hook)))

 (defcustom notmuch-before-tag-hook nil
@@ -615,26 +616,26 @@ the messages that were tagged"
(forward-line 1))
   output)))

-(defun notmuch-search-tag-thread ( tags)
+(defun notmuch-search-tag-thread ( tag-changes)
   "Change tags for the currently selected thread.

 See `notmuch-search-tag-region' for details."
-  (apply 'notmuch-search-tag-region (point) (point) tags))
+  (apply 'notmuch-search-tag-region (point) (point) tag-changes))

-(defun notmuch-search-tag-region (beg end  tags)
+(defun notmuch-search-tag-region (beg end  tag-changes)
   "Change tags for threads in the given region.

 TAGS is a list of tag operations for `notmuch-tag'.  The tags are
 added or removed for all threads in the region from BEG to END."
   (let ((search-string (notmuch-search-find-thread-id-region-search beg end)))
-(apply 'notmuch-tag search-string tags)
+(apply 'notmuch-tag search-string tag-changes)
 (save-excursion
   (let ((last-line (line-number-at-pos end))
(max-line (- (line-number-at-pos (point-max)) 2)))
(goto-char beg)
(while (<= (line-number-at-pos) (min last-line max-line))
  (notmuch-search-set-tags
-  (notmuch-update-tags (notmuch-search-get-tags) tags))
+  (notmuch-update-tags (notmuch-search-get-tags) tag-changes))
  (forward-line))

 (defun notmuch-search-tag ( initial-input)
@@ -885,7 +886,7 @@ non-authors is found, assume that all of the authors match."
  (goto-char found-target)))
   (delete-process proc

-(defun notmuch-search-tag-all ( actions)
+(defun notmuch-search-tag-all ( tag-changes)
   "Add/remove tags from all matching messages.

 This command adds or removes tags from all messages matching the
@@ -897,7 +898,7 @@ Each character of the tag name may consist of alphanumeric
 characters as well as `_.+-'.
 "
   (interactive (notmuch-read-tag-changes))
-  (apply 'notmuch-tag notmuch-search-query-string actions))
+  (apply 'notmuch-tag notmuch-search-query-string tag-changes))

 (defun notmuch-search-buffer-title (query)
   "Returns the title for a buffer with notmuch search results."
-- 
1.7.9

[PATCH v5 10/12] emacs: accept empty tag list in `notmuch-tag'

[Dmitry Kurochkin]

Since `notmuch-tag' is a non-interactive function and hence is meant
to be invoked programmatically, it should accept zero tags.  Also, the
tagging operations (bound to "*", "+", "-") would accept empty input
without an error.
---
 emacs/notmuch.el |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index b06d8a1..0ffdf9c 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -548,15 +548,15 @@ messages instead of running (notmuch-call-notmuch-process 
\"tag\" ..)
 directly, so that hooks specified in notmuch-before-tag-hook and
 notmuch-after-tag-hook will be run."
   ;; Perform some validation
-  (when (null tags) (error "No tags given"))
   (mapc (lambda (tag)
  (unless (string-match-p "^[-+]\\S-+$" tag)
(error "Tag must be of the form `+this_tag' or `-that_tag'")))
tags)
-  (run-hooks 'notmuch-before-tag-hook)
-  (apply 'notmuch-call-notmuch-process
-(append (list "tag") tags (list "--" query)))
-  (run-hooks 'notmuch-after-tag-hook))
+  (unless (null tags)
+(run-hooks 'notmuch-before-tag-hook)
+(apply 'notmuch-call-notmuch-process "tag"
+  (append tags (list "--" query)))
+(run-hooks 'notmuch-after-tag-hook)))

 (defcustom notmuch-before-tag-hook nil
   "Hooks that are run before tags of a message are modified.
-- 
1.7.9

[PATCH v5 09/12] emacs: relax tag syntax check in `notmuch-tag' function

[Dmitry Kurochkin]

The tag syntax check in `notmuch-tag' function was too strict and did
not allow nmbug tags with "::".  Since the check is done for all
tagging operations in Emacs UI, this basically means that no nmbug
tags can be changed.  The patch relaxes the tag syntax check to allow
any tag names that do not include whitespace characters.
---
 emacs/notmuch.el |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 862d9e8..b06d8a1 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -550,7 +550,7 @@ notmuch-after-tag-hook will be run."
   ;; Perform some validation
   (when (null tags) (error "No tags given"))
   (mapc (lambda (tag)
- (unless (string-match-p "^[-+][-+_.[:word:]]+$" tag)
+ (unless (string-match-p "^[-+]\\S-+$" tag)
(error "Tag must be of the form `+this_tag' or `-that_tag'")))
tags)
   (run-hooks 'notmuch-before-tag-hook)
-- 
1.7.9

[PATCH v5 08/12] emacs: separate history for operations which accept single and multiple tags

[Dmitry Kurochkin]

Some tag-related operations accept a single tag without prefix
(`notmuch-select-tag-with-completion'), others accept multiple tags
prefixed with '+' or '-' (`notmuch-read-tag-changes').  Before the
change, both functions used a single default minibuffer history.  This
is inconvenient because you have to skip options with incompatible
format when going through the history.  The patch adds separate
history lists for the two functions.  Note that functions that accept
the same input format (e.g. "+", "-", "*") share the history list as
before.
---
 emacs/notmuch.el |   13 +++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 1f351a5..862d9e8 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -76,6 +76,14 @@ For example:
 (defvar notmuch-query-history nil
   "Variable to store minibuffer history for notmuch queries")

+(defvar notmuch-select-tag-history nil
+  "Variable to store minibuffer history for
+`notmuch-select-tag-with-completion' function.")
+
+(defvar notmuch-read-tag-changes-history nil
+  "Variable to store minibuffer history for
+`notmuch-read-tag-changes' function.")
+
 (defun notmuch-tag-completions ( search-terms)
   (split-string
(with-output-to-string
@@ -86,7 +94,7 @@ For example:

 (defun notmuch-select-tag-with-completion (prompt  search-terms)
   (let ((tag-list (notmuch-tag-completions search-terms)))
-(completing-read prompt tag-list)))
+(completing-read prompt tag-list nil nil nil 'notmuch-select-tag-history)))

 (defun notmuch-read-tag-changes ( initial-input  search-terms)
   (let* ((all-tag-list (notmuch-tag-completions))
@@ -106,7 +114,8 @@ For example:
(define-key map " " 'self-insert-command)
map)))
 (delete "" (completing-read-multiple "Tags (+add -drop): "
-   tag-list nil nil initial-input
+   tag-list nil nil initial-input
+   'notmuch-read-tag-changes-history

 (defun notmuch-update-tags (tags tag-changes)
   "Return a copy of TAGS with additions and removals from TAG-CHANGES.
-- 
1.7.9

[PATCH v5 07/12] emacs: add "*" binding for notmuch-show view

[Dmitry Kurochkin]

The patch adds `notmuch-show-tag-all' function bound to "*" in
notmuch-show view.  The function is similar to the
`notmuch-search-tag-all' function for the notmuch-search view: it
changes tags for all messages in the current thread.
---
 emacs/notmuch-show.el |   35 +++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 48a2a60..faa9f9b 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1084,6 +1084,7 @@ thread id.  If a prefix is given, crypto processing is 
toggled."
(define-key map "c" 'notmuch-show-stash-map)
(define-key map "=" 'notmuch-show-refresh-view)
(define-key map "h" 'notmuch-show-toggle-headers)
+   (define-key map "*" 'notmuch-show-tag-all)
(define-key map "-" 'notmuch-show-remove-tag)
(define-key map "+" 'notmuch-show-add-tag)
(define-key map "x" 'notmuch-show-archive-thread-then-exit)
@@ -1181,6 +1182,15 @@ All currently available key bindings:
 (notmuch-show-move-to-message-top)
 t))

+(defun notmuch-show-mapc (function)
+  "Iterate through all messages in the current thread with
+`notmuch-show-goto-message-next' and call FUNCTION for side
+effects."
+  (save-excursion
+(goto-char (point-min))
+(loop do (funcall function)
+ while (notmuch-show-goto-message-next
+
 ;; Functions relating to the visibility of messages and their
 ;; components.

@@ -1233,6 +1243,18 @@ Some useful entries are:
   "Return the message id of the current message."
   (concat "id:\"" (notmuch-show-get-prop :id) "\""))

+(defun notmuch-show-get-messages-ids ()
+  "Return all message ids of messages in the current thread."
+  (let ((message-ids))
+(notmuch-show-mapc
+ (lambda () (push (notmuch-show-get-message-id) message-ids)))
+message-ids))
+
+(defun notmuch-show-get-messages-ids-search ()
+  "Return a search string for all message ids of messages in the
+current thread."
+  (mapconcat 'identity (notmuch-show-get-messages-ids) " or "))
+
 ;; dme: Would it make sense to use a macro for many of these?

 (defun notmuch-show-get-filename ()
@@ -1513,6 +1535,19 @@ TAG-CHANGES is a list of tag operations for 
`notmuch-tag'."
  initial-input (notmuch-show-get-message-id
 (apply 'notmuch-show-tag-message tag-changes)))

+(defun notmuch-show-tag-all ( tag-changes)
+  "Change tags for all messages in the current thread.
+
+TAG-CHANGES is a list of tag operations for `notmuch-tag'."
+  (interactive (notmuch-read-tag-changes nil notmuch-show-thread-id))
+  (apply 'notmuch-tag (notmuch-show-get-messages-ids-search) tag-changes)
+  (notmuch-show-mapc
+   (lambda ()
+ (let* ((current-tags (notmuch-show-get-tags))
+   (new-tags (notmuch-update-tags current-tags tag-changes)))
+   (unless (equal current-tags new-tags)
+(notmuch-show-set-tags new-tags))
+
 (defun notmuch-show-add-tag ()
   "Same as `notmuch-show-tag' but sets initial input to '+'."
   (interactive)
-- 
1.7.9

[PATCH v5 06/12] emacs: rename `notmuch-search-operate-all' to `notmuch-search-tag-all'

[Dmitry Kurochkin]

`Notmuch-search-tag-all' is more clear and consistent with other
tagging function names.
---
 emacs/notmuch.el |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 1b472dd..1f351a5 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -270,7 +270,7 @@ For a mouse binding, return nil."
 (define-key map "t" 'notmuch-search-filter-by-tag)
 (define-key map "f" 'notmuch-search-filter)
 (define-key map [mouse-1] 'notmuch-search-show-thread)
-(define-key map "*" 'notmuch-search-operate-all)
+(define-key map "*" 'notmuch-search-tag-all)
 (define-key map "a" 'notmuch-search-archive-thread)
 (define-key map "-" 'notmuch-search-remove-tag)
 (define-key map "+" 'notmuch-search-add-tag)
@@ -419,7 +419,7 @@ any tags).
 Pressing \\[notmuch-search-show-thread] on any line displays that thread. The 
'\\[notmuch-search-add-tag]' and '\\[notmuch-search-remove-tag]'
 keys can be used to add or remove tags from a thread. The 
'\\[notmuch-search-archive-thread]' key
 is a convenience for archiving a thread (removing the \"inbox\"
-tag). The '\\[notmuch-search-operate-all]' key can be used to add or remove a 
tag from all
+tag). The '\\[notmuch-search-tag-all]' key can be used to add or remove a tag 
from all
 threads in the current buffer.

 Other useful commands are '\\[notmuch-search-filter]' for filtering the 
current search
@@ -876,7 +876,7 @@ non-authors is found, assume that all of the authors match."
  (goto-char found-target)))
   (delete-process proc

-(defun notmuch-search-operate-all ( actions)
+(defun notmuch-search-tag-all ( actions)
   "Add/remove tags from all matching messages.

 This command adds or removes tags from all messages matching the
-- 
1.7.9

[PATCH v5 04/12] emacs: make "+" and "-" tagging operations in notmuch-show more flexible

[Dmitry Kurochkin]

Before the change, "+" and "-" tagging operations in notmuch-show view
accepted only a single tag.  The patch makes them use the recently
added `notmuch-read-tag-changes' function, which allows to enter
multiple tags with "+" and "-" prefixes.  So after the change, "+" and
"-" bindings in notmuch-show view allow to both add and remove
multiple tags.  The only difference between "+" and "-" is the
minibuffer initial input ("+" and "-" respectively).
---
 emacs/notmuch-show.el |   73 +---
 1 files changed, 26 insertions(+), 47 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 7469e2e..48a2a60 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -38,9 +38,10 @@

 (declare-function notmuch-call-notmuch-process "notmuch" ( args))
 (declare-function notmuch-fontify-headers "notmuch" nil)
-(declare-function notmuch-select-tag-with-completion "notmuch" (prompt  
search-terms))
+(declare-function notmuch-read-tag-changes "notmuch" ( initial-input 
 search-terms))
 (declare-function notmuch-search-next-thread "notmuch" nil)
 (declare-function notmuch-search-show-thread "notmuch" nil)
+(declare-function notmuch-update-tags "notmuch" (current-tags tag-changes))

 (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
   "Headers that should be shown in a message, in this order.
@@ -1282,7 +1283,7 @@ Some useful entries are:

 (defun notmuch-show-mark-read ()
   "Mark the current message as read."
-  (notmuch-show-remove-tag "unread"))
+  (notmuch-show-tag-message "-unread"))

 ;; Functions for getting attributes of several messages in the current
 ;; thread.
@@ -1495,51 +1496,32 @@ than only the current message."
(message (format "Command '%s' exited abnormally with code %d"
 shell-command exit-code

-(defun notmuch-show-add-tags-worker (current-tags add-tags)
-  "Add to `current-tags' with any tags from `add-tags' not
-currently present and return the result."
-  (let ((result-tags (copy-sequence current-tags)))
-(mapc (lambda (add-tag)
-   (unless (member add-tag current-tags)
- (setq result-tags (push add-tag result-tags
-   add-tags)
-(sort result-tags 'string<)))
-
-(defun notmuch-show-del-tags-worker (current-tags del-tags)
-  "Remove any tags in `del-tags' from `current-tags' and return
-the result."
-  (let ((result-tags (copy-sequence current-tags)))
-(mapc (lambda (del-tag)
-   (setq result-tags (delete del-tag result-tags)))
- del-tags)
-result-tags))
-
-(defun notmuch-show-add-tag ( toadd)
-  "Add a tag to the current message."
-  (interactive
-   (list (notmuch-select-tag-with-completion "Tag to add: ")))
+(defun notmuch-show-tag-message ( tag-changes)
+  "Change tags for the current message.

+TAG-CHANGES is a list of tag operations for `notmuch-tag'."
   (let* ((current-tags (notmuch-show-get-tags))
-(new-tags (notmuch-show-add-tags-worker current-tags toadd)))
-
+(new-tags (notmuch-update-tags current-tags tag-changes)))
 (unless (equal current-tags new-tags)
-  (apply 'notmuch-tag (notmuch-show-get-message-id)
-(mapcar (lambda (s) (concat "+" s)) toadd))
+  (apply 'notmuch-tag (notmuch-show-get-message-id) tag-changes)
   (notmuch-show-set-tags new-tags

-(defun notmuch-show-remove-tag ( toremove)
-  "Remove a tag from the current message."
-  (interactive
-   (list (notmuch-select-tag-with-completion
- "Tag to remove: " (notmuch-show-get-message-id
+(defun notmuch-show-tag ( initial-input)
+  "Change tags for the current message, read input from the minibuffer."
+  (interactive)
+  (let ((tag-changes (notmuch-read-tag-changes
+ initial-input (notmuch-show-get-message-id
+(apply 'notmuch-show-tag-message tag-changes)))

-  (let* ((current-tags (notmuch-show-get-tags))
-(new-tags (notmuch-show-del-tags-worker current-tags toremove)))
+(defun notmuch-show-add-tag ()
+  "Same as `notmuch-show-tag' but sets initial input to '+'."
+  (interactive)
+  (notmuch-show-tag "+"))

-(unless (equal current-tags new-tags)
-  (apply 'notmuch-tag (notmuch-show-get-message-id)
-(mapcar (lambda (s) (concat "-" s)) toremove))
-  (notmuch-show-set-tags new-tags
+(defun notmuch-show-remove-tag ()
+  "Same as `notmuch-show-tag' but sets initial input to '-'."
+  (interactive)
+  (notmuch-show-tag "-"))

 (defun notmuch-show-toggle-headers ()
   "Toggle the visibility of the current message headers."
@@ -1587,10 +1569,8 @@ argument, hide all of the messages."
 If the remove switch is given, tags will be removed instead of
 added."
   (goto-char (point-min))
-  (let ((tag-function (if remove
- 'notmuch-show-remove-tag
-   'notmuch-show-add-tag)))
-(loop do (funcall tag-function tag)
+  (let ((op (if remove "-" "+")))
+(loop do 

[PATCH v5 03/12] emacs: make "+" and "-" tagging operations in notmuch-search more flexible

[Dmitry Kurochkin]

Before the change, "+" and "-" tagging operations in notmuch-search
view accepted only a single tag.  The patch makes them use the
recently added `notmuch-read-tag-changes' function (renamed
`notmuch-select-tags-with-completion'), which allows to enter multiple
tags with "+" and "-" prefixes.  So after the change, "+" and "-"
bindings in notmuch-search view allow to both add and remove multiple
tags.  The only difference between "+" and "-" is the minibuffer
initial input ("+" and "-" respectively).
---
 emacs/notmuch.el |  163 +++---
 1 files changed, 81 insertions(+), 82 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 5980fea..1b472dd 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -76,38 +76,56 @@ For example:
 (defvar notmuch-query-history nil
   "Variable to store minibuffer history for notmuch queries")

-(defun notmuch-tag-completions ( prefixes search-terms)
-  (let ((tag-list
-(split-string
- (with-output-to-string
-   (with-current-buffer standard-output
- (apply 'call-process notmuch-command nil t
-nil "search-tags" search-terms)))
- "\n+" t)))
-(if (null prefixes)
-   tag-list
-  (apply #'append
-(mapcar (lambda (tag)
-  (mapcar (lambda (prefix)
-(concat prefix tag)) prefixes))
-tag-list)
+(defun notmuch-tag-completions ( search-terms)
+  (split-string
+   (with-output-to-string
+ (with-current-buffer standard-output
+   (apply 'call-process notmuch-command nil t
+ nil "search-tags" search-terms)))
+   "\n+" t))

 (defun notmuch-select-tag-with-completion (prompt  search-terms)
-  (let ((tag-list (notmuch-tag-completions nil search-terms)))
+  (let ((tag-list (notmuch-tag-completions search-terms)))
 (completing-read prompt tag-list)))

-(defun notmuch-select-tags-with-completion (prompt  prefixes  
search-terms)
-  (let ((tag-list (notmuch-tag-completions prefixes search-terms))
-   (crm-separator " ")
-   ;; By default, space is bound to "complete word" function.
-   ;; Re-bind it to insert a space instead.  Note that 
-   ;; still does the completion.
-   (crm-local-completion-map
-(let ((map (make-sparse-keymap)))
-  (set-keymap-parent map crm-local-completion-map)
-  (define-key map " " 'self-insert-command)
-  map)))
-(delete "" (completing-read-multiple prompt tag-list
+(defun notmuch-read-tag-changes ( initial-input  search-terms)
+  (let* ((all-tag-list (notmuch-tag-completions))
+(add-tag-list (mapcar (apply-partially 'concat "+") all-tag-list))
+(remove-tag-list (mapcar (apply-partially 'concat "-")
+ (if (null search-terms)
+ all-tag-list
+   (notmuch-tag-completions search-terms
+(tag-list (append add-tag-list remove-tag-list))
+(crm-separator " ")
+;; By default, space is bound to "complete word" function.
+;; Re-bind it to insert a space instead.  Note that 
+;; still does the completion.
+(crm-local-completion-map
+ (let ((map (make-sparse-keymap)))
+   (set-keymap-parent map crm-local-completion-map)
+   (define-key map " " 'self-insert-command)
+   map)))
+(delete "" (completing-read-multiple "Tags (+add -drop): "
+   tag-list nil nil initial-input
+
+(defun notmuch-update-tags (tags tag-changes)
+  "Return a copy of TAGS with additions and removals from TAG-CHANGES.
+
+TAG-CHANGES must be a list of tags names, each prefixed with
+either a \"+\" to indicate the tag should be added to TAGS if not
+present or a \"-\" to indicate that the tag should be removed
+from TAGS if present."
+  (let ((result-tags (copy-sequence tags)))
+(dolist (tag-change tag-changes)
+  (let ((op (string-to-char tag-change))
+   (tag (unless (string= tag-change "") (substring tag-change 1
+   (case op
+ (?+ (unless (member tag result-tags)
+   (push tag result-tags)))
+ (?- (setq result-tags (delete tag result-tags)))
+ (otherwise
+  (error "Changed tag must be of the form `+this_tag' or 
`-that_tag'")
+(sort result-tags 'string<)))

 (defun notmuch-foreach-mime-part (function mm-handle)
   (cond ((stringp (car mm-handle))
@@ -447,6 +465,10 @@ Complete list of currently available key bindings:
   "Return a list of threads for the current region"
   (notmuch-search-properties-in-region 'notmuch-search-thread-id beg end))

+(defun notmuch-search-find-thread-id-region-search (beg end)
+  "Return a search string for threads for the current region"
+  (mapconcat 'identity (notmuch-search-find-thread-id-region beg end) " or "))
+
 (defun notmuch-search-find-authors ()
   "Return the authors for the 

[PATCH v5 02/12] emacs: remove text properties from `notmuch-search-get-tags' result

[Dmitry Kurochkin]

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

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 19206db..5980fea 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -571,7 +571,7 @@ the messages that were tagged"
 (let ((beg (+ (point) 1)))
   (re-search-forward ")")
   (let ((end (- (point) 1)))
-   (split-string (buffer-substring beg end))
+   (split-string (buffer-substring-no-properties beg end))

 (defun notmuch-search-get-tags-region (beg end)
   (save-excursion
-- 
1.7.9

[PATCH v5 01/12] emacs: move tag format validation to `notmuch-tag' function

[Dmitry Kurochkin]

Before the change, tag format validation was done in
`notmuch-search-operate-all' function only.  The patch moves it down
to `notmuch-tag', so that all users of that function get input
validation.
---
 emacs/notmuch.el |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index cd04ffd..19206db 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -516,6 +516,12 @@ Note: Other code should always use this function alter 
tags of
 messages instead of running (notmuch-call-notmuch-process \"tag\" ..)
 directly, so that hooks specified in notmuch-before-tag-hook and
 notmuch-after-tag-hook will be run."
+  ;; Perform some validation
+  (when (null tags) (error "No tags given"))
+  (mapc (lambda (tag)
+ (unless (string-match-p "^[-+][-+_.[:word:]]+$" tag)
+   (error "Tag must be of the form `+this_tag' or `-that_tag'")))
+   tags)
   (run-hooks 'notmuch-before-tag-hook)
   (apply 'notmuch-call-notmuch-process
 (append (list "tag") tags (list "--" query)))
@@ -883,12 +889,6 @@ characters as well as `_.+-'.
   (interactive (notmuch-select-tags-with-completion
"Operations (+add -drop): notmuch tag "
'("+" "-")))
-  ;; Perform some validation
-  (when (null actions) (error "No operations given"))
-  (mapc (lambda (action)
- (unless (string-match-p "^[-+][-+_.[:word:]]+$" action)
-   (error "Action must be of the form `+this_tag' or `-that_tag'")))
-   actions)
   (apply 'notmuch-tag notmuch-search-query-string actions))

 (defun notmuch-search-buffer-title (query)
-- 
1.7.9

[PATCH v5 00/12] emacs: more flexible and consistent tagging operations

[Dmitry Kurochkin]

Changes:

v4:

* rebased on master, no conflicts so no need for another review

v4:

* rebased on master after Jameson's archiving changes

v3:

* merged 3 `notmuch-show-tag-all'-related patches into one

* add patch to clean up tagging function argument names

* fix other comments from Austin's reviews [5,6]

v2:

* add patch to remove "No tags given" error from `notmuch-tag' as
  suggested by Austin in [1]

* split patch 3 in two (search and show) for easier review

* add patch with NEWS entry

* rename `notmuch-{search,show}-operate-all' to
  `notmuch-{search,show}-tag-all'

* fix other comments from Austin's reviews [2,3,4]

Regards,
  Dmitry

[1] id:"20120129231650.GK17991 at mit.edu"
[2] id:"20120129225710.GG17991 at mit.edu"
[3] id:"20120129230229.GI17991 at mit.edu"
[4] id:"20120129231120.GJ17991 at mit.edu"
[5] id:"20120130044806.GM17991 at mit.edu"
[6] id:"20120130050402.GP17991 at mit.edu"

[PATCH 1/2] test: auto load elisp tests file in test_emacs if available

[Dmitry Kurochkin]

Hi David.

I think this is a pretty simple change.  And there is one +1 for it.  So
I am going to remove the needs-review tag, please add it back if you
disagree.

Regards,
  Dmitry

[Patch v2 0/4] Control selection of From: header when replying

[Dmitry Kurochkin]

Hi Mark.

I am not sure I like this solution.  My concerns are:

* New option looks too complex, too specific.

* There are more aspects of notmuch reply behavior which users would
  like to change (e.g. which part to quote).  If we add an option for
  each, we complicate both nomtuch show UI and code.

The problem is that notmuch show output format is too limiting.  Instead
of providing myriad of options for tweaking notmuch show text format
behavior, we should add JSON format for notmuch reply similar to nomtuch
show.  That would allow notmuch reply to produce structured output with
required additional information, which should be enough for users to
construct whatever reply they want.

In this particular case, notmuch reply JSON format could have
"from-source" attribute that would indicate how it was guessed.

Now the best part.  Not so long ago, Adam (in Cc) provided a patch for
improving nomtuch reply for HTML-only emails.  At first he added an
option for notmuch reply, like you did for from-guessing.  I suggested
him to implement it based on the JSON format instead and he did.  AFAIK
the latest version of his patches is [1].  I did not look at the code
though.  It seems that it is waiting for more review.

So, instead of adding more nomtuch show options, I think a better
solution is to work with Adam to get the notmuch reply JSON format to
master and then fix the from-guessing issue by adding an attribute to
notmuch reply JSON format.

Regards,
  Dmitry

[1] id:"1326995217-27423-1-git-send-email-awg+notmuch at xvx.ca"

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

[Mark Walters]

On Sat, 14 Jan 2012 21:40:23 -0400, David Bremner  wrote:
> 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 

[PATCH v2 1/4] cli: add --from option to reply to restrict guessing of the From: header.

[Jani Nikula]

On Sat,  4 Feb 2012 20:45:14 +, Mark Walters  
wrote:
> Add an option --from= to notmuch-reply.c to restrict guessing of the
> From: header. The existing logic looks as the main headers, then at
> the delivery headers, and finally defaults to the config file address.
> 
> This patch allows the user to restrict which of these guesses are
> made.  Currently the supported values are:
>default|fallback-allcurrent behaviour
>fallback-received   fallback to delivery headers but not config 
> file
>fallback-none only look at from/reply-to/to/cc/ headers
>none  From: header is always left empty

The patch looks good. The "primary" option added in v2 is missing from
the commit message, but no need to send a new version because of that.

I didn't check the other patches in the series.

BR,
Jani.


> 
> If the code does not find an allowed address it outputs an empty From:
> line and the caller can decide how to respond.
> ---
>  notmuch-reply.c |   45 -
>  1 files changed, 36 insertions(+), 9 deletions(-)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index f55b1d2..8c73cb7 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -24,6 +24,15 @@
>  #include "gmime-filter-reply.h"
>  #include "gmime-filter-headers.h"
>  
> +/* The order here matters as we use '<' when deciding how to behave. */
> +enum {
> +FROM_FALLBACK_ALL,
> +FROM_FALLBACK_RECEIVED,
> +FROM_FALLBACK_NONE,
> +FROM_NONE,
> +FROM_PRIMARY
> +};
> +
>  static void
>  reply_headers_message_part (GMimeMessage *message);
>  
> @@ -510,7 +519,8 @@ notmuch_reply_format_default(void *ctx,
>notmuch_config_t *config,
>notmuch_query_t *query,
>notmuch_show_params_t *params,
> -  notmuch_bool_t reply_all)
> +  notmuch_bool_t reply_all,
> +  int from_select)
>  {
>  GMimeMessage *reply;
>  notmuch_messages_t *messages;
> @@ -542,15 +552,22 @@ notmuch_reply_format_default(void *ctx,
>   from_addr = add_recipients_from_message (reply, config, message,
>reply_all);
>  
> - if (from_addr == NULL)
> + if (from_addr == NULL && from_select <= FROM_FALLBACK_RECEIVED)
>   from_addr = guess_from_received_header (config, message);
>  
> - if (from_addr == NULL)
> + if ((from_addr == NULL && from_select <= FROM_FALLBACK_ALL) ||
> + from_select == FROM_PRIMARY)
>   from_addr = notmuch_config_get_user_primary_email (config);
>  
> - from_addr = talloc_asprintf (ctx, "%s <%s>",
> -  notmuch_config_get_user_name (config),
> -  from_addr);
> + /* If we have an address and we want an address print
> +  * it. Otherwise set an empty From: header. */
> + if (from_addr != NULL && from_select != FROM_NONE) {
> + from_addr = talloc_asprintf (ctx, "%s <%s>",
> +  notmuch_config_get_user_name (config),
> +  from_addr);
> + } else {
> + from_addr = talloc_strdup (ctx, "");
> + }
>   g_mime_object_set_header (GMIME_OBJECT (reply),
> "From", from_addr);
>  
> @@ -590,7 +607,8 @@ notmuch_reply_format_headers_only(void *ctx,
> notmuch_config_t *config,
> notmuch_query_t *query,
> unused (notmuch_show_params_t *params),
> -   notmuch_bool_t reply_all)
> +   notmuch_bool_t reply_all,
> +   unused (int from_select))
>  {
>  GMimeMessage *reply;
>  notmuch_messages_t *messages;
> @@ -657,10 +675,11 @@ notmuch_reply_command (void *ctx, int argc, char 
> *argv[])
>  notmuch_query_t *query;
>  char *query_string;
>  int opt_index, ret = 0;
> -int (*reply_format_func)(void *ctx, notmuch_config_t *config, 
> notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t 
> reply_all);
> +int (*reply_format_func)(void *ctx, notmuch_config_t *config, 
> notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t 
> reply_all, int from_select);
>  notmuch_show_params_t params = { .part = -1 };
>  int format = FORMAT_DEFAULT;
>  int reply_all = TRUE;
> +int from_select = FROM_FALLBACK_ALL;
>  notmuch_bool_t decrypt = FALSE;
>  
>  notmuch_opt_desc_t options[] = {
> @@ -672,6 +691,14 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> (notmuch_keyword_t []){ { "all", TRUE },
> { "sender", FALSE },
> { 0, 0 } } },
> + { NOTMUCH_OPT_KEYWORD, _select, "from", 'F',
> +   

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

[Mark Walters]

On Thu, 19 Jan 2012 10:46:54 -0700, Adam Wolfe Gordon awg+notm...@xvx.ca 
wrote:
 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 this is only a preliminary look so far as I read the code. Note this
is the first time I have tried reviewing a substantial chunk of code so
sorry for any stupidities on my part!

Best wishes

Mark

  notmuch-reply.c |  271 
 +++
  1 files changed, 231 insertions(+), 40 deletions(-)
 
 diff --git a/notmuch-reply.c b/notmuch-reply.c
 index 0f682db..b4c2426 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);
 +
  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)
  {
 @@ -86,6 +111,17 @@ reply_headers_message_part (GMimeMessage *message)
  printf ( Date: %s\n, g_mime_message_get_date_as_string (message));
  }
  
 +static notmuch_bool_t
 +reply_check_part_type (GMimeObject *part, const char *type, const char 
 *subtype,
 +const char *disposition)
 +{
 +GMimeContentType *content_type = g_mime_object_get_content_type 
 (GMIME_OBJECT (part));
 +GMimeContentDisposition *part_disposition = 
 g_mime_object_get_content_disposition (part);
 +
 +return (g_mime_content_type_is_type (content_type, type, subtype) 
 + (!part_disposition ||
 +  strcmp (part_disposition-disposition, disposition) == 0));
 +}
  
  static void
  reply_part_content (GMimeObject *part)
 @@ -147,6 +183,63 @@ reply_part_content (GMimeObject *part)
  }
  }
  
 +static void
 +reply_part_start_json (GMimeObject *part, unused (int *part_count))
 +{
 +if (reply_check_part_type (part, text, *, GMIME_DISPOSITION_INLINE))
 + printf ({ );
 +}
 +
 +static void
 +reply_part_end_json (GMimeObject *part)
 +{
 +if (reply_check_part_type (part, text, *, GMIME_DISPOSITION_INLINE))
 + printf (}, );
 +}
 +
 +static void
 +reply_part_content_json (GMimeObject *part)
 +{
 +GMimeContentType *content_type = g_mime_object_get_content_type 
 (GMIME_OBJECT (part));
 +void *ctx = talloc_new (NULL);
 +
 +/* We only care about inline text parts for reply purposes */
 +if (reply_check_part_type (part, text, *, GMIME_DISPOSITION_INLINE)) 
 {

This seems to be different from the logic in the text output: I think
that inlines all text/* regardless of disposition. I think the JSON
output should include at least as much as the text output as it is easy
for the caller to discard parts.

 + GMimeDataWrapper *wrapper;
 + GByteArray *part_content;
 +
 + printf (\content-type\: %s, \content\: ,
 +json_quote_str (ctx, g_mime_content_type_to_string 
 (content_type)));
 +
 + wrapper = g_mime_part_get_content_object (GMIME_PART (part));
 + if (wrapper) {
 + const char *charset = g_mime_object_get_content_type_parameter 
 (part, charset);
 + GMimeStream *stream_memory = g_mime_stream_mem_new ();
 + if (stream_memory) {
 + GMimeStream *stream_filter = NULL;
 + 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));
 + }
 +
 + if (stream_filter) {

should the if (charset) block be inside the if (stream_filter) block?

 + g_mime_data_wrapper_write_to_stream (wrapper, 
 stream_filter);
 + part_content = g_mime_stream_mem_get_byte_array 
 (GMIME_STREAM_MEM (stream_memory));
 +
 + printf (%s, json_quote_chararray (ctx, (char *) 
 part_content-data, part_content-len));
 + g_object_unref (stream_filter);
 + }
 + }
 +
 + if (stream_memory)
 + g_object_unref (stream_memory);
 + }
 +}
 +
 +talloc_free (ctx);

Does 

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

[Mark Walters]

On Thu, 19 Jan 2012 10:46:56 -0700, Adam Wolfe Gordon awg+notm...@xvx.ca 
wrote:
 Using the new JSON reply format allows emacs to quote HTML
 parts nicely by using mm-display-part to turn them into displayable
 text, then quoting them. This is very useful for users who
 regularly receive HTML-only email.
 
 The behavior for messages that contain plain text parts should be
 unchanged.

Hi I have tried using this patch and it did give an identical emacs
buffer when replying to text/plain or multipart/alternative messages
except for two things:

the old one put a line in the reply for each attachment not being
included in the reply (eg Attachment: file.pdf (application/pdf) ) and
the new one does not.

and your version does not include text/plain attachments with
disposition attachment (obviously as the cli part does not).

[Note I am not saying whether either of these correct or not, just that
they are changes.]

It also worked nicely on the html only messages I tried it on.

I have not reviewed the lisp yet; I will try and have a look at it but I
am a lisp beginner.

Best wishes

Mark

PS Sorry I sent this from my unsubscribed address first: since I am
using your patch-set my recent reply From: modification wasn't there!
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

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

[Mark Walters]

On Sun, 05 Feb 2012 11:50:12 +, Mark Walters markwalters1...@gmail.com 
wrote:
 On Thu, 19 Jan 2012 10:46:54 -0700, Adam Wolfe Gordon awg+notm...@xvx.ca 
 wrote:
  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 this is only a preliminary look so far as I read the code. Note this
 is the first time I have tried reviewing a substantial chunk of code so
 sorry for any stupidities on my part!

After Austin's show modifications (commit 7430a42) I needed the
following patch which is probably trivial but I was only guessing based
on the other change to notmuch-reply Austin made at the time.

Best wishes

Mark

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 9aefce8..1c62b54 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -56,7 +56,7 @@ static const notmuch_show_format_t format_reply = {
 };
 
 static const notmuch_show_format_t format_json = {
-,
+, NULL,
, NULL,
, NULL, NULL, ,
,


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

Re: [Patch v2 0/4] Control selection of From: header when replying

[Mark Walters]

On Sun, 05 Feb 2012 10:58:04 +0400, Dmitry Kurochkin 
dmitry.kuroch...@gmail.com wrote:
 Hi Mark.
 
 I am not sure I like this solution.  My concerns are:
 
 * New option looks too complex, too specific.
 
 * There are more aspects of notmuch reply behavior which users would
   like to change (e.g. which part to quote).  If we add an option for
   each, we complicate both nomtuch show UI and code.
 
 The problem is that notmuch show output format is too limiting.  Instead
 of providing myriad of options for tweaking notmuch show text format
 behavior, we should add JSON format for notmuch reply similar to nomtuch
 show.  That would allow notmuch reply to produce structured output with
 required additional information, which should be enough for users to
 construct whatever reply they want.
 
 In this particular case, notmuch reply JSON format could have
 from-source attribute that would indicate how it was guessed.
 
 Now the best part.  Not so long ago, Adam (in Cc) provided a patch for
 improving nomtuch reply for HTML-only emails.  At first he added an
 option for notmuch reply, like you did for from-guessing.  I suggested
 him to implement it based on the JSON format instead and he did.  AFAIK
 the latest version of his patches is [1].  I did not look at the code
 though.  It seems that it is waiting for more review.
 
 So, instead of adding more nomtuch show options, I think a better
 solution is to work with Adam to get the notmuch reply JSON format to
 master and then fix the from-guessing issue by adding an attribute to
 notmuch reply JSON format.

Yes I think you are quite right: this would be a better solution
(particularly as Adam has already done all the hard work!)

Many thanks

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

Re: [Patch v2 0/4] Control selection of From: header when replying

[Jani Nikula]

On Sun, 05 Feb 2012 10:58:04 +0400, Dmitry Kurochkin 
dmitry.kuroch...@gmail.com wrote:
 Hi Mark.
 
 I am not sure I like this solution.  My concerns are:
 
 * New option looks too complex, too specific.

*shrug* The --from parameter is simple to implement, simple to test, and
simple to use.

 * There are more aspects of notmuch reply behavior which users would
   like to change (e.g. which part to quote).  If we add an option for
   each, we complicate both nomtuch show UI and code.

This I agree is more of an issue.

 The problem is that notmuch show output format is too limiting.  Instead
 of providing myriad of options for tweaking notmuch show text format
 behavior, we should add JSON format for notmuch reply similar to nomtuch
 show.  That would allow notmuch reply to produce structured output with
 required additional information, which should be enough for users to
 construct whatever reply they want.

Heh, when I told Mark on IRC to just send the patches and not discuss
and worry about it too much, I added ...and then someone will come up
with an approach we failed to think of, and scrap the whole
thing. Thanks Dmitry! ;)

 In this particular case, notmuch reply JSON format could have
 from-source attribute that would indicate how it was guessed.

My first thought is that it's offloading things that are trivial in the
cli to the users of the cli where it might be slightly more complicated,
but...

 Now the best part.  Not so long ago, Adam (in Cc) provided a patch for
 improving nomtuch reply for HTML-only emails.  At first he added an
 option for notmuch reply, like you did for from-guessing.  I suggested
 him to implement it based on the JSON format instead and he did.  AFAIK
 the latest version of his patches is [1].  I did not look at the code
 though.  It seems that it is waiting for more review.
 
 So, instead of adding more nomtuch show options, I think a better
 solution is to work with Adam to get the notmuch reply JSON format to
 master and then fix the from-guessing issue by adding an attribute to
 notmuch reply JSON format.

...no matter what the solution will be in the end, I agree it's best to
get Adam's work merged first, and see how easily this can be handled
using JSON vs. the parameter.

Thanks for your insights.


BR,
Jani.


 Regards,
   Dmitry
 
 [1] id:1326995217-27423-1-git-send-email-awg+notm...@xvx.ca
 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 0/2] Rewrite text show format

[Austin Clements]

v2

* Remove unnecessary braces (id:87lioooj7m@gmail.com)

* Trivial rebase against master

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

[PATCH v2 2/2] show: Simplify new text formatter code

[Austin Clements]

This makes the text formatter take advantage of the new code
structure.  The previously duplicated header logic is now unified,
several things that we used to compute repeatedly across different
callbacks are now computed once, and the code is simpler overall and
32% shorter.

Unifying the header logic causes this to format some dates slightly
differently, so the two affected test cases are updated.
---
 notmuch-show.c |   85 +---
 test/crypto|2 +-
 test/thread-naming |   16 +-
 3 files changed, 30 insertions(+), 73 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 6a890b2..816e0f8 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -727,67 +727,48 @@ format_part_text (const void *ctx, mime_node_t *node,
 GMimeObject *meta = node-envelope_part ?
GMIME_OBJECT (node-envelope_part) : node-part;
 GMimeContentType *content_type = g_mime_object_get_content_type (meta);
+const notmuch_bool_t leaf = GMIME_IS_PART (node-part);
+const char *part_type;
 int i;
 
 if (node-envelope_file) {
notmuch_message_t *message = node-envelope_file;
-   const char *headers[] = {
-   Subject, From, To, Cc, Bcc, Date
-   };
-   const char *name, *value;
-   unsigned int i;
 
-   printf (\fmessage{ );
-   printf (id:%s depth:%d match:%d filename:%s\n,
+   part_type = message;
+   printf (\f%s{ id:%s depth:%d match:%d filename:%s\n,
+   part_type,
notmuch_message_get_message_id (message),
indent,
notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
notmuch_message_get_filename (message));
-
-   printf (\fheader{\n);
-
-   printf (%s\n, _get_one_line_summary (ctx, message));
-
-   for (i = 0; i  ARRAY_SIZE (headers); i++) {
-   name = headers[i];
-   value = notmuch_message_get_header (message, name);
-   if (value  strlen (value))
-   printf (%s: %s\n, name, value);
-   }
-   printf (\fheader}\n);
 } else {
GMimeContentDisposition *disposition = 
g_mime_object_get_content_disposition (meta);
const char *cid = g_mime_object_get_content_id (meta);
+   const char *filename = leaf ?
+   g_mime_part_get_filename (GMIME_PART (node-part)) : NULL;
 
if (disposition 
strcmp (disposition-disposition, GMIME_DISPOSITION_ATTACHMENT) == 
0)
-   {
-   printf (\fattachment{ ID: %d, node-part_num);
-
-   } else {
-
-   printf (\fpart{ ID: %d, node-part_num);
-   }
-
-   if (GMIME_IS_PART (node-part))
-   {
-   const char *filename = g_mime_part_get_filename (GMIME_PART 
(node-part));
-   if (filename)
-   printf (, Filename: %s, filename);
-   }
+   part_type = attachment;
+   else
+   part_type = part;
 
+   printf (\f%s{ ID: %d, part_type, node-part_num);
+   if (filename)
+   printf (, Filename: %s, filename);
if (cid)
printf (, Content-id: %s, cid);
-
printf (, Content-type: %s\n, g_mime_content_type_to_string 
(content_type));
 }
 
-if (node-envelope_part) {
+if (GMIME_IS_MESSAGE (node-part)) {
GMimeMessage *message = GMIME_MESSAGE (node-part);
InternetAddressList *recipients;
const char *recipients_string;
 
printf (\fheader{\n);
+   if (node-envelope_file)
+   printf (%s\n, _get_one_line_summary (ctx, node-envelope_file));
printf (Subject: %s\n, g_mime_message_get_subject (message));
printf (From: %s\n, g_mime_message_get_sender (message));
recipients = g_mime_message_get_recipients (message, 
GMIME_RECIPIENT_TYPE_TO);
@@ -800,9 +781,11 @@ format_part_text (const void *ctx, mime_node_t *node,
printf (Cc: %s\n, recipients_string);
printf (Date: %s\n, g_mime_message_get_date_as_string (message));
printf (\fheader}\n);
+
+   printf (\fbody{\n);
 }
 
-if (!node-envelope_file) {
+if (leaf) {
if (g_mime_content_type_is_type (content_type, text, *) 
!g_mime_content_type_is_type (content_type, text, html))
{
@@ -810,45 +793,19 @@ format_part_text (const void *ctx, mime_node_t *node,
g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), 
FALSE);
show_text_part_content (node-part, stream_stdout);
g_object_unref(stream_stdout);
-   }
-   else if (g_mime_content_type_is_type (content_type, multipart, *) ||
-g_mime_content_type_is_type (content_type, message, 
rfc822))
-   {
-   /* Do nothing for multipart since its content will be printed
-* when recursing. */
-   }
-   else
-   {
+   } else {
printf (Non-text part: %s\n,
g_mime_content_type_to_string (content_type));
}
   

[PATCH v2 1/2] show: Convert text format to the new self-recursive style

[Austin Clements]

This is all code movement and a smidgen of glue.  This moves the
existing text formatter code into one self-recursive function, but
doesn't change any of the logic.  The next patch will actually take
advantage of what the new structure has to offer.

Note that this patch retains format_headers_message_part_text because
it is also used by the raw format.
---
 notmuch-show.c |  270 +---
 1 files changed, 139 insertions(+), 131 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index dec799c..6a890b2 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -21,40 +21,17 @@
 #include notmuch-client.h
 
 static void
-format_message_text (unused (const void *ctx),
-notmuch_message_t *message,
-int indent);
-static void
-format_headers_text (const void *ctx,
-notmuch_message_t *message);
-
-static void
 format_headers_message_part_text (GMimeMessage *message);
 
 static void
-format_part_start_text (GMimeObject *part,
-   int *part_count);
-
-static void
-format_part_content_text (GMimeObject *part);
-
-static void
-format_part_end_text (GMimeObject *part);
+format_part_text (const void *ctx, mime_node_t *node,
+ int indent, const notmuch_show_params_t *params);
 
 static const notmuch_show_format_t format_text = {
-, NULL,
-   \fmessage{ , format_message_text,
-   \fheader{\n, format_headers_text, 
format_headers_message_part_text, \fheader}\n,
-   \fbody{\n,
-   format_part_start_text,
-   NULL,
-   NULL,
-   format_part_content_text,
-   format_part_end_text,
-   ,
-   \fbody}\n,
-   \fmessage}\n, ,
-
+.message_set_start = ,
+.part = format_part_text,
+.message_set_sep = ,
+.message_set_end = 
 };
 
 static void
@@ -191,16 +168,6 @@ _get_one_line_summary (const void *ctx, notmuch_message_t 
*message)
 }
 
 static void
-format_message_text (unused (const void *ctx), notmuch_message_t *message, int 
indent)
-{
-printf (id:%s depth:%d match:%d filename:%s\n,
-   notmuch_message_get_message_id (message),
-   indent,
-   notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH),
-   notmuch_message_get_filename (message));
-}
-
-static void
 format_message_json (const void *ctx, notmuch_message_t *message, unused (int 
indent))
 {
 notmuch_tags_t *tags;
@@ -338,26 +305,6 @@ format_message_mbox (const void *ctx,
 fclose (file);
 }
 
-
-static void
-format_headers_text (const void *ctx, notmuch_message_t *message)
-{
-const char *headers[] = {
-   Subject, From, To, Cc, Bcc, Date
-};
-const char *name, *value;
-unsigned int i;
-
-printf (%s\n, _get_one_line_summary (ctx, message));
-
-for (i = 0; i  ARRAY_SIZE (headers); i++) {
-   name = headers[i];
-   value = notmuch_message_get_header (message, name);
-   if (value  strlen (value))
-   printf (%s: %s\n, name, value);
-}
-}
-
 static void
 format_headers_message_part_text (GMimeMessage *message)
 {
@@ -523,78 +470,6 @@ signer_status_to_string (GMimeSignerStatus x)
 #endif
 
 static void
-format_part_start_text (GMimeObject *part, int *part_count)
-{
-GMimeContentDisposition *disposition = 
g_mime_object_get_content_disposition (part);
-
-if (disposition 
-   strcmp (disposition-disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
-{
-   printf (\fattachment{ ID: %d, *part_count);
-
-} else {
-
-   printf (\fpart{ ID: %d, *part_count);
-}
-}
-
-static void
-format_part_content_text (GMimeObject *part)
-{
-const char *cid = g_mime_object_get_content_id (part);
-GMimeContentType *content_type = g_mime_object_get_content_type 
(GMIME_OBJECT (part));
-
-if (GMIME_IS_PART (part))
-{
-   const char *filename = g_mime_part_get_filename (GMIME_PART (part));
-   if (filename)
-   printf (, Filename: %s, filename);
-}
-
-if (cid)
-   printf (, Content-id: %s, cid);
-
-printf (, Content-type: %s\n, g_mime_content_type_to_string 
(content_type));
-
-if (g_mime_content_type_is_type (content_type, text, *) 
-   !g_mime_content_type_is_type (content_type, text, html))
-{
-   GMimeStream *stream_stdout = g_mime_stream_file_new (stdout);
-   g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-   show_text_part_content (part, stream_stdout);
-   g_object_unref(stream_stdout);
-}
-else if (g_mime_content_type_is_type (content_type, multipart, *) ||
-g_mime_content_type_is_type (content_type, message, rfc822))
-{
-   /* Do nothing for multipart since its content will be printed
-* when recursing. */
-}
-else
-{
-   printf (Non-text part: %s\n,
-   g_mime_content_type_to_string (content_type));
-}
-}
-
-static void

Re: [PATCH v2 0/2] Rewrite text show format

[Dmitry Kurochkin]

On Sat,  4 Feb 2012 16:24:24 -0500, Austin Clements amdra...@mit.edu wrote:
 v2
 
 * Remove unnecessary braces (id:87lioooj7m@gmail.com)
 
 * Trivial rebase against master
 

Based on the change log and IRC discussion, I think this series does not
need another round of reviews.  So it looks like ready for master and I
removed needs-review tag.

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

Re: [PATCH] tag: remove unused attribute from notmuch_tag_command() arguments

[Dmitry Kurochkin]

On Sat, 04 Feb 2012 07:37:45 -0500, David Bremner da...@tethera.net wrote:
 On Sat, 28 Jan 2012 12:02:33 +0400, Dmitry Kurochkin 
 dmitry.kuroch...@gmail.com wrote:
  Argc and argv arguments are used in notmuch_tag_command() function.
  So unused attribute is not appropriate for them.
 
 pushed, 
 

Are you sure?  I do not see it in master.

Regards,
  Dmitry

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

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

[Adam Wolfe Gordon]

Thanks for the review. The style nits are things I missed in my
previous cleanup, so thanks for pointing them out. I should probably
run uncrustify and see if it complains about anything else.

The other points are definitely up for discussion, and some are areas
where I was unsure to start with. Discussion inline:

On Sun, Feb 5, 2012 at 04:50, Mark Walters markwalters1...@gmail.com wrote:
 +    /* We only care about inline text parts for reply purposes */
 +    if (reply_check_part_type (part, text, *, 
 GMIME_DISPOSITION_INLINE)) {

 This seems to be different from the logic in the text output: I think
 that inlines all text/* regardless of disposition. I think the JSON
 output should include at least as much as the text output as it is easy
 for the caller to discard parts.

Indeed, the text output includes all text/* parts except for
text/html, regardless of disposition. My thought was that it doesn't
really make sense to quote an attachment, or at least it's not the
behavior I would expect. But, perhaps it makes more sense to include
all the text parts, with their dispositions, and let the MUA decide
what it wants to quote. If anyone has thoughts on this I'm happy to
hear them.

 Does wrapper need to a free/unref somewhere?

The text format doesn't free or unref wrapper, so I followed its
example. But, I'm not a gmime expert, and I agree intuitively that it
should be freed somehow. Can anyone enlighten me?

 If replying to multiple messages (such as a whole thread) you get
 multiple sets of new headers. I think that probably is not what is
 wanted but its still better than the weird things the text version
 does. Might be worth putting a comment. [What I think should happen is
 that a union of all the headers from all these is taken throwing away
 duplicate addresses but that is obviously not part of this patch set]

I've never been sure about what the intended behavior is when replying
to multiple messages in the CLI. My thought was that it should create
a reply to each message, so an MUA could iterate over them allowing
you to compose replies to multiple messages. But, I've never wanted or
used such a feature, so I'm agnostic on whether it's right. The emacs
MUA (at least with my patch) ignores all but the first reply object in
the array, my assumption being that reply only operates on multiple
messages by accident.

Does anyone use reply with multiple messages? If so, what semantics do
you expect?
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

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

[Dmitry Kurochkin]

On Sun, 5 Feb 2012 12:42:12 -0700, Adam Wolfe Gordon awg+notm...@xvx.ca wrote:
 Thanks for the review. The style nits are things I missed in my
 previous cleanup, so thanks for pointing them out. I should probably
 run uncrustify and see if it complains about anything else.
 
 The other points are definitely up for discussion, and some are areas
 where I was unsure to start with. Discussion inline:
 
 On Sun, Feb 5, 2012 at 04:50, Mark Walters markwalters1...@gmail.com wrote:
  +    /* We only care about inline text parts for reply purposes */
  +    if (reply_check_part_type (part, text, *, 
  GMIME_DISPOSITION_INLINE)) {
 
  This seems to be different from the logic in the text output: I think
  that inlines all text/* regardless of disposition. I think the JSON
  output should include at least as much as the text output as it is easy
  for the caller to discard parts.
 
 Indeed, the text output includes all text/* parts except for
 text/html, regardless of disposition. My thought was that it doesn't
 really make sense to quote an attachment, or at least it's not the
 behavior I would expect. But, perhaps it makes more sense to include
 all the text parts, with their dispositions, and let the MUA decide
 what it wants to quote. If anyone has thoughts on this I'm happy to
 hear them.
 
  Does wrapper need to a free/unref somewhere?
 
 The text format doesn't free or unref wrapper, so I followed its
 example. But, I'm not a gmime expert, and I agree intuitively that it
 should be freed somehow. Can anyone enlighten me?
 
  If replying to multiple messages (such as a whole thread) you get
  multiple sets of new headers. I think that probably is not what is
  wanted but its still better than the weird things the text version
  does. Might be worth putting a comment. [What I think should happen is
  that a union of all the headers from all these is taken throwing away
  duplicate addresses but that is obviously not part of this patch set]
 
 I've never been sure about what the intended behavior is when replying
 to multiple messages in the CLI. My thought was that it should create
 a reply to each message, so an MUA could iterate over them allowing
 you to compose replies to multiple messages. But, I've never wanted or
 used such a feature, so I'm agnostic on whether it's right. The emacs
 MUA (at least with my patch) ignores all but the first reply object in
 the array, my assumption being that reply only operates on multiple
 messages by accident.
 

In some cases, notmuch CLI insists that the search query matches exactly
one message (e.g. notmuch show for parts).  IMO the same behavior makes
sense for notmuch reply.

Regards,
  Dmitry

 Does anyone use reply with multiple messages? If so, what semantics do
 you expect?
 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: NEWS: add entries for the changes in the python bindings

[Tomi Ollila]

On Sun, 29 Jan 2012 18:08:50 +0100, Justus Winter 
4win...@informatik.uni-hamburg.de wrote:
 This patch series adds a section for the python binding changes to the
 NEWS file.

I marked patches 2/3 and 3/3 stale as git-am doesn't accept those anymore
on top of 2c6710e3ba22f5af6e5813dad8bee732e6c5d02c.

 
 Cheers,
 Justus

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

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

[Austin Clements]

Quoth Adam Wolfe Gordon on Jan 19 at 10:46 am:
 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.

Sorry for coming late to the party.  I really like this idea, but it
seems like your implementation is duplicating a lot of the work of
notmuch show.  This makes me wonder if it would be better to return
reply header information in the JSON (which is definitely the way to
go) but to fetch the part body from the UI via show (and maybe reuse
some of the show-mode logic, if it makes sense to do so).  If this has
already been discussed, just point me at the thread and I'll catch
myself up.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/2] cli: convert notmuch show to use the new argument parser

[Austin Clements]

Yikes.  I don't envy you this patch.  Two minor nits, otherwise this
and the next patch LGTM.

Quoth Jani Nikula on Feb 04 at 12:41 am:
 Use the new notmuch argument parser to handle arguments in notmuch
 show. There are two corner case functional changes:
 
 1) Also set params.raw = 1 when defaulting to raw format when part is
requested but format is not specified.

Huh.  So notmuch show --part=0 query was broken before.

 2) Do not set params.decrypt if crypto context creation fails.

Technically this also behaves differently if given multiple --format
arguments, but I'll let that slide.

 
 Signed-off-by: Jani Nikula j...@nikula.org
 ---
  notmuch-show.c |  153 
 +---
  1 files changed, 79 insertions(+), 74 deletions(-)
 
 diff --git a/notmuch-show.c b/notmuch-show.c
 index dec799c..f93e121 100644
 --- a/notmuch-show.c
 +++ b/notmuch-show.c
 @@ -1049,6 +1049,14 @@ do_show (void *ctx,
  return 0;
  }
  
 +enum {
 +NOTMUCH_FORMAT_NOT_SPECIFIED,
 +NOTMUCH_FORMAT_JSON,
 +NOTMUCH_FORMAT_TEXT,
 +NOTMUCH_FORMAT_MBOX,
 +NOTMUCH_FORMAT_RAW
 +};
 +
  int
  notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
  {
 @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), 
 unused (char *argv[]))
  notmuch_database_t *notmuch;
  notmuch_query_t *query;
  char *query_string;
 -char *opt;
 +int opt_index;
  const notmuch_show_format_t *format = format_text;
 -notmuch_show_params_t params;
 -int mbox = 0;
 -int format_specified = 0;
 -int i;
 +notmuch_show_params_t params = { .part = -1 };
 +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
 +notmuch_bool_t decrypt = FALSE;
 +notmuch_bool_t verify = FALSE;
 +notmuch_bool_t entire_thread = FALSE;
 +
 +notmuch_opt_desc_t options[] = {
 + { NOTMUCH_OPT_KEYWORD, format_sel, format, 'f',
 +   (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON },
 +   { text, NOTMUCH_FORMAT_TEXT },
 +   { mbox, NOTMUCH_FORMAT_MBOX },
 +   { raw, NOTMUCH_FORMAT_RAW },
 +   { 0, 0 } } },
 + { NOTMUCH_OPT_INT, params.part, part, 'p', 0 },
 + { NOTMUCH_OPT_BOOLEAN, entire_thread, entire-thread, 't', 0 },
 + { NOTMUCH_OPT_BOOLEAN, decrypt, decrypt, 'd', 0 },
 + { NOTMUCH_OPT_BOOLEAN, verify, verify, 'v', 0 },
 + { 0, 0, 0, 0, 0 }
 +};
 +
 +opt_index = parse_arguments (argc, argv, options, 1);
 +if (opt_index  0) {
 + /* diagnostics already printed */
 + return 1;
 +}
  
 -params.entire_thread = 0;
 -params.raw = 0;
 -params.part = -1;
 -params.cryptoctx = NULL;
 -params.decrypt = 0;
 +params.entire_thread = entire_thread;

If you make params.entire_thread a notmuch_bool_t (instead of an int),
you could pass params.entire_thread in the notmuch_opt_desc_t and get
rid of the local variable.

  
 -argc--; argv++; /* skip subcommand argument */
 +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) {
 + /* if part was requested and format was not specified, use format=raw */
 + if (params.part = 0)
 + format_sel = NOTMUCH_FORMAT_RAW;
 + else
 + format_sel = NOTMUCH_FORMAT_TEXT;
 +}
  
 -for (i = 0; i  argc  argv[i][0] == '-'; i++) {
 - if (strcmp (argv[i], --) == 0) {
 - i++;
 - break;
 +switch (format_sel) {
 +case NOTMUCH_FORMAT_JSON:
 + format = format_json;
 + params.entire_thread = 1;
 + break;
 +case NOTMUCH_FORMAT_TEXT:
 + format = format_text;
 + break;
 +case NOTMUCH_FORMAT_MBOX:
 + if (params.part  0) {
 + fprintf (stderr, Error: specifying parts is incompatible with mbox 
 output format.\n);
 + return 1;
   }
 - if (STRNCMP_LITERAL (argv[i], --format=) == 0) {
 - opt = argv[i] + sizeof (--format=) - 1;
 - if (strcmp (opt, text) == 0) {
 - format = format_text;
 - } else if (strcmp (opt, json) == 0) {
 - format = format_json;
 - params.entire_thread = 1;
 - } else if (strcmp (opt, mbox) == 0) {
 - format = format_mbox;
 - mbox = 1;
 - } else if (strcmp (opt, raw) == 0) {
 - format = format_raw;
 - params.raw = 1;
 - } else {
 - fprintf (stderr, Invalid value for --format: %s\n, opt);
 - return 1;
 - }
 - format_specified = 1;
 - } else if (STRNCMP_LITERAL (argv[i], --part=) == 0) {
 - params.part = atoi(argv[i] + sizeof (--part=) - 1);
 - } else if (STRNCMP_LITERAL (argv[i], --entire-thread) == 0) {
 - params.entire_thread = 1;
 - } else if ((STRNCMP_LITERAL (argv[i], --verify) == 0) ||
 -(STRNCMP_LITERAL (argv[i], --decrypt) == 0)) {
 - if (params.cryptoctx == NULL) {
 + format = 

Re: [PATCH] STYLE: Initial draft of coding style document

[Austin Clements]

Quoth David Bremner on Feb 03 at  8:50 pm:
 On Mon, 30 Jan 2012 18:01:15 +0200, Jani Nikula j...@nikula.org wrote:
  On Jan 30, 2012 4:38 PM, Tomi Ollila tomi.oll...@iki.fi wrote:
  
+
+* Indent is 4 spaces with mixed tabs/spaces and a tab width of 8.
+  Tabs should be only at the beginning of the line.
  
   So, after initial indentation (with tabs) there should not be further
   tabs? We'll be using the former instead of the latter in these 2 below?
 
 Ah, I only meant to explain what whitespace-mode complains about, namely
 
 spacespacespacespacetabx=1;
 
 as opposed to 
 
 tabspacespacespacespacetabx=1;
 
 I'm not sure it's that important, but apparently if it stays in the
 style guide it needs rewording.

Any indentation style this difficult to explain can't be a good idea.
How about,

* Indent is 4 spaces with mixed tab/spaces and a tab width of 8.
  (Specifically, a line should begin with zero or more tabs followed
  by fewer than eight spaces.)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

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

[Adam Wolfe Gordon]

On Sun, Feb 5, 2012 at 20:44, Austin Clements amdra...@mit.edu wrote:
 Sorry for coming late to the party.  I really like this idea, but it
 seems like your implementation is duplicating a lot of the work of
 notmuch show.  This makes me wonder if it would be better to return
 reply header information in the JSON (which is definitely the way to
 go) but to fetch the part body from the UI via show (and maybe reuse
 some of the show-mode logic, if it makes sense to do so).  If this has
 already been discussed, just point me at the thread and I'll catch
 myself up.

Thanks for taking a look. Dmitry noted on IRC that inlining the HTML
in JSON could cause issues with non-UTF8 character sets. Right now I'm
working on essentially what you've suggested - having the CLI produce
only headers, and then using show to get the quotable body.

Something else that was mentioned on IRC is using some of the notmuch
show logic to produce the show JSON format as part of reply. I looked
into this, but it would take some serious refactoring (to make the
show JSON stuff accessible to reply), and since emacs will need to end
up calling show anyway, I'm not sure it's worth it. I do like the idea
of different CLI commands being able to produce standardized formats
through some shared interface, I'm just not sure it's necessary here,
and not sure what the interface should look like.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: NEWS: add entries for the changes in the python bindings

[Tomi Ollila]

On Mon, 06 Feb 2012 00:21:58 -, Justus Winter 
4win...@informatik.uni-hamburg.de wrote:
 Quoting Tomi Ollila (2012-02-05 22:19:16)
 On Sun, 29 Jan 2012 18:08:50 +0100, Justus Winter 
 4win...@informatik.uni-hamburg.de wrote:
  This patch series adds a section for the python binding changes to the
  NEWS file.
 
 I marked patches 2/3 and 3/3 stale as git-am doesn't accept those anymore
 on top of 2c6710e3ba22f5af6e5813dad8bee732e6c5d02c.
 
 come on... these are patches for the NEWS file, it's as much work for
 me to merge these as for anyone else ;)

Yes I just happened to add the 'notmuch::patch' tag without checking
that the patches apply (and those appeared to ready list)... So I did
not have much choice for the time being...


 Justus

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

Re: newbie questions

[Tomi Ollila]

On Fri, 3 Feb 2012 12:59:34 +0100, Tamas Papp tkp...@gmail.com wrote:
 Hi,
 
 I just started using notmuch.  It is fascinating, but I still need to
 figure out a few things:
 
 1. How can I restrict searches (eg of my inbox) to the last few
 messages (eg 50-100) or some date (eg last 2 weeks)?  I am using the
 Emacs interface.

Currently, if you have GNU date you can run from command line:

date +%s.. --date '2 weeks ago'

Then paste the string 1327300096.. to the search field.

 2. Could someone point me to some guides on workflow with notmuch in
 Emacs?  So far I have been using mutt, and I guess I need to rethink a
 few things.

guides??? what guides??? ;) http://notmuchmail.org/ has some (old)
practices available...

 3. If I have multiple accounts, how can I change my e-mail address
 (From:) when I am writing messages (within emacs, using the m key).

This is something I've long been planning to look out but so far
my needs have been so small that the effort threshold has not been 
passed...

 Thanks,
 
 Tamas

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

Re: [PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[Jameson Graef Rollins]

Sorry to be so late on this, but I'm not a big fan of this new feature.
I would prefer to always see the subject (or any other field for that
matter) as is.

As a principle I would prefer there not be text replacements unless it's
very clear that text has been replaced.  Buttons work because it's clear
they're buttons.  This is not the case here, though, since this text
replacement could actually be confused with real text.

It's also not clear to me why this feature would be needed.  I have
never found blank subjects confusing.  The field is always clearly
delineated, at least in search and show mode.  If it's not clear
elsewhere, maybe we can make the delineation of the subject field
clearer, but leave the actual subject text string alone.

If some feel this feature is really needed we should at least have a
customization variable.  notmuch-unblank-subject?  I don't have any good
name suggestions, though.

jamie.


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

Re: [PATCH 2/2 v2] emacs: Prefer '[No Subject]' to blank subjects.

[David Edmondson]

On Sun, 05 Feb 2012 23:07:02 -0800, Jameson Graef Rollins 
jroll...@finestructure.net wrote:
 Sorry to be so late on this, but I'm not a big fan of this new feature.
 I would prefer to always see the subject (or any other field for that
 matter) as is.

The Emacs UI always replaced blank subjects with '[No Subject]' in
buffer names. The printing code also needed something other than a blank
subject for buffer renaming.

 As a principle I would prefer there not be text replacements unless it's
 very clear that text has been replaced.  Buttons work because it's clear
 they're buttons.  This is not the case here, though, since this text
 replacement could actually be confused with real text.
 
 It's also not clear to me why this feature would be needed.  I have
 never found blank subjects confusing.  The field is always clearly
 delineated, at least in search and show mode.  If it's not clear
 elsewhere, maybe we can make the delineation of the subject field
 clearer, but leave the actual subject text string alone.
 
 If some feel this feature is really needed we should at least have a
 customization variable.  notmuch-unblank-subject?  I don't have any good
 name suggestions, though.

Updating `notmuch-prettify-subject' to use a user configurable string
(that can be set to the empty string) sounds like a good idea. Please
ensure that the various other bits of code that require something other
than a blank subject still work properly.


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

Re: [PATCH 1/2] cli: convert notmuch show to use the new argument parser

[Jani Nikula]

On Sun, 5 Feb 2012 23:12:05 -0500, Austin Clements amdra...@mit.edu wrote:
 Yikes.  I don't envy you this patch.  Two minor nits, otherwise this
 and the next patch LGTM.

Thanks for the review. I'll roll another version, I think it will be
better with your suggestions incorporated.

 Quoth Jani Nikula on Feb 04 at 12:41 am:
  Use the new notmuch argument parser to handle arguments in notmuch
  show. There are two corner case functional changes:
  
  1) Also set params.raw = 1 when defaulting to raw format when part is
 requested but format is not specified.
 
 Huh.  So notmuch show --part=0 query was broken before.

Hmm, yes, it seems so. Do you think I should make this a separate fix?

BTW an alternative would be to require setting --format if --part is
specified, and not adapt the default format depending on --part.

  2) Do not set params.decrypt if crypto context creation fails.
 
 Technically this also behaves differently if given multiple --format
 arguments, but I'll let that slide.

Ugh, right, the old parsing was broken also that way. Luckily that falls
in the corner case department too.

  
  Signed-off-by: Jani Nikula j...@nikula.org
  ---
   notmuch-show.c |  153 
  +---
   1 files changed, 79 insertions(+), 74 deletions(-)
  
  diff --git a/notmuch-show.c b/notmuch-show.c
  index dec799c..f93e121 100644
  --- a/notmuch-show.c
  +++ b/notmuch-show.c
  @@ -1049,6 +1049,14 @@ do_show (void *ctx,
   return 0;
   }
   
  +enum {
  +NOTMUCH_FORMAT_NOT_SPECIFIED,
  +NOTMUCH_FORMAT_JSON,
  +NOTMUCH_FORMAT_TEXT,
  +NOTMUCH_FORMAT_MBOX,
  +NOTMUCH_FORMAT_RAW
  +};
  +
   int
   notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
   {
  @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), 
  unused (char *argv[]))
   notmuch_database_t *notmuch;
   notmuch_query_t *query;
   char *query_string;
  -char *opt;
  +int opt_index;
   const notmuch_show_format_t *format = format_text;
  -notmuch_show_params_t params;
  -int mbox = 0;
  -int format_specified = 0;
  -int i;
  +notmuch_show_params_t params = { .part = -1 };
  +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED;
  +notmuch_bool_t decrypt = FALSE;
  +notmuch_bool_t verify = FALSE;
  +notmuch_bool_t entire_thread = FALSE;
  +
  +notmuch_opt_desc_t options[] = {
  +   { NOTMUCH_OPT_KEYWORD, format_sel, format, 'f',
  + (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON },
  + { text, NOTMUCH_FORMAT_TEXT },
  + { mbox, NOTMUCH_FORMAT_MBOX },
  + { raw, NOTMUCH_FORMAT_RAW },
  + { 0, 0 } } },
  +   { NOTMUCH_OPT_INT, params.part, part, 'p', 0 },
  +   { NOTMUCH_OPT_BOOLEAN, entire_thread, entire-thread, 't', 0 },
  +   { NOTMUCH_OPT_BOOLEAN, decrypt, decrypt, 'd', 0 },
  +   { NOTMUCH_OPT_BOOLEAN, verify, verify, 'v', 0 },
  +   { 0, 0, 0, 0, 0 }
  +};
  +
  +opt_index = parse_arguments (argc, argv, options, 1);
  +if (opt_index  0) {
  +   /* diagnostics already printed */
  +   return 1;
  +}
   
  -params.entire_thread = 0;
  -params.raw = 0;
  -params.part = -1;
  -params.cryptoctx = NULL;
  -params.decrypt = 0;
  +params.entire_thread = entire_thread;
 
 If you make params.entire_thread a notmuch_bool_t (instead of an int),
 you could pass params.entire_thread in the notmuch_opt_desc_t and get
 rid of the local variable.

You're right; I was a bit lazy and didn't consider changing
notmuch_show_params_t. I'll change both this and params.decrypt to
notmuch_bool_t.

 
   
  -argc--; argv++; /* skip subcommand argument */
  +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) {
  +   /* if part was requested and format was not specified, use format=raw */
  +   if (params.part = 0)
  +   format_sel = NOTMUCH_FORMAT_RAW;
  +   else
  +   format_sel = NOTMUCH_FORMAT_TEXT;
  +}
   
  -for (i = 0; i  argc  argv[i][0] == '-'; i++) {
  -   if (strcmp (argv[i], --) == 0) {
  -   i++;
  -   break;
  +switch (format_sel) {
  +case NOTMUCH_FORMAT_JSON:
  +   format = format_json;
  +   params.entire_thread = 1;
  +   break;
  +case NOTMUCH_FORMAT_TEXT:
  +   format = format_text;
  +   break;
  +case NOTMUCH_FORMAT_MBOX:
  +   if (params.part  0) {
  +   fprintf (stderr, Error: specifying parts is incompatible with mbox 
  output format.\n);
  +   return 1;
  }
  -   if (STRNCMP_LITERAL (argv[i], --format=) == 0) {
  -   opt = argv[i] + sizeof (--format=) - 1;
  -   if (strcmp (opt, text) == 0) {
  -   format = format_text;
  -   } else if (strcmp (opt, json) == 0) {
  -   format = format_json;
  -   params.entire_thread = 1;
  -   } else if (strcmp (opt, mbox) == 0) {
  -   format = format_mbox;
  -   mbox = 1;