On disk tag storage format

[Ethan Glasser-Camp]

Ethan Glasser-Camp  writes:

> I've modified the script so that it would run by mangling filenames,
> which is irreversible (the original tried to encode/decode filenames
> reversibly). Then I got a little carried away, adding --verbose and
> --dry-run options as well as removing a couple trailing
> semicolons. Here's my version, in case it should interest anyone else.

Hi guys,

There was a bug in the previous version I sent. It didn't handle
unlinking tags correctly. Also, I spotted a bug in syncing to untagged
messages. Maybe I should stop using emails as version control.

 8< 


-- next part --
A non-text attachment was scrubbed...
Name: linksync.py
Type: text/x-python
Size: 6574 bytes
Desc: slightly more tested this time
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20131007/ce212066/attachment-0002.py>
-- next part --

 8< 

Of course, the next step is to sync using this mechanism. Rsync doesn't
really have a concept of history, which basically makes it unusable for
this purpose [1]. Unison doesn't really understand renames, so it gets
confused when you mark a message as read (which might move it from new
to cur, and definitely changes its tags). Bremner suggested
syncmaildir. Syncmaildir doesn't understand links at all. Bremner
suggested that we could use some parts of syncmaildir to implement the
tag syncing we need.

I didn't have anything else going on this weekend so I tried to
prototype the approach. It turns out to be possible to leverage some
parts of syncmaildir. I translated a bunch of smd-client into a new
program, tagsync-client, that links to messages in an existing notmuch
DB. It seems like it's possible to use it in place of the existing
smd-client by putting lines like this in your config:

SMDCLIENT=~/src/tagsync.git/tagsync-client.py
REMOTESMDCLIENT=~/src/tagsync.git/tagsync-client.py

The sequence of commands I ran:

- linksync.py to dump tags to ~/Mail/.notmuch/exported-tags
- smd-pull mail to sync ~/Mail but excluding .notmuch
- notmuch new
- smd-pull tagsync (using the above client) to sync 
~/Mail/.notmuch/exported-tags
- linksync.py to pull tags from ~/Mail/.notmuch/exported-tags

syncmaildir doesn't cope well with drafts, so it might choke on that,
and it doesn't like symlinks (it thinks they're always to directories),
so be sure to run linksync with -l hard.

Here's the script. It's a work in progress; I have only tested it once in one 
direction.

 8< 

-- next part --
A non-text attachment was scrubbed...
Name: tagsync-client.py
Type: text/x-python
Size: 19931 bytes
Desc: client script
URL: 
<http://notmuchmail.org/pipermail/notmuch/attachments/20131007/ce212066/attachment-0003.py>

Re: On disk tag storage format

[Ethan Glasser-Camp]

Ethan Glasser-Camp ethan.glasser.c...@gmail.com writes:

 I've modified the script so that it would run by mangling filenames,
 which is irreversible (the original tried to encode/decode filenames
 reversibly). Then I got a little carried away, adding --verbose and
 --dry-run options as well as removing a couple trailing
 semicolons. Here's my version, in case it should interest anyone else.

Hi guys,

There was a bug in the previous version I sent. It didn't handle
unlinking tags correctly. Also, I spotted a bug in syncing to untagged
messages. Maybe I should stop using emails as version control.

 8 


# Copyright 2013, David Bremner da...@tethera.net

# Licensed under the same terms as notmuch.

import notmuch
import re
import os, errno
import sys
from collections import defaultdict
import argparse
import hashlib

# skip automatic and maildir tags

skiptags = re.compile(r^(attachement|signed|encrypted|draft|flagged|passed|replied|unread)$)

# some random person on stack overflow suggests:

def mkdir_p(path):
try:
os.makedirs(path)
except OSError as exc: # Python 2.5
if exc.errno == errno.EEXIST and os.path.isdir(path):
pass
else: raise

VERBOSE = False

def log(msg):
if VERBOSE:
print(msg)

CHARSET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+_@=.,-'

encode_re = '([^{0}])'.format(CHARSET)

decode_re = '[%]([0-7][0-9A-Fa-f])'

def encode_one_char(match):
return('%{:02x}'.format(ord(match.group(1

def encode_for_fs(str):
return re.sub(encode_re,encode_one_char, str,0)

def mangle_message_id(msg_id):

Return a mangled version of the message id, suitable for use as a filename.

MAX_LENGTH = 143
FLAGS_LENGTH = 8# :2,S...??
encoded = encode_for_fs(msg_id)
if len(encoded)  MAX_LENGTH - FLAGS_LENGTH:
return encoded

SHA_LENGTH = 8
TRUNCATED_ID_LENGTH = MAX_LENGTH - SHA_LENGTH - FLAGS_LENGTH
PREFIX_LENGTH = SUFFIX_LENGTH = (TRUNCATED_ID_LENGTH - 3) // 2
prefix = encoded[:PREFIX_LENGTH]
suffix = encoded[-SUFFIX_LENGTH:]
sha = hashlib.sha256()
sha.update(encoded)
return prefix + '...' + suffix + sha.hexdigest()[:SHA_LENGTH]

def decode_one_char(match):
return chr(int(match.group(1),16))

def decode_from_fs(str):
return re.sub(decode_re,decode_one_char, str, 0)

def mk_tag_dir(tagdir):

mkdir_p (os.path.join(tagdir, 'cur'))
mkdir_p (os.path.join(tagdir, 'new'))
mkdir_p (os.path.join(tagdir, 'tmp'))


flagpart = '(:2,[^:]*)'
flagre = re.compile(flagpart + '$');

def path_for_msg (dir, msg):
filename = msg.get_filename()
flagsmatch = flagre.search(filename)
if flagsmatch == None:
flags = ''
else:
flags = flagsmatch.group(1)

return os.path.join(dir, 'cur', mangle_message_id(msg.get_message_id()) + flags)


def unlink_message(dir, msg):

dir = os.path.join(dir, 'cur')

mangled_id = mangle_message_id(msg.get_message_id())
filepattern = '^' + re.escape(mangled_id)  + flagpart +'?$'

filere = re.compile(filepattern)

for file in os.listdir(dir):
if filere.match(file):
log(Unlinking {}.format(os.path.join(dir, file)))
if not opts.dry_run:
os.unlink(os.path.join(dir, file))

def dir_for_tag(tag):
enc_tag = encode_for_fs (tag)
return os.path.join(tagroot, enc_tag)

disk_tags = defaultdict(set)
disk_ids = set()

def read_tags_from_disk(rootdir):

for root, subFolders, files in os.walk(rootdir):
for filename in files:
mangled_id = filename.split(':')[0]
tag = root.split('/')[-2]
disk_ids.add(mangled_id)
disk_tags[mangled_id].add(decode_from_fs(tag))

# Main program

parser = argparse.ArgumentParser(description='Sync notmuch tag database to/from link farm')
parser.add_argument('-l','--link-style',choices=['hard','symbolic', 'adaptive'],
default='adaptive')
parser.add_argument('-d','--destination',choices=['disk','notmuch'], default='disk')
parser.add_argument('-t','--threshold', default=5L, type=int)
parser.add_argument('-n','--dry-run', default=False, action='store_true')
parser.add_argument('-v','--verbose', default=False, action='store_true')

parser.add_argument('tagroot')

opts=parser.parse_args()
VERBOSE = opts.verbose

tagroot=opts.tagroot

sync_from_links = (opts.destination == 'notmuch')

read_tags_from_disk(tagroot)

if sync_from_links:
db = notmuch.Database(mode=notmuch.Database.MODE.READ_WRITE)
else:
db = notmuch.Database(mode=notmuch.Database.MODE.READ_ONLY)

dbtags = filter (lambda tag: not skiptags.match(tag), db.get_all_tags())

if sync_from_links:
# have to iterate over even untagged messages
querystr = '*'
else:
querystr = ' OR '.join(map (lambda tag: 'tag:'+tag,  dbtags))

q_new = notmuch.Query(db, querystr)
q_new.set_sort(notmuch.Query.SORT.UNSORTED)
for msg in q_new.search_messages():

# silently ignore

On disk tag storage format

[Ethan Glasser-Camp]

David Bremner  writes:

> It's still a prototype, and there is not much error checking, and there
> are certain issues not dealt with at all (the ones I thought about are
> commented).

Hi everyone,

I'm very interested in running notmuch on all my laptops and having my
mail and its tags be synchronized for me, so at Bremner's direction on
IRC, I played around with this script a little. At first it wouldn't run
on my computer; the script uses message IDs as filenames, which can be
quite long, whereas I keep my mail in my $HOME, which is on an ecryptfs
filesystem, and has a filename limit of 143 characters.

I've modified the script so that it would run by mangling filenames,
which is irreversible (the original tried to encode/decode filenames
reversibly). Then I got a little carried away, adding --verbose and
--dry-run options as well as removing a couple trailing
semicolons. Here's my version, in case it should interest anyone else.

-- next part --
A non-text attachment was scrubbed...
Name: linksync.py
Type: text/x-python
Size: 6434 bytes
Desc: linksync.py
URL: 

Re: On disk tag storage format

[Ethan Glasser-Camp]

David Bremner da...@tethera.net writes:

 It's still a prototype, and there is not much error checking, and there
 are certain issues not dealt with at all (the ones I thought about are
 commented).

Hi everyone,

I'm very interested in running notmuch on all my laptops and having my
mail and its tags be synchronized for me, so at Bremner's direction on
IRC, I played around with this script a little. At first it wouldn't run
on my computer; the script uses message IDs as filenames, which can be
quite long, whereas I keep my mail in my $HOME, which is on an ecryptfs
filesystem, and has a filename limit of 143 characters.

I've modified the script so that it would run by mangling filenames,
which is irreversible (the original tried to encode/decode filenames
reversibly). Then I got a little carried away, adding --verbose and
--dry-run options as well as removing a couple trailing
semicolons. Here's my version, in case it should interest anyone else.

# Copyright 2013, David Bremner da...@tethera.net

# Licensed under the same terms as notmuch.

import notmuch
import re
import os, errno
import sys
from collections import defaultdict
import argparse
import hashlib

# skip automatic and maildir tags

skiptags = re.compile(r^(attachement|signed|encrypted|draft|flagged|passed|replied|unread)$)

# some random person on stack overflow suggests:

def mkdir_p(path):
try:
os.makedirs(path)
except OSError as exc: # Python 2.5
if exc.errno == errno.EEXIST and os.path.isdir(path):
pass
else: raise

VERBOSE = False

def log(msg):
if VERBOSE:
print(msg)

CHARSET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+_@=.,-'

encode_re = '([^{0}])'.format(CHARSET)

decode_re = '[%]([0-7][0-9A-Fa-f])'

def encode_one_char(match):
return('%{:02x}'.format(ord(match.group(1

def encode_for_fs(str):
return re.sub(encode_re,encode_one_char, str,0)

def mangle_message_id(msg_id):

Return a mangled version of the message id, suitable for use as a filename.

MAX_LENGTH = 143
FLAGS_LENGTH = 8# :2,S...??
encoded = encode_for_fs(msg_id)
if len(encoded)  MAX_LENGTH - FLAGS_LENGTH:
return encoded

SHA_LENGTH = 8
TRUNCATED_ID_LENGTH = MAX_LENGTH - SHA_LENGTH - FLAGS_LENGTH
PREFIX_LENGTH = SUFFIX_LENGTH = (TRUNCATED_ID_LENGTH - 3) // 2
prefix = encoded[:PREFIX_LENGTH]
suffix = encoded[-SUFFIX_LENGTH:]
sha = hashlib.sha256()
sha.update(encoded)
return prefix + '...' + suffix + sha.hexdigest()[:SHA_LENGTH]

def decode_one_char(match):
return chr(int(match.group(1),16))

def decode_from_fs(str):
return re.sub(decode_re,decode_one_char, str, 0)

def mk_tag_dir(tagdir):

mkdir_p (os.path.join(tagdir, 'cur'))
mkdir_p (os.path.join(tagdir, 'new'))
mkdir_p (os.path.join(tagdir, 'tmp'))


flagpart = '(:2,[^:]*)'
flagre = re.compile(flagpart + '$')

def path_for_msg (dir, msg):
filename = msg.get_filename()
flagsmatch = flagre.search(filename)
if flagsmatch == None:
flags = ''
else:
flags = flagsmatch.group(1)

return os.path.join(dir, 'cur', mangle_message_id(msg.get_message_id()) + flags)


def unlink_message(dir, msg):

dir = os.path.join(dir, 'cur')

filepattern = mangle_filename_for_fs(msg.get_message_id())  + flagpart +'?$'

filere = re.compile(filepattern)

for file in os.listdir(dir):
if filere.match(file):
log(Unlinking {}.format(os.path.join(dir, file)))
if not opts.dry_run:
os.unlink(os.path.join(dir, file))

def dir_for_tag(tag):
enc_tag = encode_for_fs (tag)
return os.path.join(tagroot, enc_tag)

disk_tags = defaultdict(set)
disk_ids = set()

def read_tags_from_disk(rootdir):

for root, subFolders, files in os.walk(rootdir):
for filename in files:
mangled_id = filename.split(':')[0]
tag = root.split('/')[-2]
disk_ids.add(mangled_id)
disk_tags[mangled_id].add(decode_from_fs(tag))

# Main program

parser = argparse.ArgumentParser(description='Sync notmuch tag database to/from link farm')
parser.add_argument('-l','--link-style',choices=['hard','symbolic', 'adaptive'],
default='adaptive')
parser.add_argument('-d','--destination',choices=['disk','notmuch'], default='disk')
parser.add_argument('-t','--threshold', default=5L, type=int)
parser.add_argument('-n','--dry-run', default=False, action='store_true')
parser.add_argument('-v','--verbose', default=False, action='store_true')

parser.add_argument('tagroot')

opts=parser.parse_args()
VERBOSE = opts.verbose

tagroot=opts.tagroot

sync_from_links = (opts.destination == 'notmuch')

read_tags_from_disk(tagroot)

if sync_from_links:
db = notmuch.Database(mode=notmuch.Database.MODE.READ_WRITE)
else:
db = notmuch.Database(mode=notmuch.Database.MODE.READ_ONLY)

dbtags = filter (lambda tag: not skiptags.match(tag), db.get_all_tags())

Notmuch scripts (again), now with more usenet

[Ethan Glasser-Camp]

Jan Pobrislo  writes:

> Hi! I was having some hardware issues and had to migrate the site. It should
> be all up again and ready for inclusion.
>
> Added stuff from last time:
> * source function & actions for zaw (https://github.com/zsh-users/zaw)
> * LICENSE (CC0)

Hi! Sorry for the delay, real life got in the way.

In previous emails, David Bremner said he'd like someone with Python
expertise to look at notmuch-new.py. He notes that pylint goes
ballistic, but most of these errors don't bother me much (complaints
about variable naming and missing docstrings).

As for the rest, I'm not a zsh expert, but I looked at zmuch and
notmuch-colorize and they looked fine.

If Bremner is willing to put this package in contrib, I think he should
do so.

Ethan

[PATCH v2] emacs: display tags in notmuch-show with links

[Ethan Glasser-Camp]

Damien Cassou  writes:

> This patch obsoletes
> id:1352565719-12397-1-git-send-email-damien.cassou at gmail.com
>
> [PATCH 1/4] emacs: Add a thread's tags to emacs header-line
> [PATCH 2/4] emacs: Make tags in header-line clickable
> [PATCH 3/4] emacs: Make tags that appear in `notmuch-show' clickable
> [PATCH 4/4] emacs: Add unit-tests for clickable tags in notmuch-show
>
> This patch makes clickable all tags that appear in notmuch-show
> buffers. Each tag is a link to open a new notmuch-search buffer for
> this tag. Additionally, the buffer's header-line now shows the
> thread's tags (clickable only if the `header-button' library is loaded
> or loadable).

Looks fine to me. Let me just get the notes from my bikeshed, in case
you get asked to roll another version :)

- You might want to use #' on lambdas.

- It bothers me how similar notmuch-tagger-{body,header}-button-action
  are. I thought it might be better to unify them by seeing what type
  the button argument was. Here's my (untested) approach which you might
  find prettier or uglier.

(notmuch-tagger-all-button-get (button attrib)
  "Utility function to do button-get on different kinds of buttons."
  (cond
((integer-or-marker-p button)
  (button-get button attrib))
((and (featurep 'header-button)
  (listp button))
  (header-button-get button attrib))
(t (error "unknown type of button %s" button))

- The comment for notmuch-tagger-make-body-link reads that it will work
  "everywhere except in the header-line". Does this mean mode-line, menu
  bar, or what? How about just "won't work in the header-line"?

- In patch 3:

 +If tags the result of this function is to be used within the

I think this should just read "If the result".

Ethan

Add new dump/restore format and batch tagging.

[Ethan Glasser-Camp]

Ethan Glasser-Camp  writes:

> - Patch 4 still has a subject line that ends in a period. I don't think
>   this is mandatory for everyone but some people consider it best
>   practice.

Best practice, of course, would be to remove the period at the end of
the subject line. Patch 12 also has one.

Ethan

Add new dump/restore format and batch tagging.

[Ethan Glasser-Camp]

david at tethera.net writes:

> which was revied by Tomi and Ethan. I think I implemented their
> suggestions.

Actually, I don't think you implemented all of mine.

- Patch 4 still has a subject line that ends in a period. I don't think
  this is mandatory for everyone but some people consider it best
  practice. You still have the spelling "seperate" (also, patch 7 has
  "seperated").

- In patch 4, I still think this would look better if you switched the
  branches.

+if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
+   _notmuch_message_add_term (message, "type", "mail");
+} else {
+   return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
+}

- In patch 5, I still think this looks funny:

+   int num_tags = random () % (max_tags + 1);
+   int this_mid_len = random () % message_id_len + 1;

Additionally, I would like a check that message_id_len is reasonable
(more than 1, say).

- Patch 8:

+int
+parse_tag_stream (void *ctx,
+ notmuch_database_t *notmuch,
+ FILE *input,
+ tag_callback_t callback,
+ tag_op_flag_t flags,
+ volatile sig_atomic_t *interrupted);

Am I going crazy, or does this function not get implemented?

- Patch 11: DUMP_FORMAT can change from DUMP_FORMAT_BATCH_TAG to
  DUMP_FORMAT_SUP if a paren is anywhere on the first line. I'd prefer
  this only happens if we have DUMP_FORMAT_AUTO.

You probably want to move the comment "Dump output is..." closer to the
regex.

I don't see why it's necessary to have this:

+   query_string = query_string + 3;

- Patch 13:

+cp /dev/null EXPECTED.$test_count
+notmuch dump --format=batch-tag -- from:cworth |\
+awk "{ print \"+$enc1 +$enc2 +$enc3 -- \" \$5 }" > EXPECTED.$test_count

What's the purpose of the CP here? It just creates an empty file. You
could do it with touch. Why even bother since you're going to fill it
with stuff in a second? Actually, care to explain the dump and sed call?
It looks like you're using this dump to encode the message IDs. If
format=batch-tag skips a message for some reason or terminates early,
the test won't fail.

- Patch 16:

+message-ids may contained arbitrary non-null characters. Note also

I think this should be "may contain", or something else entirely if
you're trying to describe past behavior of sup?

Ethan

[PATCH 2/2] emacs: less guessing of character set in messages

[Ethan Glasser-Camp]

Tomi Ollila  writes:

> The macro with-current-notmuch-show-message executes command
> `notmuch show --format=raw id:...` which just outputs the contents
> of the mail file verbatim (into temporary buffer). In case e.g. utf-8
> locale is used the temporary buffer has buffer-file-coding-system as
> utf-8. In this case Emacs converts the data to multibyte format, guessing
> that input is in utf-8.
> However, the "raw" (MIME) message may contain octet data in any other
> 8bit format, and as no (MIME-)content spesific handling to the message
> is done at this point, conversion to other formats may lose information.
> By setting coding-system-for-read 'no-conversion drops the conversion part
> and makes this handle input as notmuch-get-bodypart-internal() does.
> This marks the broken test in previous change fixed.

This looks good to me, though you might need to apply it with notmuch
show --format="mbox" :).

Ethan

[BUG] Saving attachments containing UTF-8 chars

[Ethan Glasser-Camp]

Ethan Glasser-Camp  writes:

> Writing this buffer using C-x C-w encodes it correctly too. So I think
> this is an emacs MIME problem. We call mm-save-part, which calls
> mm-save-part-to-file, which calls mm-with-unibyte-buffer. Hmm..
>
> Indeed, it seems that inserting this character into a file that's been
> marked "unibyte" using (set-buffer-multibyte nil) turns it into the ^Y
> character (ASCII code 0x19 -- the character that comes out in the patch
> file). There's probably a technical reason that this should be true, but
> I can't think of why that would be.

The more I think about this, the more convinced I become that this is a
bug in emacs's multibyte handling. I've filed a bug, see:

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12925

But I think Tomi's going to formalize his hacky patch and send that out
later too. When he does, it has my +1.

Ethan

[BUG] Saving attachments containing UTF-8 chars

[Ethan Glasser-Camp]

Tomi Ollila  writes:

> I can verify this bug: I copied 'rawmail' to my mail store and attempted
> to 'w' the attacment and got the same result (after notmuch new).
>
> The saving code first does
> notmuch show --format=raw id:"508953E6.70006 at gmail.com"
> which decodes OK on command line, and to the buffer when 
> kill-buffer is outcommented in (with-current-notmuch-show-message ...) 
> macro.

I was able to see this behavior, and Tomi did a good job tracking down
where it was :)

I even see the bytes as presented in the file. When moving point to the
problematic character, and doing M-x describe-char, it says:

  buffer code: #xE2 #x80 #x99
file code: #xE2 #x80 #x99 (encoded by coding system utf-8)

buffer-file-coding-system is, of course, utf-8.

Writing this buffer using C-x C-w encodes it correctly too. So I think
this is an emacs MIME problem. We call mm-save-part, which calls
mm-save-part-to-file, which calls mm-with-unibyte-buffer. Hmm..

Indeed, it seems that inserting this character into a file that's been
marked "unibyte" using (set-buffer-multibyte nil) turns it into the ^Y
character (ASCII code 0x19 -- the character that comes out in the patch
file). There's probably a technical reason that this should be true, but
I can't think of why that would be.

> I attempted a set of trial-&-error tricks to get the attachment
> saved "correctly", and at least this seems to do the trick:
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index f273eb4..a6a85c0 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -203,9 +203,11 @@ For example, if you wanted to remove an \"unread\" tag 
> and add a
>   (let ((id (notmuch-show-get-message-id)))
> (let ((buf (generate-new-buffer (concat "*notmuch-msg-" id "*"
>   (with-current-buffer buf
> - (call-process notmuch-command nil t nil "show" "--format=raw" id)
> -   , at body)
> -  (kill-buffer buf)
> +(let ((coding-system-for-read 'no-conversion)
> +  (coding-system-for-write 'no-conversion))
> +  (call-process notmuch-command nil t nil "show" "--format=raw" id)
> +  , at body))
> +%%(kill-buffer buf)
[snip]
> (kill-buffer is outcommented above for testing purposes)
>
> To test this this needs to me evaluated and then the functions
> using this macro (notmuch-show-save-attachments  in this case)
>
> Smart suggestions for proper fix ?

Well, we could limit it just to saving attachments (putting the let
around the with-current-notmuch-show-message). That feels like it could
be right, because intuitively saving an attachment should be done
without any conversions. Or even the above doesn't seem so bad. My vague
feeling is that messages should always be ASCII, or at least mm-* will
interpret it that way, so decoding them into any other character set
might cause problems. Anyone understand character sets?

Ethan

Re: [BUG] Saving attachments containing UTF-8 chars

[Ethan Glasser-Camp]

Ethan Glasser-Camp ethan.glasser.c...@gmail.com writes:

 Writing this buffer using C-x C-w encodes it correctly too. So I think
 this is an emacs MIME problem. We call mm-save-part, which calls
 mm-save-part-to-file, which calls mm-with-unibyte-buffer. Hmm..

 Indeed, it seems that inserting this character into a file that's been
 marked unibyte using (set-buffer-multibyte nil) turns it into the ^Y
 character (ASCII code 0x19 -- the character that comes out in the patch
 file). There's probably a technical reason that this should be true, but
 I can't think of why that would be.

The more I think about this, the more convinced I become that this is a
bug in emacs's multibyte handling. I've filed a bug, see:

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=12925

But I think Tomi's going to formalize his hacky patch and send that out
later too. When he does, it has my +1.

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

Re: Add new dump/restore format and batch tagging.

[Ethan Glasser-Camp]

da...@tethera.net writes:

 which was revied by Tomi and Ethan. I think I implemented their
 suggestions.

Actually, I don't think you implemented all of mine.

- Patch 4 still has a subject line that ends in a period. I don't think
  this is mandatory for everyone but some people consider it best
  practice. You still have the spelling seperate (also, patch 7 has
  seperated).

- In patch 4, I still think this would look better if you switched the
  branches.

+if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
+   _notmuch_message_add_term (message, type, mail);
+} else {
+   return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
+}

- In patch 5, I still think this looks funny:

+   int num_tags = random () % (max_tags + 1);
+   int this_mid_len = random () % message_id_len + 1;

Additionally, I would like a check that message_id_len is reasonable
(more than 1, say).

- Patch 8:

+int
+parse_tag_stream (void *ctx,
+ notmuch_database_t *notmuch,
+ FILE *input,
+ tag_callback_t callback,
+ tag_op_flag_t flags,
+ volatile sig_atomic_t *interrupted);

Am I going crazy, or does this function not get implemented?

- Patch 11: DUMP_FORMAT can change from DUMP_FORMAT_BATCH_TAG to
  DUMP_FORMAT_SUP if a paren is anywhere on the first line. I'd prefer
  this only happens if we have DUMP_FORMAT_AUTO.

You probably want to move the comment Dump output is... closer to the
regex.

I don't see why it's necessary to have this:

+   query_string = query_string + 3;

- Patch 13:

+cp /dev/null EXPECTED.$test_count
+notmuch dump --format=batch-tag -- from:cworth |\
+awk { print \+$enc1 +$enc2 +$enc3 -- \ \$5 }  EXPECTED.$test_count

What's the purpose of the CP here? It just creates an empty file. You
could do it with touch. Why even bother since you're going to fill it
with stuff in a second? Actually, care to explain the dump and sed call?
It looks like you're using this dump to encode the message IDs. If
format=batch-tag skips a message for some reason or terminates early,
the test won't fail.

- Patch 16:

+message-ids may contained arbitrary non-null characters. Note also

I think this should be may contain, or something else entirely if
you're trying to describe past behavior of sup?

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

Re: Add new dump/restore format and batch tagging.

[Ethan Glasser-Camp]

Ethan Glasser-Camp ethan.glasser.c...@gmail.com writes:

 - Patch 4 still has a subject line that ends in a period. I don't think
   this is mandatory for everyone but some people consider it best
   practice.

Best practice, of course, would be to remove the period at the end of
the subject line. Patch 12 also has one.

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

Re: [PATCH v2] emacs: display tags in notmuch-show with links

[Ethan Glasser-Camp]

Damien Cassou damien.cas...@gmail.com writes:

 This patch obsoletes
 id:1352565719-12397-1-git-send-email-damien.cas...@gmail.com

 [PATCH 1/4] emacs: Add a thread's tags to emacs header-line
 [PATCH 2/4] emacs: Make tags in header-line clickable
 [PATCH 3/4] emacs: Make tags that appear in `notmuch-show' clickable
 [PATCH 4/4] emacs: Add unit-tests for clickable tags in notmuch-show

 This patch makes clickable all tags that appear in notmuch-show
 buffers. Each tag is a link to open a new notmuch-search buffer for
 this tag. Additionally, the buffer's header-line now shows the
 thread's tags (clickable only if the `header-button' library is loaded
 or loadable).

Looks fine to me. Let me just get the notes from my bikeshed, in case
you get asked to roll another version :)

- You might want to use #' on lambdas.

- It bothers me how similar notmuch-tagger-{body,header}-button-action
  are. I thought it might be better to unify them by seeing what type
  the button argument was. Here's my (untested) approach which you might
  find prettier or uglier.

(notmuch-tagger-all-button-get (button attrib)
  Utility function to do button-get on different kinds of buttons.
  (cond
((integer-or-marker-p button)
  (button-get button attrib))
((and (featurep 'header-button)
  (listp button))
  (header-button-get button attrib))
(t (error unknown type of button %s button))

- The comment for notmuch-tagger-make-body-link reads that it will work
  everywhere except in the header-line. Does this mean mode-line, menu
  bar, or what? How about just won't work in the header-line?

- In patch 3:

 +If tags the result of this function is to be used within the

I think this should just read If the result.

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

Re: Notmuch scripts (again), now with more usenet

[Ethan Glasser-Camp]

Jan Pobrislo c...@webprojekty.cz writes:

 Hi! I was having some hardware issues and had to migrate the site. It should
 be all up again and ready for inclusion.

 Added stuff from last time:
 * source function  actions for zaw (https://github.com/zsh-users/zaw)
 * LICENSE (CC0)

Hi! Sorry for the delay, real life got in the way.

In previous emails, David Bremner said he'd like someone with Python
expertise to look at notmuch-new.py. He notes that pylint goes
ballistic, but most of these errors don't bother me much (complaints
about variable naming and missing docstrings).

As for the rest, I'm not a zsh expert, but I looked at zmuch and
notmuch-colorize and they looked fine.

If Bremner is willing to put this package in contrib, I think he should
do so.

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

[PATCH 2/2] test: add nontrivial test for restore --accumulate.

[Ethan Glasser-Camp]

david at tethera.net writes:

> From: David Bremner 
>
> It seems we have never tested the case that restore --accumulate
> actually adds tags. I noticed this when I started optimizing and no
> tests failed.
>
> The bracketing with "restore --input=dump.expected" are to make sure
> we start in a known state, and we leave the database in a known state
> for the next test.

OK, these LGTM.

Ethan

Re: [PATCH 2/2] test: add nontrivial test for restore --accumulate.

[Ethan Glasser-Camp]

da...@tethera.net writes:

 From: David Bremner brem...@debian.org

 It seems we have never tested the case that restore --accumulate
 actually adds tags. I noticed this when I started optimizing and no
 tests failed.

 The bracketing with restore --input=dump.expected are to make sure
 we start in a known state, and we leave the database in a known state
 for the next test.

OK, these LGTM.

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

Re: [BUG] Saving attachments containing UTF-8 chars

[Ethan Glasser-Camp]

Tomi Ollila tomi.oll...@iki.fi writes:

 I can verify this bug: I copied 'rawmail' to my mail store and attempted
 to 'w' the attacment and got the same result (after notmuch new).

 The saving code first does
 notmuch show --format=raw id:508953e6.70...@gmail.com
 which decodes OK on command line, and to the buffer when 
 kill-buffer is outcommented in (with-current-notmuch-show-message ...) 
 macro.

I was able to see this behavior, and Tomi did a good job tracking down
where it was :)

I even see the bytes as presented in the file. When moving point to the
problematic character, and doing M-x describe-char, it says:

  buffer code: #xE2 #x80 #x99
file code: #xE2 #x80 #x99 (encoded by coding system utf-8)

buffer-file-coding-system is, of course, utf-8.

Writing this buffer using C-x C-w encodes it correctly too. So I think
this is an emacs MIME problem. We call mm-save-part, which calls
mm-save-part-to-file, which calls mm-with-unibyte-buffer. Hmm..

Indeed, it seems that inserting this character into a file that's been
marked unibyte using (set-buffer-multibyte nil) turns it into the ^Y
character (ASCII code 0x19 -- the character that comes out in the patch
file). There's probably a technical reason that this should be true, but
I can't think of why that would be.

 I attempted a set of trial--error tricks to get the attachment
 saved correctly, and at least this seems to do the trick:

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index f273eb4..a6a85c0 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -203,9 +203,11 @@ For example, if you wanted to remove an \unread\ tag 
 and add a
   (let ((id (notmuch-show-get-message-id)))
 (let ((buf (generate-new-buffer (concat *notmuch-msg- id *
   (with-current-buffer buf
 - (call-process notmuch-command nil t nil show --format=raw id)
 -   ,@body)
 -  (kill-buffer buf)
 +(let ((coding-system-for-read 'no-conversion)
 +  (coding-system-for-write 'no-conversion))
 +  (call-process notmuch-command nil t nil show --format=raw id)
 +  ,@body))
 +%%(kill-buffer buf)
[snip]
 (kill-buffer is outcommented above for testing purposes)

 To test this this needs to me evaluated and then the functions
 using this macro (notmuch-show-save-attachments  in this case)

 Smart suggestions for proper fix ?

Well, we could limit it just to saving attachments (putting the let
around the with-current-notmuch-show-message). That feels like it could
be right, because intuitively saving an attachment should be done
without any conversions. Or even the above doesn't seem so bad. My vague
feeling is that messages should always be ASCII, or at least mm-* will
interpret it that way, so decoding them into any other character set
might cause problems. Anyone understand character sets?

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

[PATCH] test: add nontrivial test for restore --accumulate.

[Ethan Glasser-Camp]

david at tethera.net writes:

> From: David Bremner 
>
> It seems we have never tested the case that restore --accumulate
> actually adds tags. I noticed this when I started optimizing and no
> tests failed.
>
> I also had to modify the next test. Perhaps a seperate patch could
> make these tests more independent of the previous ones.
> ---
>  test/dump-restore |   14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/test/dump-restore b/test/dump-restore
> index f25f7cf..ca7a730 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -29,18 +29,20 @@ test_expect_success 'Accumulate original tags' \
>notmuch dump > dump.actual &&
>test_cmp dump-ABC_DEF.expected dump.actual'
>
> -test_expect_success 'Restoring original tags' \
> -  'notmuch restore --input=dump.expected &&
> -  notmuch dump > dump.actual &&
> -  test_cmp dump.expected dump.actual'
> -

I guess you're removing this test because it just shows that restore can
remove tags, and we already see that in earlier tests?

>  test_expect_success 'Restore with nothing to do' \
>'notmuch restore < dump.expected &&
>notmuch dump > dump.actual &&
>test_cmp dump.expected dump.actual'

Maybe change the name of this test, as now it certainly does something?

> +test_expect_success 'Accumulate with changes' \
> +  'notmuch restore --input=dump.expected &&
> +   notmuch restore --accumulate --input=dump-ABC_DEF.expected &&
> +  notmuch dump >  OUTPUT.$test_count &&
> +  test_cmp dump-ABC_DEF.expected OUTPUT.$test_count'

Alignment? I think each line should start with two spaces.

> +
>  test_expect_success 'Restore with nothing to do, II' \
> -  'notmuch restore --accumulate --input=dump.expected &&
> +  'notmuch restore --input=dump.expected &&
> +  notmuch restore --accumulate --input=dump.expected &&
>notmuch dump > dump.actual &&
>test_cmp dump.expected dump.actual'

Maybe change the name? "Accumulate with nothing to do", for instance?

Ethan

Re: [PATCH] test: add nontrivial test for restore --accumulate.

[Ethan Glasser-Camp]

da...@tethera.net writes:

 From: David Bremner brem...@debian.org

 It seems we have never tested the case that restore --accumulate
 actually adds tags. I noticed this when I started optimizing and no
 tests failed.

 I also had to modify the next test. Perhaps a seperate patch could
 make these tests more independent of the previous ones.
 ---
  test/dump-restore |   14 --
  1 file changed, 8 insertions(+), 6 deletions(-)

 diff --git a/test/dump-restore b/test/dump-restore
 index f25f7cf..ca7a730 100755
 --- a/test/dump-restore
 +++ b/test/dump-restore
 @@ -29,18 +29,20 @@ test_expect_success 'Accumulate original tags' \
notmuch dump  dump.actual 
test_cmp dump-ABC_DEF.expected dump.actual'

 -test_expect_success 'Restoring original tags' \
 -  'notmuch restore --input=dump.expected 
 -  notmuch dump  dump.actual 
 -  test_cmp dump.expected dump.actual'
 -

I guess you're removing this test because it just shows that restore can
remove tags, and we already see that in earlier tests?

  test_expect_success 'Restore with nothing to do' \
'notmuch restore  dump.expected 
notmuch dump  dump.actual 
test_cmp dump.expected dump.actual'

Maybe change the name of this test, as now it certainly does something?

 +test_expect_success 'Accumulate with changes' \
 +  'notmuch restore --input=dump.expected 
 +   notmuch restore --accumulate --input=dump-ABC_DEF.expected 
 +  notmuch dump   OUTPUT.$test_count 
 +  test_cmp dump-ABC_DEF.expected OUTPUT.$test_count'

Alignment? I think each line should start with two spaces.

 +
  test_expect_success 'Restore with nothing to do, II' \
 -  'notmuch restore --accumulate --input=dump.expected 
 +  'notmuch restore --input=dump.expected 
 +  notmuch restore --accumulate --input=dump.expected 
notmuch dump  dump.actual 
test_cmp dump.expected dump.actual'

Maybe change the name? Accumulate with nothing to do, for instance?

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

[PATCH v2] test: Fix HTML rendering test

[Ethan Glasser-Camp]

Austin Clements  writes:

> The test designed to exercise Emacs' rendering of HTML emails
> containing images inadvertently assumed w3m was available under Emacs
> 23.  The real point of this test was to check that Emacs 24's shr
> renderer didn't crash when given img tags, so use shr if it's
> available, html2text otherwise (which is built in), and do only a
> simple sanity check of the result.

This fixes the failing test on my machine.

Ethan

[PATCH v2 0/3] Better id: link buttonization

[Ethan Glasser-Camp]

Austin Clements  writes:

> This is v2 of id:"1351650561-7331-1-git-send-email-amdragon at mit.edu".
> This makes Jani's suggested additions to the regexp and adds support
> for RFC 2392 mid: links, as suggested by Sascha.

This series looks fine to me.

Ethan

[PATCH v2] emacs: display tags in notmuch-show with links

[Ethan Glasser-Camp]

Damien Cassou  writes:

> +(defun notmuch-tagger-present-tags (tags  headerline)
> +  "Return a property list which nicely presents all TAGS.
> +
> +If HEADERLINE is non-nil the returned list will be ready for
> +inclusion in the buffer's header-line. HEADERLINE must be nil in
> +all other cases."
> +  (list
> +   "("
> +   (notmuch-tagger-separate-elems (notmuch-tagger-format-tags tags 
> headerline) " ")
> +   ")"))

It is kind of appalling that it takes 128 lines just to do this. It
seems like there has to be an easier way, or several easier
ways. Unfortunately, I don't see any.

> diff --git a/test/emacs b/test/emacs
> index 44f641e..ecdc841 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -820,5 +820,66 @@ Date: Fri, 05 Jan 2001 15:43:57 +
>  EOF
>  test_expect_equal_file OUTPUT EXPECTED
>
> +test_begin_subtest "Extracting all tags from a thread"
> +add_message \
> +'[subject]="Extracting all tags from a thread"' \
> +'[body]="body 1"'
> +parent=${gen_msg_id}
> +add_message \
> +'[subject]="Extracting all tags from a thread"' \
> +'[body]="body 2"' \
> +"[in-reply-to]=\<$parent\>"
> +add_message \
> +'[subject]="Extracting all tags from a thread"' \
> +'[body]="body 3"' \
> +"[in-reply-to]=\<$parent\>"
> +latest=${gen_msg_id}
> +# Extract the thread-id from one of the emails
> +thread_id=$(notmuch search id:${latest} | sed -e 
> "s/thread:\([a-f0-9]*\).*/\1/")

I think the accepted idiom is to use "notmuch search
--output=threads". This will output just a string like
"thread:00b9", so if you really need just the ID, you could
still use sed here...

> +# Add tag "mytagfoo" to one of the emails
> +notmuch tag +mytagfoo id:${latest}
> +test_emacs_expect_t \
> +"(notmuch-show \"thread:${thread_id}\")

... but it seems like "thread:..." is good enough for you.

> +   (error \"We must be in notmch-show at this point but we are in %s.\" 
> major-mode))
> +(push-button) ;; simulate a press on the RET key
> +(if (eq major-mode 'notmuch-search-mode)
> +t
> +   (format \"We must be in notmch-search at this point but we are in 
> %s.\" major-mode))"

s/notmch/notmuch/ here.

Otherwise I think the code looks fine. I think the design concerns
raised by Mark Walters are probably valid, though.

Ethan

emacs: Handling external dependencies

[Ethan Glasser-Camp]

Damien Cassou  writes:

> 4) distribute the dependency with the rest of notmuch (in a separate
> "fallback-libs/" directory) and load it only when requiring the
> library with the standard load-path does not work. Jonas Bernoulli
> gave me a way to do that:
>
> ,
> | (or (require 'THE-LIB nil t)
> | (let ((load-path
> |   (cons (expand-file-name
> |  "fallback-libs"
> |  (file-name-directory (or load-file-name buffer-file-name)))
> | load-path)))
> |   (require 'THE-LIB)))
> `
>
> What do you think?

Why not just append it to the *end* of load-path? Then it won't shadow
anything.

Ethan

Re: emacs: Handling external dependencies

[Ethan Glasser-Camp]

Damien Cassou damien.cas...@gmail.com writes:

 4) distribute the dependency with the rest of notmuch (in a separate
 fallback-libs/ directory) and load it only when requiring the
 library with the standard load-path does not work. Jonas Bernoulli
 gave me a way to do that:

 ,
 | (or (require 'THE-LIB nil t)
 | (let ((load-path
 |   (cons (expand-file-name
 |  fallback-libs
 |  (file-name-directory (or load-file-name buffer-file-name)))
 | load-path)))
 |   (require 'THE-LIB)))
 `

 What do you think?

Why not just append it to the *end* of load-path? Then it won't shadow
anything.

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

Re: [PATCH v2] emacs: display tags in notmuch-show with links

[Ethan Glasser-Camp]

Damien Cassou damien.cas...@gmail.com writes:

 +(defun notmuch-tagger-present-tags (tags optional headerline)
 +  Return a property list which nicely presents all TAGS.
 +
 +If HEADERLINE is non-nil the returned list will be ready for
 +inclusion in the buffer's header-line. HEADERLINE must be nil in
 +all other cases.
 +  (list
 +   (
 +   (notmuch-tagger-separate-elems (notmuch-tagger-format-tags tags 
 headerline)  )
 +   )))

It is kind of appalling that it takes 128 lines just to do this. It
seems like there has to be an easier way, or several easier
ways. Unfortunately, I don't see any.

 diff --git a/test/emacs b/test/emacs
 index 44f641e..ecdc841 100755
 --- a/test/emacs
 +++ b/test/emacs
 @@ -820,5 +820,66 @@ Date: Fri, 05 Jan 2001 15:43:57 +
  EOF
  test_expect_equal_file OUTPUT EXPECTED

 +test_begin_subtest Extracting all tags from a thread
 +add_message \
 +'[subject]=Extracting all tags from a thread' \
 +'[body]=body 1'
 +parent=${gen_msg_id}
 +add_message \
 +'[subject]=Extracting all tags from a thread' \
 +'[body]=body 2' \
 +[in-reply-to]=\$parent\
 +add_message \
 +'[subject]=Extracting all tags from a thread' \
 +'[body]=body 3' \
 +[in-reply-to]=\$parent\
 +latest=${gen_msg_id}
 +# Extract the thread-id from one of the emails
 +thread_id=$(notmuch search id:${latest} | sed -e 
 s/thread:\([a-f0-9]*\).*/\1/)

I think the accepted idiom is to use notmuch search
--output=threads. This will output just a string like
thread:00b9, so if you really need just the ID, you could
still use sed here...

 +# Add tag mytagfoo to one of the emails
 +notmuch tag +mytagfoo id:${latest}
 +test_emacs_expect_t \
 +(notmuch-show \thread:${thread_id}\)

... but it seems like thread:... is good enough for you.

 +   (error \We must be in notmch-show at this point but we are in %s.\ 
 major-mode))
 +(push-button) ;; simulate a press on the RET key
 +(if (eq major-mode 'notmuch-search-mode)
 +t
 +   (format \We must be in notmch-search at this point but we are in 
 %s.\ major-mode))

s/notmch/notmuch/ here.

Otherwise I think the code looks fine. I think the design concerns
raised by Mark Walters are probably valid, though.

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

Re: [PATCH v2 0/3] Better id: link buttonization

[Ethan Glasser-Camp]

Austin Clements amdra...@mit.edu writes:

 This is v2 of id:1351650561-7331-1-git-send-email-amdra...@mit.edu.
 This makes Jani's suggested additions to the regexp and adds support
 for RFC 2392 mid: links, as suggested by Sascha.

This series looks fine to me.

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

Re: [PATCH v2] test: Fix HTML rendering test

[Ethan Glasser-Camp]

Austin Clements amdra...@mit.edu writes:

 The test designed to exercise Emacs' rendering of HTML emails
 containing images inadvertently assumed w3m was available under Emacs
 23.  The real point of this test was to check that Emacs 24's shr
 renderer didn't crash when given img tags, so use shr if it's
 available, html2text otherwise (which is built in), and do only a
 simple sanity check of the result.

This fixes the failing test on my machine.

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

[PATCH 1/1] uncrustify.cfg: added 3 new types for uncrustify to know

[Ethan Glasser-Camp]

Tomi Ollila  writes:

> Added FILE, notmuch_show_params_t and sprinter_t to be
> types when uncrustifying sources. This affect spacing
> when uncrustify is deciding for type declaration instead
> of binary multiplication operation.

This looks good to me. If you had plenty of time and no more patches to
review, I'd prefer the slightly cleaner English:

This affects how uncrustify puts spacing around pointers to these types,
since it can parse them as type declarations instead of binary
multiplication operations.


... but even suggesting this indicates I've moved past the bike shed and
into somebody else's kitchen.

Ethan

[PATCH] fix notmuch_database_open call in addrlookup

[Ethan Glasser-Camp]

James Vasile  writes:

> What's the best way to submit changes to addrlookup?  Right now, it is
> out of date vs the latest libnotmuch.  The addrlookup repo is vala code
> but the wiki [1] points to a generated c file [2].
>
> [1] http://github.com/spaetz/vala-notmuch/raw/static-sources/src/addrlookup.c
> [2] http://notmuchmail.org/emacstips/
>
> At any rate, a patch to that c file is below.  If you upgraded notmuch
> and now addrlookup gives errors about not finding libnotmuch.so.2, this
> patch might be what you need.

I'd try to contact the author of the code, perhaps by submitting an
issue or pull request on the github page for the project. I'm sure he'd
prefer a patch to the Vala source.

Ethan

[PATCH v2 0/2] include Reply-To headers in json output

[Ethan Glasser-Camp]

Peter Wang  writes:

> This obsoletes the series 1340508470-16606-1-git-send-email-novalazy at 
> gmail.com
> Only json output is affected now.
>
> Peter Wang (2):
>   show: include Reply-To header in json output
>   test: add test for showing Reply-To headers

LGTM. Removed needs-review, added notmuch::trivial.

Ethan

Re: [PATCH v2 0/2] include Reply-To headers in json output

[Ethan Glasser-Camp]

Peter Wang noval...@gmail.com writes:

 This obsoletes the series 1340508470-16606-1-git-send-email-noval...@gmail.com
 Only json output is affected now.

 Peter Wang (2):
   show: include Reply-To header in json output
   test: add test for showing Reply-To headers

LGTM. Removed needs-review, added notmuch::trivial.

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

Re: [PATCH] fix notmuch_database_open call in addrlookup

[Ethan Glasser-Camp]

James Vasile ja...@hackervisions.org writes:

 What's the best way to submit changes to addrlookup?  Right now, it is
 out of date vs the latest libnotmuch.  The addrlookup repo is vala code
 but the wiki [1] points to a generated c file [2].

 [1] http://github.com/spaetz/vala-notmuch/raw/static-sources/src/addrlookup.c
 [2] http://notmuchmail.org/emacstips/

 At any rate, a patch to that c file is below.  If you upgraded notmuch
 and now addrlookup gives errors about not finding libnotmuch.so.2, this
 patch might be what you need.

I'd try to contact the author of the code, perhaps by submitting an
issue or pull request on the github page for the project. I'm sure he'd
prefer a patch to the Vala source.

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

Re: [PATCH 1/1] uncrustify.cfg: added 3 new types for uncrustify to know

[Ethan Glasser-Camp]

Tomi Ollila tomi.oll...@iki.fi writes:

 Added FILE, notmuch_show_params_t and sprinter_t to be
 types when uncrustifying sources. This affect spacing
 when uncrustify is deciding for type declaration instead
 of binary multiplication operation.

This looks good to me. If you had plenty of time and no more patches to
review, I'd prefer the slightly cleaner English:

This affects how uncrustify puts spacing around pointers to these types,
since it can parse them as type declarations instead of binary
multiplication operations.


... but even suggesting this indicates I've moved past the bike shed and
into somebody else's kitchen.

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

[PATCH (draft) 1/2] emacs: allow the user to toggle the visibility of multipart/alternative parts

[Ethan Glasser-Camp]

Mark Walters  writes:

> This patch adds a keybinding to the buttons in the notmuch-show emacs
> buffer to allow the user to toggle the visibility of each part of a
> message in the show buffer.  This is particularly useful for
> multipart/alternative parts where the parts are not really
> alternatives but contain different information.
> ---
>  emacs/notmuch-show.el |   47 +++
>  1 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 0f54259..9157669 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -155,6 +155,10 @@ indentation."
>  (make-variable-buffer-local 'notmuch-show-indent-content)
>  (put 'notmuch-show-indent-content 'permanent-local t)
>
> +(defvar notmuch-show-message-multipart/alternative-display-parts nil)
> +(make-variable-buffer-local 
> 'notmuch-show-message-multipart/alternative-display-parts)
> +(put 'notmuch-show-message-multipart/alternative-display-parts 
> 'permanent-local t)
> +
>  (defcustom notmuch-show-stash-mlarchive-link-alist
>'(("Gmane" . "http://mid.gmane.org/;)
>  ("MARC" . "http://marc.info/?i=;)
> @@ -455,6 +459,7 @@ message at DEPTH in the current thread."
>  (define-key map "v" 'notmuch-show-part-button-view)
>  (define-key map "o" 'notmuch-show-part-button-interactively-view)
>  (define-key map "|" 'notmuch-show-part-button-pipe)
> +(define-key map "t" 'notmuch-show-part-button-internally-show)
>  map)
>"Submap for button commands")
>  (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
> @@ -531,6 +536,16 @@ message at DEPTH in the current thread."
>  (let ((handle (mm-make-handle (current-buffer) (list content-type
>(mm-pipe-part handle
>
> +(defun notmuch-show-internally-show-part (message-id nth  filename 
> content-type)
> +  "Set a part to be displayed internally"
> +  (let ((current-parts (lax-plist-get 
> notmuch-show-message-multipart/alternative-display-parts message-id)))
> +(setq notmuch-show-message-multipart/alternative-display-parts
> +   (lax-plist-put 
> notmuch-show-message-multipart/alternative-display-parts message-id
> +  (if (memq nth current-parts)
> +  (delq nth current-parts)
> +(cons nth current-parts)
> +  (notmuch-show-refresh-view))
> +
>  (defun notmuch-show-multipart/*-to-list (part)
>(mapcar (lambda (inner-part) (plist-get inner-part :content-type))
> (plist-get part :content)))
> @@ -543,12 +558,15 @@ message at DEPTH in the current thread."
>  ;; This inserts all parts of the chosen type rather than just one,
>  ;; but it's not clear that this is the wrong thing to do - which
>  ;; should be chosen if there are more than one that match?
> +
> +;; The variable user-parts says which parts should override the
> +;; default so we use xor (handcoded since lisp does not have it).

I don't follow the comment. user-parts isn't used in this
function. Neither is xor.

>  (mapc (lambda (inner-part)
>   (let ((inner-type (plist-get inner-part :content-type)))
> -   (if (or notmuch-show-all-multipart/alternative-parts
> -   (string= chosen-type inner-type))
> -   (notmuch-show-insert-bodypart msg inner-part depth)
> - (notmuch-show-insert-part-header (plist-get inner-part :id) 
> inner-type inner-type nil " (not shown)"
> +   (notmuch-show-insert-bodypart msg inner-part depth
> + (not (or 
> notmuch-show-all-multipart/alternative-parts
> +  (string=
>   chosen-type inner-type))

For what it's worth, I found this not-shown logic very confusing, and
have had to think about it seven or eight different times to make sure I
understood what's going on. I'm not sure why exactly this is, though I
could offer hypotheses -- the fact that it's split across two functions,
or the fiddling with mime-types. I'm satisfied that it's correct, but I
wish it could be made clearer.

This is just armchair hypothesizing, but here are some ideas that might
make it more obvious what's going on: bringing the user-parts logic into
this function; making user-parts, instead of a "t" meaning "user has
toggled this", something like 'opened or 'closed and if user-parts for
this message is absent, falling back to this calculation; alternately,
prefilling user-parts with t when show is invoked, according to this
calculation, and then not using it any more; moving this
not-shown calculation into a separate function, something like 
notmuch-show-get-message-visibility.

I guess I jumped into this series halfway, but why are we doing this
with the wipe/redraw technique instead of just using invisible overlays,
like we do more generally with notmuch-show? I think I agree that
toggling individual parts is a good 

[PATCH 1/3] contrib: add notmuch-pick.el file itself

[Ethan Glasser-Camp]

Mark Walters  writes:

> +(defvar notmuch-pick-json-parser nil
> +  "Incremental JSON parser for the search process filter.")
> +
> +(defun notmuch-pick-process-filter (proc string)
> +  "Process and filter the output of \"notmuch show\" (for pick)"
> +  (let ((results-buf (process-buffer proc))
> +(parse-buf (process-get proc 'parse-buf))
> +(inhibit-read-only t)
> +done)
> +(if (not (buffer-live-p results-buf))
> +(delete-process proc)
> +  (with-current-buffer parse-buf
> +;; Insert new data
> +(save-excursion
> +  (goto-char (point-max))
> +  (insert string)))
> +  (with-current-buffer results-buf
> + (save-excursion
> +   (goto-char (point-max))
> +   (while (not done)
> + (condition-case nil
> + (case notmuch-pick-process-state

This looks awfully familiar. Not looking too close, but why can't this
re-use the JSON parser from your other patch? Just not to rely on the
other patch series?

Still, let's get this pushed.

Ethan

[PATCH 1/3] emacs: Introduce generic boolean term escaping function

[Ethan Glasser-Camp]

Tomi Ollila  writes:

> These 3 patches LGTM.

Me too. But I wouldn't be averse to some tests :)

Ethan

[PATCH 1/2] tag: Disallow adding malformed tags to messages

[Ethan Glasser-Camp]

Tomi Ollila  writes:

> LGTM (NEWS too)

Yep! Removing needs-review.

Ethan

Re: [PATCH 1/3] emacs: Introduce generic boolean term escaping function

[Ethan Glasser-Camp]

Tomi Ollila tomi.oll...@iki.fi writes:

 These 3 patches LGTM.

Me too. But I wouldn't be averse to some tests :)

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

Re: [PATCH 1/3] contrib: add notmuch-pick.el file itself

[Ethan Glasser-Camp]

Mark Walters markwalters1...@gmail.com writes:

 +(defvar notmuch-pick-json-parser nil
 +  Incremental JSON parser for the search process filter.)
 +
 +(defun notmuch-pick-process-filter (proc string)
 +  Process and filter the output of \notmuch show\ (for pick)
 +  (let ((results-buf (process-buffer proc))
 +(parse-buf (process-get proc 'parse-buf))
 +(inhibit-read-only t)
 +done)
 +(if (not (buffer-live-p results-buf))
 +(delete-process proc)
 +  (with-current-buffer parse-buf
 +;; Insert new data
 +(save-excursion
 +  (goto-char (point-max))
 +  (insert string)))
 +  (with-current-buffer results-buf
 + (save-excursion
 +   (goto-char (point-max))
 +   (while (not done)
 + (condition-case nil
 + (case notmuch-pick-process-state

This looks awfully familiar. Not looking too close, but why can't this
re-use the JSON parser from your other patch? Just not to rely on the
other patch series?

Still, let's get this pushed.

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

Re: [PATCH (draft) 1/2] emacs: allow the user to toggle the visibility of multipart/alternative parts

[Ethan Glasser-Camp]

Mark Walters markwalters1...@gmail.com writes:

 This patch adds a keybinding to the buttons in the notmuch-show emacs
 buffer to allow the user to toggle the visibility of each part of a
 message in the show buffer.  This is particularly useful for
 multipart/alternative parts where the parts are not really
 alternatives but contain different information.
 ---
  emacs/notmuch-show.el |   47 +++
  1 files changed, 39 insertions(+), 8 deletions(-)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index 0f54259..9157669 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -155,6 +155,10 @@ indentation.
  (make-variable-buffer-local 'notmuch-show-indent-content)
  (put 'notmuch-show-indent-content 'permanent-local t)

 +(defvar notmuch-show-message-multipart/alternative-display-parts nil)
 +(make-variable-buffer-local 
 'notmuch-show-message-multipart/alternative-display-parts)
 +(put 'notmuch-show-message-multipart/alternative-display-parts 
 'permanent-local t)
 +
  (defcustom notmuch-show-stash-mlarchive-link-alist
'((Gmane . http://mid.gmane.org/;)
  (MARC . http://marc.info/?i=;)
 @@ -455,6 +459,7 @@ message at DEPTH in the current thread.
  (define-key map v 'notmuch-show-part-button-view)
  (define-key map o 'notmuch-show-part-button-interactively-view)
  (define-key map | 'notmuch-show-part-button-pipe)
 +(define-key map t 'notmuch-show-part-button-internally-show)
  map)
Submap for button commands)
  (fset 'notmuch-show-part-button-map notmuch-show-part-button-map)
 @@ -531,6 +536,16 @@ message at DEPTH in the current thread.
  (let ((handle (mm-make-handle (current-buffer) (list content-type
(mm-pipe-part handle

 +(defun notmuch-show-internally-show-part (message-id nth optional filename 
 content-type)
 +  Set a part to be displayed internally
 +  (let ((current-parts (lax-plist-get 
 notmuch-show-message-multipart/alternative-display-parts message-id)))
 +(setq notmuch-show-message-multipart/alternative-display-parts
 +   (lax-plist-put 
 notmuch-show-message-multipart/alternative-display-parts message-id
 +  (if (memq nth current-parts)
 +  (delq nth current-parts)
 +(cons nth current-parts)
 +  (notmuch-show-refresh-view))
 +
  (defun notmuch-show-multipart/*-to-list (part)
(mapcar (lambda (inner-part) (plist-get inner-part :content-type))
 (plist-get part :content)))
 @@ -543,12 +558,15 @@ message at DEPTH in the current thread.
  ;; This inserts all parts of the chosen type rather than just one,
  ;; but it's not clear that this is the wrong thing to do - which
  ;; should be chosen if there are more than one that match?
 +
 +;; The variable user-parts says which parts should override the
 +;; default so we use xor (handcoded since lisp does not have it).

I don't follow the comment. user-parts isn't used in this
function. Neither is xor.

  (mapc (lambda (inner-part)
   (let ((inner-type (plist-get inner-part :content-type)))
 -   (if (or notmuch-show-all-multipart/alternative-parts
 -   (string= chosen-type inner-type))
 -   (notmuch-show-insert-bodypart msg inner-part depth)
 - (notmuch-show-insert-part-header (plist-get inner-part :id) 
 inner-type inner-type nil  (not shown)
 +   (notmuch-show-insert-bodypart msg inner-part depth
 + (not (or 
 notmuch-show-all-multipart/alternative-parts
 +  (string=
   chosen-type inner-type))

For what it's worth, I found this not-shown logic very confusing, and
have had to think about it seven or eight different times to make sure I
understood what's going on. I'm not sure why exactly this is, though I
could offer hypotheses -- the fact that it's split across two functions,
or the fiddling with mime-types. I'm satisfied that it's correct, but I
wish it could be made clearer.

This is just armchair hypothesizing, but here are some ideas that might
make it more obvious what's going on: bringing the user-parts logic into
this function; making user-parts, instead of a t meaning user has
toggled this, something like 'opened or 'closed and if user-parts for
this message is absent, falling back to this calculation; alternately,
prefilling user-parts with t when show is invoked, according to this
calculation, and then not using it any more; moving this
not-shown calculation into a separate function, something like 
notmuch-show-get-message-visibility.

I guess I jumped into this series halfway, but why are we doing this
with the wipe/redraw technique instead of just using invisible overlays,
like we do more generally with notmuch-show? I think I agree that
toggling individual parts is a good UI approach, and this isn't a bad
way to implement it, but I wonder 

Re: [PATCH 1/2] tag: Disallow adding malformed tags to messages

[Ethan Glasser-Camp]

Tomi Ollila tomi.oll...@iki.fi writes:

 LGTM (NEWS too)

Yep! Removing needs-review.

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

[PATCH] Support OpenBSD

[Ethan Glasser-Camp]

Austin Clements  writes:

> OpenBSD's build flags are identical to FreeBSD, except that libraries
> need to be explicitly linked against libc.  No code changes are
> necessary.
>
> From: Cody Cutler 
> ---

OK, looks fine.

Ethan

[PATCH] test: Fix HTML rendering test

[Ethan Glasser-Camp]

Austin Clements  writes:

> Quoth Ethan Glasser-Camp on Oct 24 at  9:59 pm:
>> Austin Clements  writes:
>
> Emacs seems to have as many ways to convert HTML to text as there are
> people trying to run this test.  What's the value of
> mm-text-html-renderer for you in Emacs 24?

I get html2text.

I wanted to write more about what the test should and should not do, but
really I don't know and don't have the time to write about it!

Ethan

Re: [PATCH] test: Fix HTML rendering test

[Ethan Glasser-Camp]

Austin Clements amdra...@mit.edu writes:

 Quoth Ethan Glasser-Camp on Oct 24 at  9:59 pm:
 Austin Clements amdra...@mit.edu writes:

 Emacs seems to have as many ways to convert HTML to text as there are
 people trying to run this test.  What's the value of
 mm-text-html-renderer for you in Emacs 24?

I get html2text.

I wanted to write more about what the test should and should not do, but
really I don't know and don't have the time to write about it!

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

Re: [PATCH] Support OpenBSD

[Ethan Glasser-Camp]

Austin Clements amdra...@mit.edu writes:

 OpenBSD's build flags are identical to FreeBSD, except that libraries
 need to be explicitly linked against libc.  No code changes are
 necessary.

 From: Cody Cutler ccut...@csail.mit.edu
 ---

OK, looks fine.

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

[PATCH] test: Fix HTML rendering test

[Ethan Glasser-Camp]

Austin Clements  writes:

> The test designed to exercise Emacs' rendering of HTML emails
> containing images inadvertently assumed w3m was available under Emacs
> 23.  The real point of this test was to exercise Emacs 24's shr
> renderer, so if shr isn't available, we now fall back to html2text,
> which comes with Emacs.

Hi! I'm eager to apply any patch here that makes this better. But this
one doesn't fix it for me (24.1.1, although it seems to work with
23.4.1). OUTPUT is "*\nsmiley " (no space after the asterisk or before
the word smiley, but after). I see that this sed command is supposed to
normalize things, but at least on my setup, it doesn't. I also see "nil"
written to console, but I have no idea what that's about.

More generally, I guess I don't understand exactly what this test is
supposed to be exercising. The commit message says "the shr renderer",
but what about it? In
id:"1348941314-8377-4-git-send-email-amdragon at mit.edu" you write that
using shr raised a void-variable error previously, so maybe we're making
sure that error doesn't show up? In that case, even my semi-broken
output is good enough.

In a perfect world, test probably shouldn't succeed if shr isn't
present, but should note that it wasn't run. Maybe the emacs lisp code
can check for shr, and if it's not present, write "shr not present" to
an output file, and the shell code can grep for that and then call
test_skip if it sees it?

Still, I'm excited that you're working on this so please let's get it
fixed!

Ethan

Re: [PATCH] test: Fix HTML rendering test

[Ethan Glasser-Camp]

Austin Clements amdra...@mit.edu writes:

 The test designed to exercise Emacs' rendering of HTML emails
 containing images inadvertently assumed w3m was available under Emacs
 23.  The real point of this test was to exercise Emacs 24's shr
 renderer, so if shr isn't available, we now fall back to html2text,
 which comes with Emacs.

Hi! I'm eager to apply any patch here that makes this better. But this
one doesn't fix it for me (24.1.1, although it seems to work with
23.4.1). OUTPUT is *\nsmiley  (no space after the asterisk or before
the word smiley, but after). I see that this sed command is supposed to
normalize things, but at least on my setup, it doesn't. I also see nil
written to console, but I have no idea what that's about.

More generally, I guess I don't understand exactly what this test is
supposed to be exercising. The commit message says the shr renderer,
but what about it? In
id:1348941314-8377-4-git-send-email-amdra...@mit.edu you write that
using shr raised a void-variable error previously, so maybe we're making
sure that error doesn't show up? In that case, even my semi-broken
output is good enough.

In a perfect world, test probably shouldn't succeed if shr isn't
present, but should note that it wasn't run. Maybe the emacs lisp code
can check for shr, and if it's not present, write shr not present to
an output file, and the shell code can grep for that and then call
test_skip if it sees it?

Still, I'm excited that you're working on this so please let's get it
fixed!

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

[PATCH v3 2/3] emacs: Rename incremental JSON internal variables.

[Ethan Glasser-Camp]

Mark Walters  writes:

> This patch just renames the internal variables for the JSON parser now
> it is no longer specific to search mode. It also fixes up the white
> space after the previous patch. There should be no functional changes.

This series looks very good to me. I still have a couple of quibbles that I 
wouldn't say
should hold the series up. First, your commit messages have periods at
the end of the first line. I think it is considered good practice that
they not (see, e.g.,
http://blogs.gnome.org/danni/2011/10/25/a-guide-to-writing-git-commit-messages/ 
).

> -(defvar notmuch-search-process-state nil
> -  "Parsing state of the search process filter.")
> +(defvar notmuch-json-state nil
> +  "State of the internal JSON parser.")

How about,

"State of the shared JSON parser.

This variable will be a symbol, corresponding to the state of the JSON
parser's state machine. 'begin means we haven't yet entered a JSON
list. 'result means we are ready to parse a JSON object. 'end means we
have exited the JSON list."

(Or whatever the correct meanings of the parser's states are...)

> -(defvar notmuch-search-json-parser nil
> -  "Incremental JSON parser for the search process filter.")
> +(defvar notmuch-json-parser nil
> +  "Internal Incremental JSON parser Object.")

How about,

"Shared notmuch JSON parser object, as created by
(notmuch-json-create-parser).

Any notmuch code can parse part of a JSON list object by setting this
variable to their JSON parser and calling
notmuch-json-parse-partial-list (which see)."

(Or whatever one is supposed to do with the parser object...)

Ethan

Re: [PATCH v3 2/3] emacs: Rename incremental JSON internal variables.

[Ethan Glasser-Camp]

Mark Walters markwalters1...@gmail.com writes:

 This patch just renames the internal variables for the JSON parser now
 it is no longer specific to search mode. It also fixes up the white
 space after the previous patch. There should be no functional changes.

This series looks very good to me. I still have a couple of quibbles that I 
wouldn't say
should hold the series up. First, your commit messages have periods at
the end of the first line. I think it is considered good practice that
they not (see, e.g.,
http://blogs.gnome.org/danni/2011/10/25/a-guide-to-writing-git-commit-messages/ 
).

 -(defvar notmuch-search-process-state nil
 -  Parsing state of the search process filter.)
 +(defvar notmuch-json-state nil
 +  State of the internal JSON parser.)

How about,

State of the shared JSON parser.

This variable will be a symbol, corresponding to the state of the JSON
parser's state machine. 'begin means we haven't yet entered a JSON
list. 'result means we are ready to parse a JSON object. 'end means we
have exited the JSON list.

(Or whatever the correct meanings of the parser's states are...)

 -(defvar notmuch-search-json-parser nil
 -  Incremental JSON parser for the search process filter.)
 +(defvar notmuch-json-parser nil
 +  Internal Incremental JSON parser Object.)

How about,

Shared notmuch JSON parser object, as created by
(notmuch-json-create-parser).

Any notmuch code can parse part of a JSON list object by setting this
variable to their JSON parser and calling
notmuch-json-parse-partial-list (which see).

(Or whatever one is supposed to do with the parser object...)

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

[PATCH] test: new: Fix intermittent test failures with --debug

[Ethan Glasser-Camp]

Although messages are created in a particular order, it seems that
when they are created on a tmpfs, they do not always come back in the
same order, leading to the same files being ignored but being output
in a different order. This causes the test to fail because the outputs
being compared are the same.

Fix the failures by sorting the output of notmuch --debug and
comparing this to a hand-sorted version of its output.

Signed-off-by: Ethan Glasser-Camp 
---
 test/new |   10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/new b/test/new
index cc2af72..587aa11 100755
--- a/test/new
+++ b/test/new
@@ -189,17 +189,17 @@ touch "${MAIL_DIR}"/.git # change .git's mtime for 
notmuch new to rescan.
 mkdir -p "${MAIL_DIR}"/one/two/three/.git
 notmuch new > /dev/null # ensure that files/folders will be printed in ASCII 
order.
 touch "${MAIL_DIR}"/{one,one/two,one/two/three}/ignored_file
-output=$(NOTMUCH_NEW --debug 2>&1)
+output=$(NOTMUCH_NEW --debug 2>&1 | sort)
 test_expect_equal "$output" \
 "(D) add_files_recursive, pass 1: explicitly ignoring ${MAIL_DIR}/.git
 (D) add_files_recursive, pass 1: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/ignored_file
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/ignored_file
 (D) add_files_recursive, pass 2: explicitly ignoring ${MAIL_DIR}/.git
 (D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/.ignored_hidden_file
 (D) add_files_recursive, pass 2: explicitly ignoring ${MAIL_DIR}/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
 No new mail."


-- 
1.7.9.5

[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Ethan Glasser-Camp]

Peter Wang  writes:

> Does it help if you add a "sleep 1" before the second generate_message
> call, i.e. on line 35?

It turns out that this test failure is sporadic (perhaps due to the fact
that I'm running on tmpfs) and exists even before this series. Doing
"sleep 1" makes it go away, but that "fix" is unrelated to this series
and ought to be fixed elsewhere.

> I have to disagree.  The condition is wrapped over two lines.  The then
> part is wrapped over two lines.  The else part already has braces.
> All suggest braces around the then part.

If Tomi is OK with it, then I guess it's fine. And it's true that there
are a couple of places where braces are used with long conditions and
then-parts.

This series has my +1.

Ethan

[PATCH v3] test: conform to content length, encoding fields

[Ethan Glasser-Camp]

Peter Wang  writes:

> Update tests to expect content-length and content-transfer-encoding
> fields in show --format=json output, for leaf parts with omitted body
> content.

OK, this whole series looks good to me.

Ethan

Re: [PATCH v3] test: conform to content length, encoding fields

[Ethan Glasser-Camp]

Peter Wang noval...@gmail.com writes:

 Update tests to expect content-length and content-transfer-encoding
 fields in show --format=json output, for leaf parts with omitted body
 content.

OK, this whole series looks good to me.

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

Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Ethan Glasser-Camp]

Peter Wang noval...@gmail.com writes:

 Does it help if you add a sleep 1 before the second generate_message
 call, i.e. on line 35?

It turns out that this test failure is sporadic (perhaps due to the fact
that I'm running on tmpfs) and exists even before this series. Doing
sleep 1 makes it go away, but that fix is unrelated to this series
and ought to be fixed elsewhere.

 I have to disagree.  The condition is wrapped over two lines.  The then
 part is wrapped over two lines.  The else part already has braces.
 All suggest braces around the then part.

If Tomi is OK with it, then I guess it's fine. And it's true that there
are a couple of places where braces are used with long conditions and
then-parts.

This series has my +1.

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

[PATCH] test: new: Fix intermittent test failures with --debug

[Ethan Glasser-Camp]

Although messages are created in a particular order, it seems that
when they are created on a tmpfs, they do not always come back in the
same order, leading to the same files being ignored but being output
in a different order. This causes the test to fail because the outputs
being compared are the same.

Fix the failures by sorting the output of notmuch --debug and
comparing this to a hand-sorted version of its output.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
 test/new |   10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/new b/test/new
index cc2af72..587aa11 100755
--- a/test/new
+++ b/test/new
@@ -189,17 +189,17 @@ touch ${MAIL_DIR}/.git # change .git's mtime for 
notmuch new to rescan.
 mkdir -p ${MAIL_DIR}/one/two/three/.git
 notmuch new  /dev/null # ensure that files/folders will be printed in ASCII 
order.
 touch ${MAIL_DIR}/{one,one/two,one/two/three}/ignored_file
-output=$(NOTMUCH_NEW --debug 21)
+output=$(NOTMUCH_NEW --debug 21 | sort)
 test_expect_equal $output \
 (D) add_files_recursive, pass 1: explicitly ignoring ${MAIL_DIR}/.git
 (D) add_files_recursive, pass 1: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/ignored_file
-(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/ignored_file
 (D) add_files_recursive, pass 2: explicitly ignoring ${MAIL_DIR}/.git
 (D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/.ignored_hidden_file
 (D) add_files_recursive, pass 2: explicitly ignoring ${MAIL_DIR}/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/ignored_file
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
+(D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
 No new mail.
 
 
-- 
1.7.9.5

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

[PATCH] lib: fix warnings when building with clang

[Ethan Glasser-Camp]

Jani Nikula  writes:

> Building notmuch with CC=clang and CXX=clang++ produces the warnings:
>
> CC -O2 lib/tags.o
> lib/tags.c:43:5: warning: expression result unused [-Wunused-value]
> talloc_steal (tags, list);
> ^
> /usr/include/talloc.h:345:143: note: expanded from:
>   ...__location__); __talloc_steal_ret; })
> ^~
> 1 warning generated.
>
> CXX -O2 lib/message.o
> lib/message.cc:791:5: warning: expression result unused [-Wunused-value]
> talloc_reference (message, message->tag_list);
> ^
> /usr/include/talloc.h:932:36: note: expanded from:
>   ...(_TALLOC_TYPEOF(ptr))_talloc_reference_loc((ctx),(ptr), __location__)
>  ^
> 1 warning generated.
>
> Check talloc_reference() return value, and explicitly ignore
> talloc_steal() return value as it has no failure modes, to silence the
> warnings.
> ---
>  lib/message.cc |4 +++-
>  lib/tags.c |2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/message.cc b/lib/message.cc
> index 978de06..320901f 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -788,7 +788,9 @@ notmuch_message_get_tags (notmuch_message_t *message)
>   * possible to modify the message tags (which talloc_unlink's the
>   * current list from the message) while still iterating because
>   * the iterator will keep the current list alive. */
> -talloc_reference (message, message->tag_list);
> +if (!talloc_reference (message, message->tag_list))
> + return NULL;
> +
>  return tags;
>  }

Hi! What you did with talloc_steal is obviously fine. 

I'd be happier about what you did with talloc_reference() if there were
precedent, or a clearly-articulated convention for notmuch. Instead this
is the third use in the codebase that I can see, and the other two are
each unique to themselves. In mime-node.c we print an "out-of-memory"
error and in lib/filenames.c we cast (void) talloc_reference (...), I
guess figuring that we're pretty much hosed anyhow if we run out of
memory.

Why return NULL here? It seems like if talloc_reference fails, we're
going to crash eventually, so we should print an error to explain our
impending doom. I'd guess you're uneasy printing anything from lib/, but
still want to signal an error, and the only way you can do so is to
return NULL. I guess that silences the compiler warning, but it's not
really the correct way to handle the error IMO. On the other hand, it's
such a weird corner case that I don't even think it merits a FIXME
comment.

How about an assert instead of a return NULL?

Ethan

[PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

[Ethan Glasser-Camp]

Tomi Ollila  writes:

> From: Tomi Ollila 
>
> When shell executes background process using '&' the scheduling of
> that new process is arbitrary. It could be that smtp-dummy doesn't
> get execution time to listen() it's server socket until some other
> process attempts to connect() to it. The --background option in
> smtp-dummy makes it to go background *after* it started to listen
> its server socket.

This looks fine to me, Michal Sojka's reviewed this version, and this
patch has been bouncing around for almost a year! I'm taking off
needs-review.

Ethan

[PATCHv3] notmuch-show: include Bcc header in json output

[Ethan Glasser-Camp]

Michal Nazarewicz  writes:

> From: Michal Nazarewicz 
>
> With this change, emacs users can use notmuch-message-headers
> variable to configure notmuch-show display Bcc header.
> ---

This patch looks pretty straightforward and has seen a certain amount of
review so I'm taking off needs-review.

Ethan

[PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t

[Ethan Glasser-Camp]

Jani Nikula  writes:

> On Wed, 17 Oct 2012, Adrien Bustany  wrote:
>> The code of the patches in unchanged, but the formatting issues are now
>> hopefully fixed.
>
> Hi Adrien, please check at what version flush and reopen have been
> introduced to xapian. If they are new-ish (I don't know, didn't have the
> time to check), please add appropriate #ifdefs. [1] lays the groundwork
> for this. We'll also need to decide what is the minimum xapian version
> required in general, i.e. features earlier than that don't need
> conditional compilation.

Hi! The new versions of these patches are still pretty trivial and they
still look OK to me, but based on Jani's prompting I decided to look up
the methods. Seems that flush() is a very old (pre-1.1.0, 2009-04) name
for commit(), which is the preferred name these days. You should
probably therefore rename the function notmuch_database_commit, and have
it call the WritableDatabase::commit() method.

reopen() is a very very old method, seems like it has been around since
2004.

So I think Adrien is safe from having to do version checks, but we
should probably use commit() instead of flush().

Ethan

random corpus generator, v3

[Ethan Glasser-Camp]

david at tethera.net writes:

> This obsoletes the series at:
>
>  id:"134431-4301-1-git-send-email-bremner at debian.org"
>
> Changes since v2:
>
> - clean up new test-binaries and objects
>
> - remove the "set -o pipefail" leftover from debugging.  Possibly this
>   makes sense as a global setting, but in a seperate patch.
>
> - add hex-escape to test/basic
>
> - rebase against updated master.

Hi! This looks pretty good to me and I am for improving the test
infrastructure.

Some minor problems:

- Patch 2 doesn't apply; neither do patches 4 or 5, presumably due to changes
  that weren't made due to patch 2.

- Commit message discipline: the subject line of patch 4 ends in a
  period. "Seperate" is spelled by most people as "separate", though I
  would encourage you to buck the trend if you are so inclined.

- In patch 4:

> +if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
> +   _notmuch_message_add_term (message, "type", "mail");
> +} else {
> +   return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
> +}

Why not switch the branches? That is, check for private_status !=
NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND and return immediately?

- In patch 5:

> +for (count = 0; count < num_messages; count++) {
> + int j;
> + int num_tags = random () % (max_tags + 1);
> + int this_mid_len = random () % message_id_len + 1;

This looks odd. I'm pretty sure it's correct, but my brain keeps saying,
"Why are there no parentheses on (message_id_len + 1)?" Maybe just a
comment that message ids must be at least one character long, or the
ranges of values necessary for both of these variables.

Ethan

Re: [PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t

[Ethan Glasser-Camp]

Jani Nikula j...@nikula.org writes:

 On Wed, 17 Oct 2012, Adrien Bustany adr...@bustany.org wrote:
 The code of the patches in unchanged, but the formatting issues are now
 hopefully fixed.

 Hi Adrien, please check at what version flush and reopen have been
 introduced to xapian. If they are new-ish (I don't know, didn't have the
 time to check), please add appropriate #ifdefs. [1] lays the groundwork
 for this. We'll also need to decide what is the minimum xapian version
 required in general, i.e. features earlier than that don't need
 conditional compilation.

Hi! The new versions of these patches are still pretty trivial and they
still look OK to me, but based on Jani's prompting I decided to look up
the methods. Seems that flush() is a very old (pre-1.1.0, 2009-04) name
for commit(), which is the preferred name these days. You should
probably therefore rename the function notmuch_database_commit, and have
it call the WritableDatabase::commit() method.

reopen() is a very very old method, seems like it has been around since
2004.

So I think Adrien is safe from having to do version checks, but we
should probably use commit() instead of flush().

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

Re: [PATCHv3] notmuch-show: include Bcc header in json output

[Ethan Glasser-Camp]

Michal Nazarewicz m...@google.com writes:

 From: Michal Nazarewicz min...@mina86.com

 With this change, emacs users can use notmuch-message-headers
 variable to configure notmuch-show display Bcc header.
 ---

This patch looks pretty straightforward and has seen a certain amount of
review so I'm taking off needs-review.

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

Re: [PATCH V3 1/2] test/smtp-dummy: add --background option and functionality

[Ethan Glasser-Camp]

Tomi Ollila tomi.oll...@iki.fi writes:

 From: Tomi Ollila t...@iki.fi

 When shell executes background process using '' the scheduling of
 that new process is arbitrary. It could be that smtp-dummy doesn't
 get execution time to listen() it's server socket until some other
 process attempts to connect() to it. The --background option in
 smtp-dummy makes it to go background *after* it started to listen
 its server socket.

This looks fine to me, Michal Sojka's reviewed this version, and this
patch has been bouncing around for almost a year! I'm taking off
needs-review.

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

[PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative

[Ethan Glasser-Camp]

Jameson Graef Rollins  writes:

> +diff OUTPUT.{text,html} >OUTPUT.diff
> +cat  +7,9c7,10
> +< [ text/html (not shown) ]
> +< [ text/plain ]
> +< This is the text/plain part of a multipart/alternative.
> +---
> +> [ text/html ]
> +> This is the text/html part of a multipart/alternative.
> +> 
> +> [ text/plain (not shown) ]
> +EOF

Hmm, old-school diff. I guess this (instead of unified diff) is so that
if the diff isn't like this, then the diff of the diffs are clearly
readable? :)

git am complains at me because this patch introduces trailing whitespace
(to mimic, of course, the output of diff which will have trailing
whitespace as well). I would love for you to write this file using some
other technique which would not require putting trailing whitespace in
the test file.

Additionally the patch is stale, perhaps because my patch that moved some tests
to emacs-show was just pushed too.

Ethan

[PATCH v2 3/3] test: conform to content length, encoding fields

[Ethan Glasser-Camp]

Peter Wang  writes:

> Update tests to expect content-length and content-transfer-encoding
> fields in show --format=json output, for leaf parts with omitted body
> content.

These three patches all look fine to me, except for the following
problem.

> diff --git a/test/json b/test/json
> index ac8fa8e..8ce2e8a 100755
> --- a/test/json
> +++ b/test/json
> @@ -45,7 +45,7 @@ emacs_deliver_message \
>   (insert \"Message-ID: <$id>\n\")"
>  output=$(notmuch show --format=json "id:$id")
>  filename=$(notmuch search --output=files "id:$id")
> -test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, 
> \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, 
> \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": 
> {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite  notmuchmail.org>\", \"To\": \"test_suite at notmuchmail.org\", \"Date\": 
> \"Sat, 01 Jan 2000 12:00:00 +\"}, \"body\": [{\"id\": 1, 
> \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, 
> \"content-type\": \"text/plain\", \"content\": \"This is a test message with 
> inline attachment with a filename\"}, {\"id\": 3, \"content-type\": 
> \"application/octet-stream\", \"filename\": \"README\"}]}]}, ["
> +test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, 
> \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, 
> \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": 
> {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite  notmuchmail.org>\", \"To\": \"test_suite at notmuchmail.org\", \"Date\": 
> \"Sat, 01 Jan 2000 12:00:00 +\"}, \"body\": [{\"id\": 1, 
> \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, 
> \"content-type\": \"text/plain\", \"content\": \"This is a test message with 
> inline attachment with a filename\"}, {\"id\": 3, \"content-type\": 
> \"application/octet-stream\", \"content-length\": 12392, 
> \"content-transfer-encoding\": \"base64\", \"filename\": \"README\"}]}]}, 
> ["

This test fails for me. You're encoding the content-length of
test/README. test/README certainly hasn't changed in the last six months
so that seems like a reasonable thing to do... except then why is it
12392 on your machine, and 12380 on mine? I don't object to using
test/README as a simple file to test with, but then you certainly
shouldn't hard-code its length. You could pipe test/README through
base64 and then through wc -c to get an accurate length, but for my
machine a newline gets appended by base64 I think, and it gives me
12381.

I'm tagging this patch moreinfo but you would have my +1 if you fix
this.

Ethan

[PATCH 1/4] show: indicate length of omitted body content (json)

[Ethan Glasser-Camp]

Peter Wang  writes:

> If a leaf part's body content is omitted, return the content length in
> --format=json output.  This information may be used by the consumer,
> e.g. to decide whether to download a large attachment over a slow link.

It looks like this patch series was thoroughly reviewed and then
obsoleted by the series in
id:"1344428872-12374-1-git-send-email-novalazy at gmail.com". I'm therefore
marking it notmuch::obsolete, and will review the other patch set.

Ethan

[PATCH v2 (Draft)] emacs: split async json parser into utility function

[Ethan Glasser-Camp]

Mark Walters  writes:

> Split out the json parser into a utility function.
> ---
>
> Most of this patch is code movement: but I don't see how to arrange the
> patch to show that.

Hi! This looks like a straightforward patch and if it will make
notmuch-pick more efficient, I'm in favor.

I tagged this patch moreinfo because David Bremner's suggestions that
you expand on the docstrings for notmuch-json-parser and
notmuch-json-state are good ideas. I'd also propose that you split this
patch into two patches -- one that pulls out the variables and performs
the renames, and the other which breaks out the code into its own
function. This should make the code movement more obvious. I haven't
started full-time work yet so if you would like me to do this, I can ;)

Based on David Bremner's feedback that it might be a good idea to have a
commit message that explains exactly what is code movement and isn't,
here's my proposal for a commit message.

Split out the json parser into a utility function.

This patch breaks out a chunk of notmuch-search-process-filter, with the
following changes:

- notmuch-search-json-parser becomes notmuch-json-parser.
- notmuch-search-parser-state becomes notmuch-json-state.

We also rearrange the json-error case but are careful to always call
error-function in the results buffer.



Ethan

[PATCH] emacs: functions to import sender or recipient into BBDB

[Ethan Glasser-Camp]

Daniel Bergey  writes:

> From a show buffer, bbdb/notmuch-snarf-from imports the sender into
> bbdb.  bbdb/notmuch-snarf-to attempts to import all recipients.  BBDB
> displays a buffer with each contact; C-g displays the next contact, or
> returns to the notmuch-show buffer.
>
> This is my first notmuch patch.  Comments very welcome.

Hi!

>  emacs/notmuch-show.el |   28 

I don't think this belongs in notmuch-show. My first inclination is that
this should go into a new file contrib/notmuch-bbdb.el (assuming there's
no other notmuch-bbdb integration stuff floating around).

>  1 file changed, 28 insertions(+)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 6335d45..3bc1da0 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1895,6 +1895,34 @@ the user (see 
> `notmuch-show-stash-mlarchive-link-alist')."
>  (button-get button :notmuch-filename)
>  (button-get button :notmuch-content-type)))
>
> +;; bbdb interaction functions, awaiting user keybindings
> +
> +(defun bbdb/snarf-between-commas ()
> +  ; What about names written "Surname, First M" ?

Most comments in emacslisp start with two semicolons.

I do think more sophisticated parsing is necessary. If you're lucky,
somebody else already has a library to parse email addresses in this
form.

> +  (goto-char (point-min))

I'm not crazy about this. It's probably fine for something limited to
bbdb users (especially since bbdb-snarf uses a very similar technique),
but I think the better approach here is to just take a region and go
from region-beginning and region-end.

> +  (let ((comma (point)))
> +(while (search-forward "," nil "end")

The third argument of search-forward is NOERROR. I don't understand what
the value "end" means. The help says "Optional third argument, if t..."

> +  (bbdb-snarf-region comma (point))
> +  (setq comma (point)))
> +(bbdb-snarf-region comma (point)) ; last entry
> +   ))

Doesn't this cause snarf the comma into any of those entries? It seems
like point starts before the first entry but then goes before each
comma. Obviously this wouldn't be here if it didn't work. I thought
bbdb-snarf handled this kind of thing, but it doesn't. Could you explain
this?

Ethan

[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Ethan Glasser-Camp]

Ethan Glasser-Camp  writes:

> It looks like you have better wording for patch 4/8 so I'd like to see
> you resend it.
>
> I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit!

It turns out that patch 4 already has a v2 in the thread, but I didn't
see it due to some kind of selective blindness. It would be nice if
nmbug had grouped it as part of the same patch series. 

I'm marking the old patch 4/8 obsolete. Only patch 3 and 7 remain.

Ethan

[PATCH v2] emacs: add function to toggle display of all multipart/alternative parts

[Ethan Glasser-Camp]

Mark Walters  writes:

> Some messages are sent as multipart/alternative but the alternatives
> contain different information. This allows the user to cycle which
> part to view. By default this is bound to 'W'.
> ---
>
> This version at least uses the notmuch escaping for message-id which
> makes me a bit happier: it probably doesn't have any nasty security
> flaws. I do still feel that the lisp is a bit ugly though.

For what it's worth, I don't feel that this code is horrible. It seems
like there remain design decisions to be made about how notmuch show
"ought" to handle multipart/alternatives, but I can at least comment on
this code.

First, the use of a plist looks fine to me because most of the time it's
going to have length 0. At most it will have one entry per message -- a
few hundred. So I'm not worried about efficiency concerns.

>  (defcustom notmuch-show-stash-mlarchive-link-alist
>'(("Gmane" . "http://mid.gmane.org/;)
>  ("MARC" . "http://marc.info/?i=;)
> @@ -536,9 +540,19 @@ message at DEPTH in the current thread."
>  
>  (defun notmuch-show-insert-part-multipart/alternative (msg part content-type 
> nth depth declared-type)
>(notmuch-show-insert-part-header nth declared-type content-type nil)
> -  (let ((chosen-type (car (notmuch-multipart/alternative-choose 
> (notmuch-show-multipart/*-to-list part
> - (inner-parts (plist-get part :content))
> - (start (point)))
> +  (let* ((chosen-nth (or (lax-plist-get 
> notmuch-show-message-multipart/alternative-display-part
> + (notmuch-id-to-query (plist-get msg 
> :id))) 0))
> +  (chosen-type (nth chosen-nth
> +   (notmuch-multipart/alternative-choose 
> (notmuch-show-multipart/*-to-list part
> +  (inner-parts (plist-get part :content))
> +  (start (point)))

Changing let to let* makes me the slightest bit uneasy, although I'm not
sure I could explain why.

It would be nice if you could wrap the manipulation of
notmuch-show-message-multipart/alternative-display-part in functions,
ideally with names that are shorter than the variable they
manipulate. Specifically, I think the definition of chosen-nth (which is
almost repeated below) could be its own function, something like
(notmuch-show-message-current-multipart msg), which could take a msg-id
or a plist and do the plist-get and id-to-query that you do here.

> +;; If we have run out of possible content-types restart from the 
> beginning
> +(unless chosen-type
> +  (setq chosen-type (car (notmuch-multipart/alternative-choose 
> (notmuch-show-multipart/*-to-list part
> +  (setq notmuch-show-message-multipart/alternative-display-part
> + (lax-plist-put 
> notmuch-show-message-multipart/alternative-display-part
> +(notmuch-id-to-query (plist-get msg :id)) 0)))
> +
>  ;; This inserts all parts of the chosen type rather than just one,
>  ;; but it's not clear that this is the wrong thing to do - which
>  ;; should be chosen if there are more than one that match?
> @@ -942,6 +956,16 @@ message at DEPTH in the current thread."
>"Not processing cryptographic MIME parts."))
>(notmuch-show-refresh-view))
>  
> +(defun notmuch-show-cycle-message-multipart ()
> +  "Cycle which part to display of a multipart messageToggle the display of 
> non-matching messages."

This docstring is broken.

> +  (interactive)
> +  (let* ((msg-id (notmuch-show-get-message-id))
> +  (next-part (1+ (or (lax-plist-get 
> notmuch-show-message-multipart/alternative-display-part msg-id) 0
> +(setq notmuch-show-message-multipart/alternative-display-part
> + (lax-plist-put notmuch-show-message-multipart/alternative-display-part 
> msg-id next-part))
> +(message "Cycling multipart/alternative for current message")
> +(notmuch-show-refresh-view)))

Maybe move the reset-and-go-back-to-zero behavior to this function
instead of in the display function. This opens you up to weird
situations if one of the multipart/alternatives should disappear from a
message or if some other function should change the alternative on
display for a given message, but both of these seem unlikely to me..

Ethan

[PATCH] Add NEWS item for multipart/alternative toggle

[Ethan Glasser-Camp]

Users who relied on notmuch-show-all-multipart/alternative-parts
might need to know that it is now buffer-local.

Signed-off-by: Ethan Glasser-Camp 
---
Hi! I'm trying to figure out the status of this patch series, which
seems to have fallen through the cracks. It looks like Jani's solution
exists and works, though it isn't perfect. I would like to get Jani's
solution applied until someone who is sufficiently motivated decides to
work on fancy cycle-multipart-message things. It seems like the last
hurdle is that Mark would like Jani to put the buffer-localization of
notmuch-show-all-multipart/alternative-parts in NEWS. Here's that.

However, it also seems like there was a disagreement about design
direction -- should multiple alternative parts be toggled so you can
see them all at once, or cycled so you can see one at a time? See
id:"87d32yadl5.fsf at qmul.ac.uk". I'm for the code that works now which
is Jani's patch.

 NEWS |   11 +++
 1 file changed, 11 insertions(+)

diff --git a/NEWS b/NEWS
index 2b50ba3..fc9b50a 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,14 @@
+Emacs Interface
+---
+
+Display of multipart messages
+
+  Displaying multipart/alternative parts has been made a toggle, the
+  same way as indentation and message decryption.
+  notmuch-show-all-multipart/alternative-parts is now buffer-local, so
+  if you have set it using setq, you will have to update that to use
+  setq-default.
+
 Notmuch 0.14 (2012-08-20)
 =

-- 
1.7.9.5

[PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Ethan Glasser-Camp]

Peter Wang  writes:

> Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
> cover all four values of search --exclude in the cli.

This series looks good to me. It's a nice clean up and a nice new
feature. Patches all apply.

However, I'm getting test failures like:

 FAIL   Search, exclude "deleted" messages from message search --exclude=false
--- excludes.3.expected 2012-10-19 04:45:06.900518377 +
+++ excludes.3.output   2012-10-19 04:45:06.900518377 +
@@ -1,2 +1,2 @@
-id:msg-001 at notmuch-test-suite
 id:msg-002 at notmuch-test-suite
+id:msg-001 at notmuch-test-suite

 FAIL   Search, don't exclude "deleted" messages when --exclude=flag specified
--- excludes.7.expected 2012-10-19 04:45:07.004518378 +
+++ excludes.7.output   2012-10-19 04:45:07.004518378 +
@@ -1,2 +1,2 @@
-thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)
 thread:XXX   2001-01-05 [1/2] Notmuch Test Suite; Not deleted reply 
(deleted inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)

 FAIL   Search, don't exclude "deleted" messages from search if not configured
--- excludes.8.expected 2012-10-19 04:45:07.028518377 +
+++ excludes.8.output   2012-10-19 04:45:07.028518377 +
@@ -1,2 +1,2 @@
-thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)
 thread:XXX   2001-01-05 [2/2] Notmuch Test Suite; Deleted (deleted 
inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)

In other words, threads and messages are coming up out of order. I'm not
sure of the right way to fix this. If you would like me to try sticking
"| sort" here and there in the tests I will do so. I'm not sure if the
test suite is guaranteed to scan messages in a certain order.

Mark Walters wrote in
id:"1340198947-29370-5-git-send-email-novalazy at gmail.com" that he
thought patch 1/8 seemed more intrusive than he liked. Maybe I just have
a higher standard for "intrusive" than he does ;) but I thought it was
fine.

It looks like you have better wording for patch 4/8 so I'd like to see
you resend it.

> - if (query->omit_excluded != NOTMUCH_EXCLUDE_FALSE)
> + if (query->omit_excluded == NOTMUCH_EXCLUDE_TRUE ||
> + query->omit_excluded == NOTMUCH_EXCLUDE_ALL)
> + {
>   final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
>final_query, exclude_query);
> - else {
> + } else {

"House style" is to not put braces around one-line then-clauses. This is
the only place where you did that.

I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit!

Ethan

[PATCH 3/7] go: Allow notmuch objects to be garbage collected

[Ethan Glasser-Camp]

Adrien Bustany  writes:

> This makes notmuch appropriately free the underlying notmuch C objects
> when garbage collecting their Go wrappers. To make sure we don't break
> the underlying links between objects (for example, a notmuch_messages_t
> being GC'ed before a notmuch_message_t belonging to it), we add for each
> wraper struct a pointer to the owner object (Go objects with a reference
> pointing to them don't get garbage collected).

Hi Adrien! This whole series is marked moreinfo, but I don't think
that's just. It looks like there were some unresolved issues about
reference tracking and garbage collection, and some suggestions to use
the C values of enums instead of regenerating them with iota, but
there's definitely valid code that I assume would be useful if anyone
ever wanted to write in Go ;). Are you figuring to clean this series up?

This comment should s/wraper/wrapper/.

Ethan

Notmuch scripts (again), now with more usenet

[Ethan Glasser-Camp]

ccx at webprojekty.cz writes:

> Hello, for quite some time my set of scripts just lied in my repo and
> waited for polish before release. So tonight I finally managed to update
> the docs, remove old stuff, rewrite some unfortunate things etc.
>
> One notable addition is slrn2maildir script which can convert NNTP
> spool, eg. gmane mailing lists or blogs, as fetched by slrnpull to
> maildir format. This way you can follow plethora of mailing lists
> without subscribing, any blog that publishes full atom/rss feed or
> usenet newsgroup.
>
> For details see the readme:
>   http://webprojekty.cz/ccx/loggerhead/zmuch/view/head:/README
> or check out the code:
>   bzr branch http://webprojekty.cz/ccx/bzr/zmuch
>
> I hope it's now in the form acceptable for inclusion to contrib.

Hi! Sorry about the delay, but I'm going through the patch queue now and
it seems like this branch is just completely gone. I get 502 Bad Gateway
errors when I follow the first link. Did it move or is there a problem
with your site?

Ethan

[PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t

[Ethan Glasser-Camp]

Adrien Bustany  writes:

> The code of the patches in unchanged, but the formatting issues are now
> hopefully fixed.

These look fine to me, and they're pretty trivial.

Ethan

[PATCH] Add NEWS item for multipart/alternative toggle

[Ethan Glasser-Camp]

Users who relied on notmuch-show-all-multipart/alternative-parts
might need to know that it is now buffer-local.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
Hi! I'm trying to figure out the status of this patch series, which
seems to have fallen through the cracks. It looks like Jani's solution
exists and works, though it isn't perfect. I would like to get Jani's
solution applied until someone who is sufficiently motivated decides to
work on fancy cycle-multipart-message things. It seems like the last
hurdle is that Mark would like Jani to put the buffer-localization of
notmuch-show-all-multipart/alternative-parts in NEWS. Here's that.

However, it also seems like there was a disagreement about design
direction -- should multiple alternative parts be toggled so you can
see them all at once, or cycled so you can see one at a time? See
id:87d32yadl5@qmul.ac.uk. I'm for the code that works now which
is Jani's patch.

 NEWS |   11 +++
 1 file changed, 11 insertions(+)

diff --git a/NEWS b/NEWS
index 2b50ba3..fc9b50a 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,14 @@
+Emacs Interface
+---
+
+Display of multipart messages
+
+  Displaying multipart/alternative parts has been made a toggle, the
+  same way as indentation and message decryption.
+  notmuch-show-all-multipart/alternative-parts is now buffer-local, so
+  if you have set it using setq, you will have to update that to use
+  setq-default.
+
 Notmuch 0.14 (2012-08-20)
 =
 
-- 
1.7.9.5

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

Re: [PATCH v2] emacs: add function to toggle display of all multipart/alternative parts

[Ethan Glasser-Camp]

Mark Walters markwalters1...@gmail.com writes:

 Some messages are sent as multipart/alternative but the alternatives
 contain different information. This allows the user to cycle which
 part to view. By default this is bound to 'W'.
 ---

 This version at least uses the notmuch escaping for message-id which
 makes me a bit happier: it probably doesn't have any nasty security
 flaws. I do still feel that the lisp is a bit ugly though.

For what it's worth, I don't feel that this code is horrible. It seems
like there remain design decisions to be made about how notmuch show
ought to handle multipart/alternatives, but I can at least comment on
this code.

First, the use of a plist looks fine to me because most of the time it's
going to have length 0. At most it will have one entry per message -- a
few hundred. So I'm not worried about efficiency concerns.

  (defcustom notmuch-show-stash-mlarchive-link-alist
'((Gmane . http://mid.gmane.org/;)
  (MARC . http://marc.info/?i=;)
 @@ -536,9 +540,19 @@ message at DEPTH in the current thread.
  
  (defun notmuch-show-insert-part-multipart/alternative (msg part content-type 
 nth depth declared-type)
(notmuch-show-insert-part-header nth declared-type content-type nil)
 -  (let ((chosen-type (car (notmuch-multipart/alternative-choose 
 (notmuch-show-multipart/*-to-list part
 - (inner-parts (plist-get part :content))
 - (start (point)))
 +  (let* ((chosen-nth (or (lax-plist-get 
 notmuch-show-message-multipart/alternative-display-part
 + (notmuch-id-to-query (plist-get msg 
 :id))) 0))
 +  (chosen-type (nth chosen-nth
 +   (notmuch-multipart/alternative-choose 
 (notmuch-show-multipart/*-to-list part
 +  (inner-parts (plist-get part :content))
 +  (start (point)))

Changing let to let* makes me the slightest bit uneasy, although I'm not
sure I could explain why.

It would be nice if you could wrap the manipulation of
notmuch-show-message-multipart/alternative-display-part in functions,
ideally with names that are shorter than the variable they
manipulate. Specifically, I think the definition of chosen-nth (which is
almost repeated below) could be its own function, something like
(notmuch-show-message-current-multipart msg), which could take a msg-id
or a plist and do the plist-get and id-to-query that you do here.

 +;; If we have run out of possible content-types restart from the 
 beginning
 +(unless chosen-type
 +  (setq chosen-type (car (notmuch-multipart/alternative-choose 
 (notmuch-show-multipart/*-to-list part
 +  (setq notmuch-show-message-multipart/alternative-display-part
 + (lax-plist-put 
 notmuch-show-message-multipart/alternative-display-part
 +(notmuch-id-to-query (plist-get msg :id)) 0)))
 +
  ;; This inserts all parts of the chosen type rather than just one,
  ;; but it's not clear that this is the wrong thing to do - which
  ;; should be chosen if there are more than one that match?
 @@ -942,6 +956,16 @@ message at DEPTH in the current thread.
Not processing cryptographic MIME parts.))
(notmuch-show-refresh-view))
  
 +(defun notmuch-show-cycle-message-multipart ()
 +  Cycle which part to display of a multipart messageToggle the display of 
 non-matching messages.

This docstring is broken.

 +  (interactive)
 +  (let* ((msg-id (notmuch-show-get-message-id))
 +  (next-part (1+ (or (lax-plist-get 
 notmuch-show-message-multipart/alternative-display-part msg-id) 0
 +(setq notmuch-show-message-multipart/alternative-display-part
 + (lax-plist-put notmuch-show-message-multipart/alternative-display-part 
 msg-id next-part))
 +(message Cycling multipart/alternative for current message)
 +(notmuch-show-refresh-view)))

Maybe move the reset-and-go-back-to-zero behavior to this function
instead of in the display function. This opens you up to weird
situations if one of the multipart/alternatives should disappear from a
message or if some other function should change the alternative on
display for a given message, but both of these seem unlikely to me..

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

Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Ethan Glasser-Camp]

Ethan Glasser-Camp ethan.glasser.c...@gmail.com writes:

 It looks like you have better wording for patch 4/8 so I'd like to see
 you resend it.

 I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit!

It turns out that patch 4 already has a v2 in the thread, but I didn't
see it due to some kind of selective blindness. It would be nice if
nmbug had grouped it as part of the same patch series. 

I'm marking the old patch 4/8 obsolete. Only patch 3 and 7 remain.

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

Re: [PATCH] emacs: functions to import sender or recipient into BBDB

[Ethan Glasser-Camp]

Daniel Bergey ber...@alum.mit.edu writes:

 From a show buffer, bbdb/notmuch-snarf-from imports the sender into
 bbdb.  bbdb/notmuch-snarf-to attempts to import all recipients.  BBDB
 displays a buffer with each contact; C-g displays the next contact, or
 returns to the notmuch-show buffer.

 This is my first notmuch patch.  Comments very welcome.

Hi!

  emacs/notmuch-show.el |   28 

I don't think this belongs in notmuch-show. My first inclination is that
this should go into a new file contrib/notmuch-bbdb.el (assuming there's
no other notmuch-bbdb integration stuff floating around).

  1 file changed, 28 insertions(+)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index 6335d45..3bc1da0 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -1895,6 +1895,34 @@ the user (see 
 `notmuch-show-stash-mlarchive-link-alist').
  (button-get button :notmuch-filename)
  (button-get button :notmuch-content-type)))

 +;; bbdb interaction functions, awaiting user keybindings
 +
 +(defun bbdb/snarf-between-commas ()
 +  ; What about names written Surname, First M u...@server.tld?

Most comments in emacslisp start with two semicolons.

I do think more sophisticated parsing is necessary. If you're lucky,
somebody else already has a library to parse email addresses in this
form.

 +  (goto-char (point-min))

I'm not crazy about this. It's probably fine for something limited to
bbdb users (especially since bbdb-snarf uses a very similar technique),
but I think the better approach here is to just take a region and go
from region-beginning and region-end.

 +  (let ((comma (point)))
 +(while (search-forward , nil end)

The third argument of search-forward is NOERROR. I don't understand what
the value end means. The help says Optional third argument, if t...

 +  (bbdb-snarf-region comma (point))
 +  (setq comma (point)))
 +(bbdb-snarf-region comma (point)) ; last entry
 +   ))

Doesn't this cause snarf the comma into any of those entries? It seems
like point starts before the first entry but then goes before each
comma. Obviously this wouldn't be here if it didn't work. I thought
bbdb-snarf handled this kind of thing, but it doesn't. Could you explain
this?

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

Re: [PATCH v2 (Draft)] emacs: split async json parser into utility function

[Ethan Glasser-Camp]

Mark Walters markwalters1...@gmail.com writes:

 Split out the json parser into a utility function.
 ---

 Most of this patch is code movement: but I don't see how to arrange the
 patch to show that.

Hi! This looks like a straightforward patch and if it will make
notmuch-pick more efficient, I'm in favor.

I tagged this patch moreinfo because David Bremner's suggestions that
you expand on the docstrings for notmuch-json-parser and
notmuch-json-state are good ideas. I'd also propose that you split this
patch into two patches -- one that pulls out the variables and performs
the renames, and the other which breaks out the code into its own
function. This should make the code movement more obvious. I haven't
started full-time work yet so if you would like me to do this, I can ;)

Based on David Bremner's feedback that it might be a good idea to have a
commit message that explains exactly what is code movement and isn't,
here's my proposal for a commit message.

Split out the json parser into a utility function.

This patch breaks out a chunk of notmuch-search-process-filter, with the
following changes:

- notmuch-search-json-parser becomes notmuch-json-parser.
- notmuch-search-parser-state becomes notmuch-json-state.

We also rearrange the json-error case but are careful to always call
error-function in the results buffer.



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

Re: [PATCH 1/4] show: indicate length of omitted body content (json)

[Ethan Glasser-Camp]

Peter Wang noval...@gmail.com writes:

 If a leaf part's body content is omitted, return the content length in
 --format=json output.  This information may be used by the consumer,
 e.g. to decide whether to download a large attachment over a slow link.

It looks like this patch series was thoroughly reviewed and then
obsoleted by the series in
id:1344428872-12374-1-git-send-email-noval...@gmail.com. I'm therefore
marking it notmuch::obsolete, and will review the other patch set.

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

Re: [PATCH v2 3/3] test: conform to content length, encoding fields

[Ethan Glasser-Camp]

Peter Wang noval...@gmail.com writes:

 Update tests to expect content-length and content-transfer-encoding
 fields in show --format=json output, for leaf parts with omitted body
 content.

These three patches all look fine to me, except for the following
problem.

 diff --git a/test/json b/test/json
 index ac8fa8e..8ce2e8a 100755
 --- a/test/json
 +++ b/test/json
 @@ -45,7 +45,7 @@ emacs_deliver_message \
   (insert \Message-ID: $id\n\)
  output=$(notmuch show --format=json id:$id)
  filename=$(notmuch search --output=files id:$id)
 -test_expect_equal_json $output [[[{\id\: \$id\, \match\: true, 
 \excluded\: false, \filename\: \$filename\, \timestamp\: 946728000, 
 \date_relative\: \2000-01-01\, \tags\: [\inbox\], \headers\: 
 {\Subject\: \$subject\, \From\: \Notmuch Test Suite 
 test_su...@notmuchmail.org\, \To\: \test_su...@notmuchmail.org\, 
 \Date\: \Sat, 01 Jan 2000 12:00:00 +\}, \body\: [{\id\: 1, 
 \content-type\: \multipart/mixed\, \content\: [{\id\: 2, 
 \content-type\: \text/plain\, \content\: \This is a test message with 
 inline attachment with a filename\}, {\id\: 3, \content-type\: 
 \application/octet-stream\, \filename\: \README\}]}]}, [
 +test_expect_equal_json $output [[[{\id\: \$id\, \match\: true, 
 \excluded\: false, \filename\: \$filename\, \timestamp\: 946728000, 
 \date_relative\: \2000-01-01\, \tags\: [\inbox\], \headers\: 
 {\Subject\: \$subject\, \From\: \Notmuch Test Suite 
 test_su...@notmuchmail.org\, \To\: \test_su...@notmuchmail.org\, 
 \Date\: \Sat, 01 Jan 2000 12:00:00 +\}, \body\: [{\id\: 1, 
 \content-type\: \multipart/mixed\, \content\: [{\id\: 2, 
 \content-type\: \text/plain\, \content\: \This is a test message with 
 inline attachment with a filename\}, {\id\: 3, \content-type\: 
 \application/octet-stream\, \content-length\: 12392, 
 \content-transfer-encoding\: \base64\, \filename\: \README\}]}]}, 
 [

This test fails for me. You're encoding the content-length of
test/README. test/README certainly hasn't changed in the last six months
so that seems like a reasonable thing to do... except then why is it
12392 on your machine, and 12380 on mine? I don't object to using
test/README as a simple file to test with, but then you certainly
shouldn't hard-code its length. You could pipe test/README through
base64 and then through wc -c to get an accurate length, but for my
machine a newline gets appended by base64 I think, and it gives me
12381.

I'm tagging this patch moreinfo but you would have my +1 if you fix
this.

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

Re: [PATCH 1/2] test: add emacs show mode test for toggling display of multipart/alternative

[Ethan Glasser-Camp]

Jameson Graef Rollins jroll...@finestructure.net writes:

 +diff OUTPUT.{text,html} OUTPUT.diff
 +cat EOF EXPECTED.diff
 +7,9c7,10
 + [ text/html (not shown) ]
 + [ text/plain ]
 + This is the text/plain part of a multipart/alternative.
 +---
 + [ text/html ]
 + This is the text/html part of a multipart/alternative.
 + 
 + [ text/plain (not shown) ]
 +EOF

Hmm, old-school diff. I guess this (instead of unified diff) is so that
if the diff isn't like this, then the diff of the diffs are clearly
readable? :)

git am complains at me because this patch introduces trailing whitespace
(to mimic, of course, the output of diff which will have trailing
whitespace as well). I would love for you to write this file using some
other technique which would not require putting trailing whitespace in
the test file.

Additionally the patch is stale, perhaps because my patch that moved some tests
to emacs-show was just pushed too.

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

Re: random corpus generator, v3

[Ethan Glasser-Camp]

da...@tethera.net writes:

 This obsoletes the series at:

  id:134431-4301-1-git-send-email-brem...@debian.org

 Changes since v2:

 - clean up new test-binaries and objects

 - remove the set -o pipefail leftover from debugging.  Possibly this
   makes sense as a global setting, but in a seperate patch.

 - add hex-escape to test/basic

 - rebase against updated master.

Hi! This looks pretty good to me and I am for improving the test
infrastructure.

Some minor problems:

- Patch 2 doesn't apply; neither do patches 4 or 5, presumably due to changes
  that weren't made due to patch 2.

- Commit message discipline: the subject line of patch 4 ends in a
  period. Seperate is spelled by most people as separate, though I
  would encourage you to buck the trend if you are so inclined.

- In patch 4:

 +if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
 +   _notmuch_message_add_term (message, type, mail);
 +} else {
 +   return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
 +}

Why not switch the branches? That is, check for private_status !=
NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND and return immediately?

- In patch 5:

 +for (count = 0; count  num_messages; count++) {
 + int j;
 + int num_tags = random () % (max_tags + 1);
 + int this_mid_len = random () % message_id_len + 1;

This looks odd. I'm pretty sure it's correct, but my brain keeps saying,
Why are there no parentheses on (message_id_len + 1)? Maybe just a
comment that message ids must be at least one character long, or the
ranges of values necessary for both of these variables.

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

Re: [PATCH v2 0/2] Add flush/reopen methods to notmuch_database_t

[Ethan Glasser-Camp]

Adrien Bustany adr...@bustany.org writes:

 The code of the patches in unchanged, but the formatting issues are now
 hopefully fixed.

These look fine to me, and they're pretty trivial.

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

Re: Notmuch scripts (again), now with more usenet

[Ethan Glasser-Camp]

c...@webprojekty.cz writes:

 Hello, for quite some time my set of scripts just lied in my repo and
 waited for polish before release. So tonight I finally managed to update
 the docs, remove old stuff, rewrite some unfortunate things etc.

 One notable addition is slrn2maildir script which can convert NNTP
 spool, eg. gmane mailing lists or blogs, as fetched by slrnpull to
 maildir format. This way you can follow plethora of mailing lists
 without subscribing, any blog that publishes full atom/rss feed or
 usenet newsgroup.

 For details see the readme:
   http://webprojekty.cz/ccx/loggerhead/zmuch/view/head:/README
 or check out the code:
   bzr branch http://webprojekty.cz/ccx/bzr/zmuch

 I hope it's now in the form acceptable for inclusion to contrib.

Hi! Sorry about the delay, but I'm going through the patch queue now and
it seems like this branch is just completely gone. I get 502 Bad Gateway
errors when I follow the first link. Did it move or is there a problem
with your site?

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

Re: [PATCH 7/8] lib: add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t

[Ethan Glasser-Camp]

Peter Wang noval...@gmail.com writes:

 Add NOTMUCH_EXCLUDE_FLAG to notmuch_exclude_t so that it can
 cover all four values of search --exclude in the cli.

This series looks good to me. It's a nice clean up and a nice new
feature. Patches all apply.

However, I'm getting test failures like:

 FAIL   Search, exclude deleted messages from message search --exclude=false
--- excludes.3.expected 2012-10-19 04:45:06.900518377 +
+++ excludes.3.output   2012-10-19 04:45:06.900518377 +
@@ -1,2 +1,2 @@
-id:msg-001@notmuch-test-suite
 id:msg-002@notmuch-test-suite
+id:msg-001@notmuch-test-suite

 FAIL   Search, don't exclude deleted messages when --exclude=flag specified
--- excludes.7.expected 2012-10-19 04:45:07.004518378 +
+++ excludes.7.output   2012-10-19 04:45:07.004518378 +
@@ -1,2 +1,2 @@
-thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)
 thread:XXX   2001-01-05 [1/2] Notmuch Test Suite; Not deleted reply 
(deleted inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)

 FAIL   Search, don't exclude deleted messages from search if not configured
--- excludes.8.expected 2012-10-19 04:45:07.028518377 +
+++ excludes.8.output   2012-10-19 04:45:07.028518377 +
@@ -1,2 +1,2 @@
-thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)
 thread:XXX   2001-01-05 [2/2] Notmuch Test Suite; Deleted (deleted 
inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox 
unread)

In other words, threads and messages are coming up out of order. I'm not
sure of the right way to fix this. If you would like me to try sticking
| sort here and there in the tests I will do so. I'm not sure if the
test suite is guaranteed to scan messages in a certain order.

Mark Walters wrote in
id:1340198947-29370-5-git-send-email-noval...@gmail.com that he
thought patch 1/8 seemed more intrusive than he liked. Maybe I just have
a higher standard for intrusive than he does ;) but I thought it was
fine.

It looks like you have better wording for patch 4/8 so I'd like to see
you resend it.

 - if (query-omit_excluded != NOTMUCH_EXCLUDE_FALSE)
 + if (query-omit_excluded == NOTMUCH_EXCLUDE_TRUE ||
 + query-omit_excluded == NOTMUCH_EXCLUDE_ALL)
 + {
   final_query = Xapian::Query (Xapian::Query::OP_AND_NOT,
final_query, exclude_query);
 - else {
 + } else {

House style is to not put braces around one-line then-clauses. This is
the only place where you did that.

I'm marking patches 3, 4, and 7 as moreinfo. Please resubmit!

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

[PATCH] test: Move tests from emacs to emacs-show

[Ethan Glasser-Camp]

This requires changing the contents of the crypto tests, as one thread
that was marked read by the earlier tests in test/emacs is no longer
marked read.

This moves tests for:

- 09d19ac "test: emacs: toggle eliding of non-matching messages in
   `notmuch-show'", which should have actually read: "test: emacs:
   toggle processing of cryptographic MIME parts in `notmuch-show'".
   See commit 19ec74c5.

- 5ea1dbe "test: emacs: toggle eliding of non-matching messages in
  `notmuch-show'"

- 345faab "test: emacs: toggle thread content indentation in
  `notmuch-show'"

Signed-off-by: Ethan Glasser-Camp 
---
I screwed up with git commit --amend or something on the last patch,
so David Bremner suggested that I take advantage of the situation to
write this patch, which does something useful as a side effect.

 test/emacs |   67 -
 test/emacs-show|   71 ++
 .../notmuch-show-elide-non-matching-messages-off   |   79 
 .../notmuch-show-elide-non-matching-messages-on|   75 +++
 .../notmuch-show-indent-thread-content-off |   79 
 .../notmuch-show-process-crypto-mime-parts-off |   31 
 .../notmuch-show-process-crypto-mime-parts-on  |   32 
 .../notmuch-show-elide-non-matching-messages-off   |   79 
 .../notmuch-show-elide-non-matching-messages-on|   75 ---
 .../notmuch-show-indent-thread-content-off |   79 
 .../notmuch-show-process-crypto-mime-parts-off |   31 
 .../notmuch-show-process-crypto-mime-parts-on  |   32 
 12 files changed, 367 insertions(+), 363 deletions(-)
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-off
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-on
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-indent-thread-content-off
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-off
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-on
 delete mode 100644 
test/emacs.expected-output/notmuch-show-elide-non-matching-messages-off
 delete mode 100644 
test/emacs.expected-output/notmuch-show-elide-non-matching-messages-on
 delete mode 100644 
test/emacs.expected-output/notmuch-show-indent-thread-content-off
 delete mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
 delete mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on

diff --git a/test/emacs b/test/emacs
index e9d8239..1f84b91 100755
--- a/test/emacs
+++ b/test/emacs
@@ -783,71 +783,4 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED


-test_begin_subtest "don't process cryptographic MIME parts"
-test_emacs '(let ((notmuch-crypto-process-mime nil))
-   (notmuch-show "id:20091117203301.GV3165 at dottiness.seas.harvard.edu")
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-off
-
-test_begin_subtest "process cryptographic MIME parts"
-test_emacs '(let ((notmuch-crypto-process-mime t))
-   (notmuch-show "id:20091117203301.GV3165 at dottiness.seas.harvard.edu")
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
-
-test_begin_subtest "process cryptographic MIME parts (w/ 
notmuch-show-toggle-process-crypto)"
-test_emacs '(let ((notmuch-crypto-process-mime nil))
-   (notmuch-show "id:20091117203301.GV3165 at dottiness.seas.harvard.edu")
-   (notmuch-show-toggle-process-crypto)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
-
-test_begin_subtest "notmuch-show: don't elide non-matching messages"
-test_emacs '(let ((notmuch-show-only-matching-messages nil))
-   (notmuch-search "from:lars at seas.harvard.edu and subject:\"Maildir 
storage\"")
-   (notmuch-test-wait)
-   (notmuch-search-show-thread)
-   (notmuch-test-wait)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-elide-non-matching-messages-off
-
-test_begin_subtest "notmuch-show: elide non-matching messages"
-test_emacs '(let ((notmuch-show-only-matching-messages t))
-   (notmuch-search "from:lars at seas.harvard.edu and subject:\"Maildir 
storage\"")
-   (notmuch-test-wait)
-   (notmuch-search-show-thread)
-   (notmuch-test-wait)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-elide-non-matching-messages-on
-
-test_begin_subtest "notmuch-show: elide non-matching messages (w/ 
notmuch-show-t

[PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'

[Ethan Glasser-Camp]

Ethan Glasser-Camp  writes:

> This patch, and its predecessors, all look great to me.

But a note: many of the first lines in your commit messages ("{show,
hide} message headers") contain tabs. I hate tabs. Is this intentional?
I have noticed it on other patches you've sent (such as
id:"1330122640-18895-6-git-send-email-pieter at praet.org" and
id:"1330122640-18895-7-git-send-email-pieter at praet.org").

Ethan

[PATCH 1/3] test: emacs: toggle eliding of non-matching messages in `notmuch-show'

[Ethan Glasser-Camp]

From: Pieter Praet <pie...@praet.org>

See commits 44a544ed, 866ce8b1, 668b66ec.

Signed-off-by: Ethan Glasser-Camp 
---
I am embarrassed to admit I didn't try to apply these patches before I
removed the needs-review tag. This one didn't apply. Here's the
trivial fix. The tests are still placed at the bottom of test/emacs
and not in test/emacs-show. The other two patches should apply without
change.

 test/emacs |   20 
 .../notmuch-show-process-crypto-mime-parts-off |   31 +++
 .../notmuch-show-process-crypto-mime-parts-on  |   32 
 3 files changed, 83 insertions(+)
 create mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
 create mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on

diff --git a/test/emacs b/test/emacs
index 1f84b91..58ea59a 100755
--- a/test/emacs
+++ b/test/emacs
@@ -783,4 +783,24 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED


+test_begin_subtest "don't process cryptographic MIME parts"
+test_emacs '(let ((notmuch-crypto-process-mime nil))
+   (notmuch-show "id:20091117203301.GV3165 at dottiness.seas.harvard.edu")
+   (test-visible-output))'
+test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-off
+
+test_begin_subtest "process cryptographic MIME parts"
+test_emacs '(let ((notmuch-crypto-process-mime t))
+   (notmuch-show "id:20091117203301.GV3165 at dottiness.seas.harvard.edu")
+   (test-visible-output))'
+test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
+
+test_begin_subtest "process cryptographic MIME parts (w/ 
notmuch-show-toggle-process-crypto)"
+test_emacs '(let ((notmuch-crypto-process-mime nil))
+   (notmuch-show "id:20091117203301.GV3165 at dottiness.seas.harvard.edu")
+   (notmuch-show-toggle-process-crypto)
+   (test-visible-output))'
+test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
+
+
 test_done
diff --git 
a/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off 
b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
new file mode 100644
index 000..076083a
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
@@ -0,0 +1,31 @@
+Lars Kellogg-Stedman  (2009-11-17) (inbox signed)
+Subject: [notmuch] Working with Maildir storage?
+ Mikhail Gusarov  (2009-11-17) (inbox signed 
unread)
+  Lars Kellogg-Stedman  (2009-11-17) (inbox signed)
+  Subject: Re: [notmuch] Working with Maildir storage?
+  To: Mikhail Gusarov 
+  Cc: notmuch at notmuchmail.org
+  Date: Tue, 17 Nov 2009 15:33:01 -0500
+
+  [ multipart/mixed ]
+  [ multipart/signed ]
+  [ text/plain ]
+  > See the patch just posted here.
+
+  Is the list archived anywhere?  The obvious archives
+  (http://notmuchmail.org/pipermail/notmuch/) aren't available, and I
+  think I subscribed too late to get the patch (I only just saw the
+  discussion about it).
+
+  It doesn't look like the patch is in git yet.
+
+  -- Lars
+
+  [ 4-line signature. Click/Enter to show. ]
+  [ application/pgp-signature ]
+  [ text/plain ]
+  [ 4-line signature. Click/Enter to show. ]
+   Mikhail Gusarov  (2009-11-17) (inbox unread)
+   Keith Packard  (2009-11-17) (inbox unread)
+Lars Kellogg-Stedman  (2009-11-18) (inbox signed 
unread)
+ Carl Worth  (2009-11-18) (inbox unread)
diff --git 
a/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on 
b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on
new file mode 100644
index 000..588f38f
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on
@@ -0,0 +1,32 @@
+Lars Kellogg-Stedman  (2009-11-17) (inbox signed)
+Subject: [notmuch] Working with Maildir storage?
+ Mikhail Gusarov  (2009-11-17) (inbox signed 
unread)
+  Lars Kellogg-Stedman  (2009-11-17) (inbox signed)
+  Subject: Re: [notmuch] Working with Maildir storage?
+  To: Mikhail Gusarov 
+  Cc: notmuch at notmuchmail.org
+  Date: Tue, 17 Nov 2009 15:33:01 -0500
+
+  [ multipart/mixed ]
+  [ multipart/signed ]
+  [ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
+  [ text/plain ]
+  > See the patch just posted here.
+
+  Is the list archived anywhere?  The obvious archives
+  (http://notmuchmail.org/pipermail/notmuch/) aren't available, and I
+  think I subscribed too late to get the patch (I only just saw the
+  discussion about it).
+
+  It doesn't look like the patch is in git yet.
+
+  -- Lars
+
+  [ 4-line signature. Click/Enter to show. ]
+  [ application/pgp-signature ]
+  [ text/plain ]
+  [ 4-line signature. Click/Enter to show. ]
+   Mikhail Gusarov  (2009-11-17) (inbox unread)
+   Keith Packard  (2009-11-17) (inbox unread)
+Lars Kellogg-Stedman  (2009-11-18) (inbox signed 
unread)
+ Carl Worth  (2009-11-18) (inbox unread)
-- 
1.7.9.5

[PATCH 1/2] Add notmuch_database_flush method

[Ethan Glasser-Camp]

Adrien Bustany  writes:

> This method explicitly flushes the pending modifications to disk. It is
> useful if your program has various threads, each with a read only DB and
> one writer thread with a read/write DB. In that case, you most likely
> want the writer to sync the changes to disk so that the readers can see
> them, without having to close and reopen the database completely.

These patches are pretty straightforward. But to conform to notmuch style..

> +notmuch_status_t
> +notmuch_database_flush(notmuch_database_t *notmuch)
> +{
> + notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;

Indent is 4 spaces. (You have tabs here, which are 8 spaces, according
to devel/STYLE.)

> + try {
> + if (notmuch->xapian_db != NULL &&

if should be more indented than try. (So when you pull try back to 4
spaces, leave if at 8 spaces.)

> + notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
> + (static_cast  
> (notmuch->xapian_db))->flush ();

This line is 90 characters, and will remain at 86 once you indent using
the in-house style. I'm not sure if it's worth reformatting.
notmuch_database_close calls flush() using exactly the same 86-character
line. I'd say "don't make it worse", but personally I think breaking
this line might be worse.

Ethan

Re: [PATCH 1/2] Add notmuch_database_flush method

[Ethan Glasser-Camp]

Adrien Bustany adr...@bustany.org writes:

 This method explicitly flushes the pending modifications to disk. It is
 useful if your program has various threads, each with a read only DB and
 one writer thread with a read/write DB. In that case, you most likely
 want the writer to sync the changes to disk so that the readers can see
 them, without having to close and reopen the database completely.

These patches are pretty straightforward. But to conform to notmuch style..

 +notmuch_status_t
 +notmuch_database_flush(notmuch_database_t *notmuch)
 +{
 + notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;

Indent is 4 spaces. (You have tabs here, which are 8 spaces, according
to devel/STYLE.)

 + try {
 + if (notmuch-xapian_db != NULL 

if should be more indented than try. (So when you pull try back to 4
spaces, leave if at 8 spaces.)

 + notmuch-mode == NOTMUCH_DATABASE_MODE_READ_WRITE)
 + (static_cast Xapian::WritableDatabase * 
 (notmuch-xapian_db))-flush ();

This line is 90 characters, and will remain at 86 once you indent using
the in-house style. I'm not sure if it's worth reformatting.
notmuch_database_close calls flush() using exactly the same 86-character
line. I'd say don't make it worse, but personally I think breaking
this line might be worse.

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

[PATCH 1/3] test: emacs: toggle eliding of non-matching messages in `notmuch-show'

[Ethan Glasser-Camp]

From: Pieter Praet pie...@praet.org

See commits 44a544ed, 866ce8b1, 668b66ec.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
I am embarrassed to admit I didn't try to apply these patches before I
removed the needs-review tag. This one didn't apply. Here's the
trivial fix. The tests are still placed at the bottom of test/emacs
and not in test/emacs-show. The other two patches should apply without
change.

 test/emacs |   20 
 .../notmuch-show-process-crypto-mime-parts-off |   31 +++
 .../notmuch-show-process-crypto-mime-parts-on  |   32 
 3 files changed, 83 insertions(+)
 create mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
 create mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on

diff --git a/test/emacs b/test/emacs
index 1f84b91..58ea59a 100755
--- a/test/emacs
+++ b/test/emacs
@@ -783,4 +783,24 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 
+test_begin_subtest don't process cryptographic MIME parts
+test_emacs '(let ((notmuch-crypto-process-mime nil))
+   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
+   (test-visible-output))'
+test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-off
+
+test_begin_subtest process cryptographic MIME parts
+test_emacs '(let ((notmuch-crypto-process-mime t))
+   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
+   (test-visible-output))'
+test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
+
+test_begin_subtest process cryptographic MIME parts (w/ 
notmuch-show-toggle-process-crypto)
+test_emacs '(let ((notmuch-crypto-process-mime nil))
+   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
+   (notmuch-show-toggle-process-crypto)
+   (test-visible-output))'
+test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
+
+
 test_done
diff --git 
a/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off 
b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
new file mode 100644
index 000..076083a
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
@@ -0,0 +1,31 @@
+Lars Kellogg-Stedman l...@seas.harvard.edu (2009-11-17) (inbox signed)
+Subject: [notmuch] Working with Maildir storage?
+ Mikhail Gusarov dotted...@dottedmag.net (2009-11-17) (inbox signed unread)
+  Lars Kellogg-Stedman l...@seas.harvard.edu (2009-11-17) (inbox signed)
+  Subject: Re: [notmuch] Working with Maildir storage?
+  To: Mikhail Gusarov dotted...@dottedmag.net
+  Cc: notmuch@notmuchmail.org
+  Date: Tue, 17 Nov 2009 15:33:01 -0500
+
+  [ multipart/mixed ]
+  [ multipart/signed ]
+  [ text/plain ]
+   See the patch just posted here.
+
+  Is the list archived anywhere?  The obvious archives
+  (http://notmuchmail.org/pipermail/notmuch/) aren't available, and I
+  think I subscribed too late to get the patch (I only just saw the
+  discussion about it).
+
+  It doesn't look like the patch is in git yet.
+
+  -- Lars
+
+  [ 4-line signature. Click/Enter to show. ]
+  [ application/pgp-signature ]
+  [ text/plain ]
+  [ 4-line signature. Click/Enter to show. ]
+   Mikhail Gusarov dotted...@dottedmag.net (2009-11-17) (inbox unread)
+   Keith Packard kei...@keithp.com (2009-11-17) (inbox unread)
+Lars Kellogg-Stedman l...@seas.harvard.edu (2009-11-18) (inbox signed 
unread)
+ Carl Worth cwo...@cworth.org (2009-11-18) (inbox unread)
diff --git 
a/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on 
b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on
new file mode 100644
index 000..588f38f
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on
@@ -0,0 +1,32 @@
+Lars Kellogg-Stedman l...@seas.harvard.edu (2009-11-17) (inbox signed)
+Subject: [notmuch] Working with Maildir storage?
+ Mikhail Gusarov dotted...@dottedmag.net (2009-11-17) (inbox signed unread)
+  Lars Kellogg-Stedman l...@seas.harvard.edu (2009-11-17) (inbox signed)
+  Subject: Re: [notmuch] Working with Maildir storage?
+  To: Mikhail Gusarov dotted...@dottedmag.net
+  Cc: notmuch@notmuchmail.org
+  Date: Tue, 17 Nov 2009 15:33:01 -0500
+
+  [ multipart/mixed ]
+  [ multipart/signed ]
+  [ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
+  [ text/plain ]
+   See the patch just posted here.
+
+  Is the list archived anywhere?  The obvious archives
+  (http://notmuchmail.org/pipermail/notmuch/) aren't available, and I
+  think I subscribed too late to get the patch (I only just saw the
+  discussion about it).
+
+  It doesn't look like the patch is in git yet.
+
+  -- Lars
+
+  [ 4-line signature. Click/Enter to show. ]
+  [ application/pgp-signature ]
+  [ text/plain ]
+  [ 4-line signature. Click/Enter to show. ]
+   Mikhail Gusarov dotted

Re: [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'

[Ethan Glasser-Camp]

Ethan Glasser-Camp ethan.glasser.c...@gmail.com writes:

 This patch, and its predecessors, all look great to me.

But a note: many of the first lines in your commit messages ({show,
hide} message headers) contain tabs. I hate tabs. Is this intentional?
I have noticed it on other patches you've sent (such as
id:1330122640-18895-6-git-send-email-pie...@praet.org and
id:1330122640-18895-7-git-send-email-pie...@praet.org).

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

[PATCH] test: Move tests from emacs to emacs-show

[Ethan Glasser-Camp]

This requires changing the contents of the crypto tests, as one thread
that was marked read by the earlier tests in test/emacs is no longer
marked read.

This moves tests for:

- 09d19ac test: emacs: toggle eliding of non-matching messages in
   `notmuch-show', which should have actually read: test: emacs:
   toggle processing of cryptographic MIME parts in `notmuch-show'.
   See commit 19ec74c5.

- 5ea1dbe test: emacs: toggle eliding of non-matching messages in
  `notmuch-show'

- 345faab test: emacs: toggle thread content indentation in
  `notmuch-show'

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
I screwed up with git commit --amend or something on the last patch,
so David Bremner suggested that I take advantage of the situation to
write this patch, which does something useful as a side effect.

 test/emacs |   67 -
 test/emacs-show|   71 ++
 .../notmuch-show-elide-non-matching-messages-off   |   79 
 .../notmuch-show-elide-non-matching-messages-on|   75 +++
 .../notmuch-show-indent-thread-content-off |   79 
 .../notmuch-show-process-crypto-mime-parts-off |   31 
 .../notmuch-show-process-crypto-mime-parts-on  |   32 
 .../notmuch-show-elide-non-matching-messages-off   |   79 
 .../notmuch-show-elide-non-matching-messages-on|   75 ---
 .../notmuch-show-indent-thread-content-off |   79 
 .../notmuch-show-process-crypto-mime-parts-off |   31 
 .../notmuch-show-process-crypto-mime-parts-on  |   32 
 12 files changed, 367 insertions(+), 363 deletions(-)
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-off
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-on
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-indent-thread-content-off
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-off
 create mode 100644 
test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-on
 delete mode 100644 
test/emacs.expected-output/notmuch-show-elide-non-matching-messages-off
 delete mode 100644 
test/emacs.expected-output/notmuch-show-elide-non-matching-messages-on
 delete mode 100644 
test/emacs.expected-output/notmuch-show-indent-thread-content-off
 delete mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-off
 delete mode 100644 
test/emacs.expected-output/notmuch-show-process-crypto-mime-parts-on

diff --git a/test/emacs b/test/emacs
index e9d8239..1f84b91 100755
--- a/test/emacs
+++ b/test/emacs
@@ -783,71 +783,4 @@ EOF
 test_expect_equal_file OUTPUT EXPECTED
 
 
-test_begin_subtest don't process cryptographic MIME parts
-test_emacs '(let ((notmuch-crypto-process-mime nil))
-   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-off
-
-test_begin_subtest process cryptographic MIME parts
-test_emacs '(let ((notmuch-crypto-process-mime t))
-   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
-
-test_begin_subtest process cryptographic MIME parts (w/ 
notmuch-show-toggle-process-crypto)
-test_emacs '(let ((notmuch-crypto-process-mime nil))
-   (notmuch-show id:20091117203301.gv3...@dottiness.seas.harvard.edu)
-   (notmuch-show-toggle-process-crypto)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-process-crypto-mime-parts-on
-
-test_begin_subtest notmuch-show: don't elide non-matching messages
-test_emacs '(let ((notmuch-show-only-matching-messages nil))
-   (notmuch-search from:l...@seas.harvard.edu and subject:\Maildir 
storage\)
-   (notmuch-test-wait)
-   (notmuch-search-show-thread)
-   (notmuch-test-wait)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-elide-non-matching-messages-off
-
-test_begin_subtest notmuch-show: elide non-matching messages
-test_emacs '(let ((notmuch-show-only-matching-messages t))
-   (notmuch-search from:l...@seas.harvard.edu and subject:\Maildir 
storage\)
-   (notmuch-test-wait)
-   (notmuch-search-show-thread)
-   (notmuch-test-wait)
-   (test-visible-output))'
-test_expect_equal_file OUTPUT 
$EXPECTED/notmuch-show-elide-non-matching-messages-on
-
-test_begin_subtest notmuch-show: elide non-matching messages (w/ 
notmuch-show-toggle-elide-non-matching)
-test_emacs '(let ((notmuch-show-only-matching-messages nil))
-   (notmuch-search from:l...@seas.harvard.edu and subject:\Maildir 
storage\)
-   (notmuch-test

[PATCH v2 5/6] emacs: simplify `notmuch-show-get-messages-ids{, -search}'

[Ethan Glasser-Camp]

Mark Walters  writes:

> I like the use of separator rather than hard-wiring " or ". My personal
> preference would be to make that change but keep the two functions
> separate (my "C"ness makes me like functions that have clear return
> types!) But I am happy with the change too.

I agree with this comment. Especially in dynamic languages, it's a good
idea for each function to have only one return type.

Also, it seems that the commit message has a TAB character in it 
(",-search").

Ethan

[PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'

[Ethan Glasser-Camp]

Mark Walters  writes:

> The original function feels a little fragile to me as to what happens if
> predicate or function move point. Eg what happens if function collapses
> the message: where does point go, and so where does
> notmuch-show-goto-message-next go. Is this just my naivete as a lisp
> beginner? Is there someway of writing it so the user doesn't need to
> worry about such things?

Although collapsing the message doesn't seem to move point, it would
probably be a good idea to wrap the calls to predicate and function in
save-excursion, as a guard against subtle and hard-to-spot bugs with
operations not being applied to all the right messages..

Ethan

[PATCH v2 4/6] emacs: add optional predicate arg to `notmuch-show-mapc'

[Ethan Glasser-Camp]

Pieter Praet  writes:

> * emacs/notmuch-show.el (notmuch-show-mapc):
>
>   If provided with optional argument PREDICATE, only call
>   FUNCTION if calling PREDICATE returns non-nil.
>
>   Also correct original docstring: 's/thread/buffer/'.
> ---

This patch was marked stale, but isn't.

> -(defun notmuch-show-mapc (function)
> -  "Iterate through all messages in the current thread with
> +(defun notmuch-show-mapc (function  predicate)
> +  "Iterate through all messages in the current buffer with
>  `notmuch-show-goto-message-next' and call FUNCTION for side
> -effects."
> +effects.
> +
> +If provided with optional argument PREDICATE, only call
> +FUNCTION if calling PREDICATE returns non-nil."
>(save-excursion
>  (goto-char (point-min))
> -(loop do (funcall function)
> +(loop do (if predicate
> +  (if (funcall predicate)
> +  (funcall function))
> +(funcall function))

I don't like the way this if-structure looks, since I have to squint to
see whether the "else" clause matches the inner or the outer "if". Maybe
change the inner "if" to a "when" or an "and"?

Ethan