[PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'
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.
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
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
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.
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
Re: [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'
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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[ANN] notmuch-labeler: Improves notmuch way of displaying labels
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.
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
Re: [PATCH v3 2/2] test: Update test to match previous patch.
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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] test: handle filenames that have directories in them
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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] test: add check for filename argument for test_expect_equal_file
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
[PATCH] test: another test wrt ignoring user-specified files and directories
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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
More ideas about logging.
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: [PATCH v2] emacs: Add more processing of displayed headers.
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@wsheee.2x.cz". Ethan ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/2] test: add check for filename argument for test_expect_equal_file
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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [ANN] notmuch-labeler: Improves notmuch way of displaying labels
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 mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [notmuch] [PATCH] Calls to notmuch get queued and executed asynchronously.
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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: More ideas about logging.
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 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch