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

[Ethan Glasser-Camp]

Pieter Praet  writes:

> * emacs/notmuch-show.el
>
>   (notmuch-show-toggle-headers):
> Rename to `notmuch-show-toggle-visibility-headers'.

This patch, and its predecessors, all look great to me. The following
patches were already marked "stale" (and indeed they don't apply). But
it looks like David Edmonson gave his +1 on the idea, so maybe you
should rebase them.

Side note: how do you write these tests? How do you generate what the
output should look like? Copy/paste from a "real" emacs session? You
have generated an awful lot of tests :)

Ethan

[PATCH v3 2/2] test: Update test to match previous patch.

[Ethan Glasser-Camp]

David Edmondson  writes:

> Indentation now uses tabs where possible.

Hi! Just working through the patch queue. This patch is tagged
notmuch::moreinfo, although it seems like the rest of the series may
have been tagged notmuch::stale or notmuch::pushed. It's a little hard
to figure out, because the thread structure is really messed up. Is this
patch still viable? It now *breaks* the emacs test on my machine.

Ethan

[PATCH] test: handle filenames that have directories in them

[Ethan Glasser-Camp]

Since $TEST_DIRECTORY is an absolute path, any filenames generated
with it will be complete paths. Only use the basename to generate
suffixes for filenames.

Signed-off-by: Ethan Glasser-Camp 
---
Discovered this while reviewing the patch queue. test/emacs generates
filenames using $TEST_DIRECTORY, which is generated using pwd(1). Test
failures then cause failures in the test harness.

 test/test-lib.sh |8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 7448b45..8de5e32 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -498,16 +498,18 @@ test_expect_equal_file ()
error "bug in the test script: not 2 or 3 parameters to 
test_expect_equal"

file1="$1"
+   basename1=`basename "$file1"`
file2="$2"
+   basename2=`basename "$file2"`
if ! test_skip "$test_subtest_name"
then
if diff -q "$file1" "$file2" >/dev/null ; then
test_ok_ "$test_subtest_name"
else
testname=$this_test.$test_count
-   cp "$file1" "$testname.$file1"
-   cp "$file2" "$testname.$file2"
-   test_failure_ "$test_subtest_name" "$(diff -u 
"$testname.$file1" "$testname.$file2")"
+   cp "$file1" "$testname.$basename1"
+   cp "$file2" "$testname.$basename2"
+   test_failure_ "$test_subtest_name" "$(diff -u 
"$testname.$basename1" "$testname.$basename2")"
fi
 fi
 }
-- 
1.7.9.5

[PATCH] test: another test wrt ignoring user-specified files and directories

[Ethan Glasser-Camp]

From: Pieter Praet 

Demonstrates that *every* file/directory which matches one of the values
in 'new.ignore' will be ignored, independent of its depth/location in
the mail store.

Signed-off-by: Ethan Glasser-Camp 
---
This is the trivial modification of Pieter's patch that makes it work
no matter where your test directory is.

 test/new |   19 +++
 1 file changed, 19 insertions(+)

diff --git a/test/new b/test/new
index cab7c01..cc2af72 100755
--- a/test/new
+++ b/test/new
@@ -183,5 +183,24 @@ touch "${MAIL_DIR}"/.git # change .git's mtime for notmuch 
new to rescan.
 output=$(NOTMUCH_NEW 2>&1)
 test_expect_equal "$output" "Added 1 new message to the database."

+test_begin_subtest "Ignore files and directories specified in new.ignore 
(multiple occurrences)"
+notmuch config set new.ignore .git ignored_file .ignored_hidden_file
+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)
+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
+No new mail."
+

 test_done
-- 
1.7.9.5

[PATCH v2] emacs: Add more processing of displayed headers.

[Ethan Glasser-Camp]

Hi! Just going through the patch queue.

This is definitely a nice effect, but I'm not sure of the approach. It
doesn't indent the message's tags, and it doesn't work when you resize the
window. (You can get some very ugly wrapping if you put your mind to
it.)

Is there no better way to do this using visual-line-mode? I know that
the rest of notmuch uses hard filling, but that's no reason to make a
bad situation worse. It looks like you can put wrap-prefix text
properties all over, as done in the adaptive-wrap package:

http://elpa.gnu.org/packages/adaptive-wrap-0.1.el

Slightly more nit-picky comments below.

David Edmondson  writes:

> -(defvar notmuch-show-markup-headers-hook '(notmuch-show-colour-headers)
> +(defcustom notmuch-show-markup-headers-hook '(notmuch-show-colour-headers
> +   notmuch-show-fill-headers
> +   notmuch-show-indent-continuations)
>"A list of functions called to decorate the headers listed in
> -`notmuch-message-headers'.")
> +`notmuch-message-headers'."
> +  :type 'hook
> +  :options '(notmuch-show-colour-headers
> +  notmuch-show-fill-headers
> +  notmuch-show-indent-continuations)
> +  :group 'notmuch-show)

This hook is not normal because it takes an argument, and so should have
a name ending in -hooks or -functions. Also, since it's a defcustom now,
it should probably have a better explanation of how it works, that it
takes an argument, and what that argument means.

It seems extremely dicey to me that you can put
notmuch-show-indent-continuations in this list before, or without,
notmuch-show-fill-headers.

> +(defun notmuch-show-fill-headers (depth)
> +  "Wrap the text of the current headers."
> +
> +  ;; '-5' to allow for the indentation code.
> +  (let ((fill-column (- (window-width) depth 5)))

It took me a little while to figure out what this meant. How about,
"underfill by 5 so that inserting indentation doesn't cause more
wrapping"? Is it possible to be smart enough to let

> +(defun notmuch-show-indent-continuations (depth)
> +  "Indent any continuation lines."
> +  (goto-char (point-min))
> +  (while (not (eobp))
> +(if (not (looking-at "^[A-Za-z][-A-Za-z0-9]*:"))
> + ;; Four spaces tends to work well with 'To' and 'Cc' headers.
> + (insert ""))
> +(forward-line)))

I'm not crazy about this but I'm not sure I can say why exactly. Why
can't we just run indent-rigidly over the whole thing?

The comment isn't terribly useful. Only those headers? "Tends to work
well"?

>  ;; Override `notmuch-message-headers' to force `From' to be
>  ;; displayed.
>  (let ((notmuch-message-headers '("From" "Subject" "To" "Cc" "Date")))
> -  (notmuch-show-insert-headers (plist-get message :headers)))
> +  (notmuch-show-insert-headers (plist-get message :headers) 0))

This took me a long while to figure out, especially because it looks
like the depth is being passed normally to notmuch-show-insert-bodypart,
just below. A comment like "depth = 0 because we reindent below" would
have been really helpful. A good test message is
id:"87ocabvp0y.fsf at wsheee.2x.cz".

Ethan

[ANN] notmuch-labeler: Improves notmuch way of displaying labels

[David Bremner]

Damien Cassou  writes:

>
> If there is interest, I can help integrate into notmuch itself.
>

Hi Damien;

It seeems like a nice UI enchancement, and people would not have to use
it if they didn't like it, so in principle I guess we should work
towards integrating it into notmuch upstream.

I'm not an elisp expert, but I did find it strange to use use defadvice
on your own code. Maybe I'm just too conservative.

At some point we should probably get a patch series against notmuch
mainline; I'll let other people comment on whether yes, now is the time
(and thereby volunteering to review the patches ;).

[notmuch] [PATCH] Calls to notmuch get queued and executed asynchronously.

[David Bremner]

Ethan Glasser-Camp  writes:

> It seems like this patch series is useful for some people, but there are
> some design issues yet to be worked out.
>
> I propose that this thread be tagged notmuch::stale.

Go for it, now that you have awesome power to tag things in nmbug.

A periodic reminder, anyone who wants werte access to the nmbug tag
database should send me a ssh public key.

David

[PATCH 1/2] test: add check for filename argument for test_expect_equal_file

[Ethan Glasser-Camp]

Dmitry Kurochkin  writes:

> Actually, we can do both: check file name for consistent diff order
> (from expected to actual) and use file names that the caller provides.

Hi! Reviewing the patch queue a little bit here. It seems like this
patch ended up getting dropped because the other approach (using the
filenames that the caller provided) won out and was even pushed. I'm
therefore marking this series notmuch::obsolete (and also
notmuch::stale since it doesn't apply cleanly).

Ethan

More ideas about logging.

[Ethan Glasser-Camp]

Austin Clements  writes:

> The trouble with this approach is that the OS doesn't have to flush
> logfile to the disk platters in any particular order relative to the
> updates to Xapian.  So, after someone trips over your plug, you could
> come back with Xapian saying you have 500 log entries when your
> logfile comes back with only 20.  The only way I know of to fix this
> is to fsync after the logfile write, which would obviously have
> performance issues.  But maybe there are cleverer ways?

Sorry to jump in almost a year after the fact, but..

How bad do you think those performance issues are going to be? I don't
see them as prohibitive, even in the case where you write a log entry
for every message being tagged. Xapian's doing an fsync each time we
commit, isn't it? (Or is there some cute trick where it rename()s the
database?)

If, instead of truncating, you replay logged operations, I think you can
get away with just writing log entries for user-level operations (like
"notmuch tag +mytag to:somequery") which could touch a lot of
messages. This would then only require one fsync on the log file before
doing a lot of tag updates.

Ethan

Re: More ideas about logging.

[Ethan Glasser-Camp]

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

 The trouble with this approach is that the OS doesn't have to flush
 logfile to the disk platters in any particular order relative to the
 updates to Xapian.  So, after someone trips over your plug, you could
 come back with Xapian saying you have 500 log entries when your
 logfile comes back with only 20.  The only way I know of to fix this
 is to fsync after the logfile write, which would obviously have
 performance issues.  But maybe there are cleverer ways?

Sorry to jump in almost a year after the fact, but..

How bad do you think those performance issues are going to be? I don't
see them as prohibitive, even in the case where you write a log entry
for every message being tagged. Xapian's doing an fsync each time we
commit, isn't it? (Or is there some cute trick where it rename()s the
database?)

If, instead of truncating, you replay logged operations, I think you can
get away with just writing log entries for user-level operations (like
notmuch tag +mytag to:somequery) which could touch a lot of
messages. This would then only require one fsync on the log file before
doing a lot of tag updates.

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

Re: [notmuch] [PATCH] Calls to notmuch get queued and executed asynchronously.

[David Bremner]

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

 It seems like this patch series is useful for some people, but there are
 some design issues yet to be worked out.

 I propose that this thread be tagged notmuch::stale.

Go for it, now that you have awesome power to tag things in nmbug.

A periodic reminder, anyone who wants werte access to the nmbug tag
database should send me a ssh public key.

David

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

Re: [ANN] notmuch-labeler: Improves notmuch way of displaying labels

[David Bremner]

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


 If there is interest, I can help integrate into notmuch itself.


Hi Damien;

It seeems like a nice UI enchancement, and people would not have to use
it if they didn't like it, so in principle I guess we should work
towards integrating it into notmuch upstream.

I'm not an elisp expert, but I did find it strange to use use defadvice
on your own code. Maybe I'm just too conservative.

At some point we should probably get a patch series against notmuch
mainline; I'll let other people comment on whether yes, now is the time
(and thereby volunteering to review the patches ;).



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

Re: [PATCH 1/2] test: add check for expected filename argument for test_expect_equal_file

[Ethan Glasser-Camp]

Dmitry Kurochkin dmitry.kuroch...@gmail.com writes:

 Actually, we can do both: check file name for consistent diff order
 (from expected to actual) and use file names that the caller provides.

Hi! Reviewing the patch queue a little bit here. It seems like this
patch ended up getting dropped because the other approach (using the
filenames that the caller provided) won out and was even pushed. I'm
therefore marking this series notmuch::obsolete (and also
notmuch::stale since it doesn't apply cleanly).

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

Re: [PATCH v2] emacs: Add more processing of displayed headers.

[Ethan Glasser-Camp]

Hi! Just going through the patch queue.

This is definitely a nice effect, but I'm not sure of the approach. It
doesn't indent the message's tags, and it doesn't work when you resize the
window. (You can get some very ugly wrapping if you put your mind to
it.)

Is there no better way to do this using visual-line-mode? I know that
the rest of notmuch uses hard filling, but that's no reason to make a
bad situation worse. It looks like you can put wrap-prefix text
properties all over, as done in the adaptive-wrap package:

http://elpa.gnu.org/packages/adaptive-wrap-0.1.el

Slightly more nit-picky comments below.

David Edmondson d...@dme.org writes:

 -(defvar notmuch-show-markup-headers-hook '(notmuch-show-colour-headers)
 +(defcustom notmuch-show-markup-headers-hook '(notmuch-show-colour-headers
 +   notmuch-show-fill-headers
 +   notmuch-show-indent-continuations)
A list of functions called to decorate the headers listed in
 -`notmuch-message-headers'.)
 +`notmuch-message-headers'.
 +  :type 'hook
 +  :options '(notmuch-show-colour-headers
 +  notmuch-show-fill-headers
 +  notmuch-show-indent-continuations)
 +  :group 'notmuch-show)

This hook is not normal because it takes an argument, and so should have
a name ending in -hooks or -functions. Also, since it's a defcustom now,
it should probably have a better explanation of how it works, that it
takes an argument, and what that argument means.

It seems extremely dicey to me that you can put
notmuch-show-indent-continuations in this list before, or without,
notmuch-show-fill-headers.

 +(defun notmuch-show-fill-headers (depth)
 +  Wrap the text of the current headers.
 +
 +  ;; '-5' to allow for the indentation code.
 +  (let ((fill-column (- (window-width) depth 5)))

It took me a little while to figure out what this meant. How about,
underfill by 5 so that inserting indentation doesn't cause more
wrapping? Is it possible to be smart enough to let

 +(defun notmuch-show-indent-continuations (depth)
 +  Indent any continuation lines.
 +  (goto-char (point-min))
 +  (while (not (eobp))
 +(if (not (looking-at ^[A-Za-z][-A-Za-z0-9]*:))
 + ;; Four spaces tends to work well with 'To' and 'Cc' headers.
 + (insert ))
 +(forward-line)))

I'm not crazy about this but I'm not sure I can say why exactly. Why
can't we just run indent-rigidly over the whole thing?

The comment isn't terribly useful. Only those headers? Tends to work
well?

  ;; Override `notmuch-message-headers' to force `From' to be
  ;; displayed.
  (let ((notmuch-message-headers '(From Subject To Cc Date)))
 -  (notmuch-show-insert-headers (plist-get message :headers)))
 +  (notmuch-show-insert-headers (plist-get message :headers) 0))

This took me a long while to figure out, especially because it looks
like the depth is being passed normally to notmuch-show-insert-bodypart,
just below. A comment like depth = 0 because we reindent below would
have been really helpful. A good test message is
id:87ocabvp0y@wsheee.2x.cz.

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

[PATCH] test: another test wrt ignoring user-specified files and directories

[Ethan Glasser-Camp]

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

Demonstrates that *every* file/directory which matches one of the values
in 'new.ignore' will be ignored, independent of its depth/location in
the mail store.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
This is the trivial modification of Pieter's patch that makes it work
no matter where your test directory is.

 test/new |   19 +++
 1 file changed, 19 insertions(+)

diff --git a/test/new b/test/new
index cab7c01..cc2af72 100755
--- a/test/new
+++ b/test/new
@@ -183,5 +183,24 @@ touch ${MAIL_DIR}/.git # change .git's mtime for notmuch 
new to rescan.
 output=$(NOTMUCH_NEW 21)
 test_expect_equal $output Added 1 new message to the database.
 
+test_begin_subtest Ignore files and directories specified in new.ignore 
(multiple occurrences)
+notmuch config set new.ignore .git ignored_file .ignored_hidden_file
+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)
+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
+No new mail.
+
 
 test_done
-- 
1.7.9.5

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

[PATCH] test: handle filenames that have directories in them

[Ethan Glasser-Camp]

Since $TEST_DIRECTORY is an absolute path, any filenames generated
with it will be complete paths. Only use the basename to generate
suffixes for filenames.

Signed-off-by: Ethan Glasser-Camp et...@betacantrips.com
---
Discovered this while reviewing the patch queue. test/emacs generates
filenames using $TEST_DIRECTORY, which is generated using pwd(1). Test
failures then cause failures in the test harness.

 test/test-lib.sh |8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 7448b45..8de5e32 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -498,16 +498,18 @@ test_expect_equal_file ()
error bug in the test script: not 2 or 3 parameters to 
test_expect_equal
 
file1=$1
+   basename1=`basename $file1`
file2=$2
+   basename2=`basename $file2`
if ! test_skip $test_subtest_name
then
if diff -q $file1 $file2 /dev/null ; then
test_ok_ $test_subtest_name
else
testname=$this_test.$test_count
-   cp $file1 $testname.$file1
-   cp $file2 $testname.$file2
-   test_failure_ $test_subtest_name $(diff -u 
$testname.$file1 $testname.$file2)
+   cp $file1 $testname.$basename1
+   cp $file2 $testname.$basename2
+   test_failure_ $test_subtest_name $(diff -u 
$testname.$basename1 $testname.$basename2)
fi
 fi
 }
-- 
1.7.9.5

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

Re: [PATCH v3 2/2] test: Update test to match previous patch.

[Ethan Glasser-Camp]

David Edmondson d...@dme.org writes:

 Indentation now uses tabs where possible.

Hi! Just working through the patch queue. This patch is tagged
notmuch::moreinfo, although it seems like the rest of the series may
have been tagged notmuch::stale or notmuch::pushed. It's a little hard
to figure out, because the thread structure is really messed up. Is this
patch still viable? It now *breaks* the emacs test on my machine.

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

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

[Ethan Glasser-Camp]

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

 * emacs/notmuch-show.el

   (notmuch-show-toggle-headers):
 Rename to `notmuch-show-toggle-visibility-headers'.

This patch, and its predecessors, all look great to me. The following
patches were already marked stale (and indeed they don't apply). But
it looks like David Edmonson gave his +1 on the idea, so maybe you
should rebase them.

Side note: how do you write these tests? How do you generate what the
output should look like? Copy/paste from a real emacs session? You
have generated an awful lot of tests :)

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