[PATCH 1/3] test: `notmuch-show-advance-and-archive' with invisible signature
On Wed, 29 Jun 2011 09:10:19 +0400, Dmitry Kurochkin wrote: > On Wed, 29 Jun 2011 05:48:50 +0400, Dmitry Kurochkin gmail.com> wrote: > > Add Emacs test to check that `notmuch-show-advance-and-archive' > > works for the last message in thread with invisible signature. > > --- > > > > This patch series fixes the bug reported by Sebastien in [1]. I > > was able to reproduce it and confirm that the second patch from > > this series fixes the problem. Unfortunately, I can not explain > > why it fixes it. The patch uses a cleaner approach for visible > > text search. But the old approach should work fine as well. > > Apparently, it does not work when `invisible' property is not a > > single symbol but a list (which was changed in > > 95ef8da29439f2e79115c36ab4d2a80aef1a1462). I suspect that it is > > an Emacs bug. I plan to look at it later. > > > > Turns out that `point-invisible-p' is a function from notmuch-lib.el, I > did not realize that before. It implements a custom visibility check > which is incomplete and does not work correctly when `invisible' > property is a list. That is why the previous code (which used > `point-invisible-p') had the bug. I sent another patch that removes > `point-invisible-p' function. > > > Another issue is that the test does not demonstrate the bug. > > Again, I do not really know why. It passes both before and after > > the fix. Although if I run the test commands by hand I hit the > > bug. I guess it has something to do with emacs daemon mode when > > the buffer is not visible. I hope someone with a better elisp > > knowledge can tell what is going on and how to make the test > > work. > > > > Now it is clear where the bug was. Remaining question is how to test > it. > I have posted a new version of this patch series [1]. It fixes the test to actually demonstrate the bug. Regards, Dmitry [1] id:"1309376558-26284-1-git-send-email-dmitry.kurochkin at gmail.com" > Regards, > Dmitry > > > I believe patches 2 and 3 can be pushed after review even without > > a working test. > > > > Regards, > > Dmitry > > > > [1] id:"8739j5rn2d.fsf at cern.ch" > > > > test/emacs | 12 > > 1 files changed, 12 insertions(+), 0 deletions(-) > > > > diff --git a/test/emacs b/test/emacs > > index e59de47..65a96a5 100755 > > --- a/test/emacs > > +++ b/test/emacs > > @@ -347,4 +347,16 @@ test_emacs '(notmuch-show > > "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae at mail. > > (test-visible-output)' > > test_expect_equal_file OUTPUT > > $EXPECTED/notmuch-show-thread-with-hidden-messages > > > > +test_begin_subtest 'notmuch-show-advance-and-archive with invisible > > signature' > > +message1='id:20091118010116.GC25380 at dottiness.seas.harvard.edu' > > +message2='id:1258491078-29658-1-git-send-email-dottedmag at dottedmag.net' > > +test_emacs "(notmuch-search \"$message1 or $message2\") > > + (notmuch-test-wait) > > + (notmuch-search-show-thread) > > + (notmuch-show-advance-and-archive) > > + (test-output)" > > +test_emacs "(notmuch-show \"$message2\") > > + (test-output \"EXPECTED\")" > > +test_expect_equal_file OUTPUT EXPECTED > > + > > test_done > > -- > > 1.7.5.4 > >
[PATCH 6/6] emacs: remove unused `point-invisible-p' function
`point-invisible-p' does not work correctly when `invisible' property is a list. There are standard `invisible-p' and related functions that should be used instead. --- emacs/notmuch-lib.el | 15 --- 1 files changed, 0 insertions(+), 15 deletions(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index f93c957..0f856bf 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -105,21 +105,6 @@ the user hasn't set this variable with the old or new value." ;; -;; XXX: This should be a generic function in emacs somewhere, not -;; here. -(defun point-invisible-p () - "Return whether the character at point is invisible. - -Here visibility is determined by `buffer-invisibility-spec' and -the invisible property of any overlays for point. It doesn't have -anything to do with whether point is currently being displayed -within the current window." - (let ((prop (get-char-property (point) 'invisible))) -(if (eq buffer-invisibility-spec t) - prop - (or (memq prop buffer-invisibility-spec) - (assq prop buffer-invisibility-spec) - (defun notmuch-remove-if-not (predicate list) "Return a copy of LIST with all items not satisfying PREDICATE removed." (let (out) -- 1.7.5.4
[PATCH 5/6] emacs: remove no longer used functions from notmuch-show.el
Remove `notmuch-show-move-past-invisible-backward' and `notmuch-show-move-past-invisible-forward' functions which are unused. --- emacs/notmuch-show.el |8 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index ad3cc7b..dcaea65 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -977,14 +977,6 @@ All currently available key bindings: (notmuch-show-move-to-message-top) t)) -(defun notmuch-show-move-past-invisible-forward () - (while (point-invisible-p) -(forward-char))) - -(defun notmuch-show-move-past-invisible-backward () - (while (point-invisible-p) -(backward-char))) - ;; Functions relating to the visibility of messages and their ;; components. -- 1.7.5.4
[PATCH 4/6] emacs: improve hidden signatures handling in notmuch-show-advance-and-archive
Use `previous-single-char-property-change' instead of going through each character by hand and testing it's visibility. This fixes `notmuch-show-advance-and-archive' to work for the last message in thread with hidden signature. --- emacs/notmuch-show.el | 17 + 1 files changed, 9 insertions(+), 8 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 6685717..ad3cc7b 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -1113,17 +1113,18 @@ thread, (remove the \"inbox\" tag from each message). Also kill this buffer, and display the next thread from the search from which this thread was originally shown." (interactive) - (let ((end-of-this-message (notmuch-show-message-bottom))) + (let* ((end-of-this-message (notmuch-show-message-bottom)) +(visible-end-of-this-message (1- end-of-this-message))) +(while (invisible-p visible-end-of-this-message) + (setq visible-end-of-this-message + (previous-single-char-property-change visible-end-of-this-message + 'invisible))) (cond ;; Ideally we would test `end-of-this-message' against the result ;; of `window-end', but that doesn't account for the fact that - ;; the end of the message might be hidden, so we have to actually - ;; go to the end, walk back over invisible text and then see if - ;; point is visible. - ((save-excursion - (goto-char (- end-of-this-message 1)) - (notmuch-show-move-past-invisible-backward) - (> (point) (window-end))) + ;; the end of the message might be hidden. + ((and visible-end-of-this-message + (> visible-end-of-this-message (window-end))) ;; The bottom of this message is not visible - scroll. (scroll-up nil)) -- 1.7.5.4
[PATCH 3/6] test: `notmuch-show-advance-and-archive' with invisible signature
Add Emacs test to check that `notmuch-show-advance-and-archive' works for the last message in thread with invisible signature. --- test/emacs | 14 ++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/test/emacs b/test/emacs index e59de47..bad1122 100755 --- a/test/emacs +++ b/test/emacs @@ -347,4 +347,18 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae at mail. (test-visible-output)' test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages +test_begin_subtest 'notmuch-show-advance-and-archive with invisible signature' +message1='id:20091118010116.GC25380 at dottiness.seas.harvard.edu' +message2='id:1258491078-29658-1-git-send-email-dottedmag at dottedmag.net' +test_emacs "(notmuch-search \"$message1 or $message2\") + (notmuch-test-wait) + (notmuch-search-show-thread) + (goto-char (point-max)) + (redisplay) + (notmuch-show-advance-and-archive) + (test-output)" +test_emacs "(notmuch-show \"$message2\") + (test-output \"EXPECTED\")" +test_expect_equal_file OUTPUT EXPECTED + test_done -- 1.7.5.4
[PATCH 2/6] test: do not set frame width in emacs
No need for `set-frame-width' in emacs tests since it runs in screen now. --- test/test-lib.el |3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/test/test-lib.el b/test/test-lib.el index a783936..97ae593 100644 --- a/test/test-lib.el +++ b/test/test-lib.el @@ -20,9 +20,6 @@ ;; ;; Authors: Dmitry Kurochkin -;; avoid crazy 10-column default of --batch -(set-frame-width (window-frame (get-buffer-window)) 80) - ;; `read-file-name' by default uses `completing-read' function to read ;; user input. It does not respect `standard-input' variable which we ;; use in tests to provide user input. So replace it with a plain -- 1.7.5.4
[PATCH 1/6] test: run emacs inside screen
Before the change, emacs run in daemon mode without any visible
buffers. Turns out that this affects emacs behavior in some
cases. In particular, `window-end' function returns `point-max'
instead of the last visible position. That makes it hard or
impossible to implement some tests. The patch runs emacs in a
detached screen(1) session. So that it works exactly as if it
has a visible window.
Note: screen terminates when emacs exits. So the patch does not
introduce new "running processes left behind" issues.
---
test/test-lib.sh | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 079d7db..8d7c98d 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -873,10 +873,16 @@ EOF
test_emacs () {
if [ -z "$EMACS_SERVER" ]; then
EMACS_SERVER="notmuch-test-suite-$$"
- "$TMP_DIRECTORY/run_emacs" \
- --daemon \
+ # start a detached screen session with an emacs server
+ screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
+ --no-window-system \
--eval "(setq server-name \"$EMACS_SERVER\")" \
+ --eval '(server-start)' \
--eval "(orphan-watchdog $$)" || return
+ # wait until the emacs server is up
+ until test_emacs '()'; do
+ sleep 1
+ done
fi
emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
--
1.7.5.4
Re: Preventing the user shooting themself in the foot
On Wed, 29 Jun 2011 15:40:40 -0700, Carl Worth wrote:
> On Wed, 29 Jun 2011 20:42:01 +0100, Robin Green wrote:
> > It's really dangerous to use the 'a' key in notmuch-mode in an inbox
> > thread which has multiple unread replies! Yes, the other unread replies
> > will still be tagged unread, but the user might not immediately be aware
> > of them. It would be really useful to have an optional warning ("More
> > unread messages in this thread, are you sure?") for this situation!
>
> That was why I originally designed the space bar keybinding for reading
> through each message in turn, (and then it archives the thread only when you
> page past the last message).
>
> The 'a' keybinding, (in turn), was designed for cases when you *know*
> you don't want to read the rest of the thread.
And that is what I use 'a' for, please don't take the single-key binding
away for this. I often glance at a thread, decide it's not interesting
for me and archive the whole thread including all unread messages.
It works perfectly for me. Adding another key for "only archive this
message" would be fine. Do people actually use the "x" keybinding? I
know I don't.
Sebastian
pgpcFaQ23hAUm.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: Preventing the user shooting themself in the foot
On Thu, 30 Jun 2011 13:04:23 +1000, Brian May wrote: > On 30 June 2011 08:40, Carl Worth wrote: > > The 'a' keybinding, (in turn), was designed for cases when you *know* > > you don't want to read the rest of the thread. > > ... in which case it should also mark everything as read. IMHO. I know the current behavior only catches my opinion (and only an opinion I had at one particular point in time). So I won't say this is Right, but I will at least explain what I was thinking: The "unread" tag is distinct from the "inbox" tag. Why two tags? Don't they normally change at the same time? If a key like 'a' got rid of the "unread" tag as well as the "inbox" then there would be almost no need for having two tags. The idea I had is that "inbox" is fully under explicit control by the user. The user must make an intentional decision to "archive" a message in order for that tag to be removed. Distinct from that is "unread" which is handled automatically by the mail client (as well as it can tell what you've actually read or not). So this tag is removed only implicitly, (we don't have specific commands to manipulate the "unread" tag). When the client displays a message as the "current" message it immediately removes the "unread" tag. Whenever it displays a message to the user, (as the "current" message), it removes the unread tag from that message. This means that messages can lose the "unread" tag while still remaining tagged "inbox", (you read a message, but don't archive it), and that messages can lose the "archive" tag while still remaining tagged "unread", (you archive a thread before reading all messages in the thread). The distinction ends up being useful to me. If at some point someone points me to a specific message, and when I search for it I see the "unread" tag, then this highlights to me that I never even looked at the message. > Are there any keyboard bindings to go forwards to the next message or > backwards to the last message without marking anything as archived? As mentioned by someone else, you can navigate the messages in a thread with 'n' and 'p'. One of the obviously missing keybindings is a way to easily navigate From the current thread to the next thread without archiving the current thread. We should probably add that keybinding at some point, but I want to at least point out why I didn't create it originally: The lack of a "move to next thread" binding helps encourage me to form good habits. The goal I have when processing my inbox is to get everything *out* of my inbox. I can do that by deciding one of several common things: * I have nothing to do In this case I should just archive the message immediately * I can deal with this message "on the spot" (such as a quick reply) In this case, I should deal with the message, then archive it * I can't deal with this now, but need to later This is the key scenario. The wrong thing to do is to leave the message in my inbox, (that just makes things pile up and makes my future inbox processing slow, demotivating, and unreliable). The right thing to do is to tag this message in a way that I'm sure I'll find it again when I will be equipped to deal with it. And then I can archive the message. So the right answer always involves archiving the message nearly immediately, (at most after a quick reply or so), and the keybindings encourage archiving over leaving the message in the inbox. Of course, one does have an existing keybinding for "move to next message in thread without archiving"; it just consists of three key presses: 'q', 'n', Enter At that's long enough to discourage its frequent use. So that's a bit of my philosophy and methodology. But like I said, we should probably add the obviously missing keybindings so people with other philosophies and methodologies can use the program comfortably. > Also, just something I have noticed it isn't really obvious that a > thread has replies without scrolling down, and that takes time. Would > be really good if there could be some big/highlighted visual indicator > that there are still unread messages further down. That would be good, yes. -Carl -- carl.d.wo...@intel.com pgpODsHwRr0Fr.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Preventing the user shooting themself in the foot
On Thu, 30 Jun 2011 13:04:23 +1000, Brian May wrote: > On 30 June 2011 08:40, Carl Worth wrote: > > The 'a' keybinding, (in turn), was designed for cases when you *know* > > you don't want to read the rest of the thread. > > ... in which case it should also mark everything as read. IMHO. I know the current behavior only catches my opinion (and only an opinion I had at one particular point in time). So I won't say this is Right, but I will at least explain what I was thinking: The "unread" tag is distinct from the "inbox" tag. Why two tags? Don't they normally change at the same time? If a key like 'a' got rid of the "unread" tag as well as the "inbox" then there would be almost no need for having two tags. The idea I had is that "inbox" is fully under explicit control by the user. The user must make an intentional decision to "archive" a message in order for that tag to be removed. Distinct from that is "unread" which is handled automatically by the mail client (as well as it can tell what you've actually read or not). So this tag is removed only implicitly, (we don't have specific commands to manipulate the "unread" tag). When the client displays a message as the "current" message it immediately removes the "unread" tag. Whenever it displays a message to the user, (as the "current" message), it removes the unread tag from that message. This means that messages can lose the "unread" tag while still remaining tagged "inbox", (you read a message, but don't archive it), and that messages can lose the "archive" tag while still remaining tagged "unread", (you archive a thread before reading all messages in the thread). The distinction ends up being useful to me. If at some point someone points me to a specific message, and when I search for it I see the "unread" tag, then this highlights to me that I never even looked at the message. > Are there any keyboard bindings to go forwards to the next message or > backwards to the last message without marking anything as archived? As mentioned by someone else, you can navigate the messages in a thread with 'n' and 'p'. One of the obviously missing keybindings is a way to easily navigate
[PATCH 2/2] emacs: Tests for user-defined sections
--- test/emacs | 37 test/emacs.expected-output/notmuch-hello |3 +- .../notmuch-hello-new-section |4 ++ .../notmuch-hello-no-saved-searches|3 +- .../notmuch-hello-section-before | 18 + .../notmuch-hello-section-counts |5 +++ .../notmuch-hello-section-hidden-tag |4 ++ .../notmuch-hello-section-with-empty |4 ++ .../emacs.expected-output/notmuch-hello-with-empty |3 +- 9 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 test/emacs.expected-output/notmuch-hello-new-section create mode 100644 test/emacs.expected-output/notmuch-hello-section-before create mode 100644 test/emacs.expected-output/notmuch-hello-section-counts create mode 100644 test/emacs.expected-output/notmuch-hello-section-hidden-tag create mode 100644 test/emacs.expected-output/notmuch-hello-section-with-empty diff --git a/test/emacs b/test/emacs index e59de47..7d795d7 100755 --- a/test/emacs +++ b/test/emacs @@ -34,6 +34,43 @@ test_emacs '(let ((notmuch-saved-searches (test-output))' test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-no-saved-searches +test_begin_subtest "User defined section with inbox tag" +test_emacs "(let ((notmuch-hello-sections + (list (lambda () (notmuch-hello-insert-query-list + \"Test: \" '((\"inbox\" . \"tag:inbox\"))) + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-new-section + +test_begin_subtest "User defined section with empty, hidden entry" +test_emacs "(let ((notmuch-hello-sections + (list (lambda () (notmuch-hello-insert-query-list + \"Test-with-empty:\" + '((\"inbox\" . \"tag:inbox\") + (\"doesnotexist\" . \"tag:doesnotexist\")) + :hide-empty-tags t) + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-with-empty + +test_begin_subtest "User defined section, unread tag filtered out" +test_emacs "(let ((notmuch-hello-sections + (list (lambda () (notmuch-hello-insert-tags-section + :title \"Test-with-filtered: \" + :hide-tags '(\"unread\")) + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-hidden-tag + +test_begin_subtest "User defined section, different query for counts" +test_emacs "(let ((notmuch-hello-sections + (list (lambda () (notmuch-hello-insert-tags-section + :title \"Test-with-counts: \" + :make-count \"tag:signed\") + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-counts + test_begin_subtest "Basic notmuch-search view in emacs" test_emacs '(notmuch-search "tag:inbox") (notmuch-test-wait) diff --git a/test/emacs.expected-output/notmuch-hello b/test/emacs.expected-output/notmuch-hello index 64b7e42..9666327 100644 --- a/test/emacs.expected-output/notmuch-hello +++ b/test/emacs.expected-output/notmuch-hello @@ -6,9 +6,10 @@ Saved searches: [edit] Search: -[Show all tags] +All tags: [show] Type a search query and hit RET to view matching threads. Edit saved searches with the `edit' button. Hit RET or click on a saved search or tag name to view matching threads. `=' refreshes this screen. `s' jumps to the search box. `q' to quit. + diff --git a/test/emacs.expected-output/notmuch-hello-new-section b/test/emacs.expected-output/notmuch-hello-new-section new file mode 100644 index 000..be7b26a --- /dev/null +++ b/test/emacs.expected-output/notmuch-hello-new-section @@ -0,0 +1,4 @@ +Test: [hide] + + 50 inbox + diff --git a/test/emacs.expected-output/notmuch-hello-no-saved-searches b/test/emacs.expected-output/notmuch-hello-no-saved-searches index 7f8206a..744a8f1 100644 --- a/test/emacs.expected-output/notmuch-hello-no-saved-searches +++ b/test/emacs.expected-output/notmuch-hello-no-saved-searches @@ -2,9 +2,10 @@ Search: -[Show all tags] +All tags: [show] Type a search query and hit RET to view matching threads. Edit saved searches with the `edit' button. Hit RET or click on a saved search or tag name to view matching threads. `=' refreshes this screen. `s' jumps to the search box. `q' to quit. + diff --git a/test/emacs.expecte
[PATCH 1/2] emacs: User-defined sections in notmuch-hello
This patch makes the notmuch-hello screen fully customizable by allowing the user to add and remove arbitrary sections. It also provides some convenience functions for constructing sections, e.g. showing the unread message count for each tag. This is done by specifying a list of functions that will be run when notmuch-hello is invoked. --- emacs/notmuch-hello.el | 553 +--- 1 files changed, 339 insertions(+), 214 deletions(-) diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el index 65fde75..e4c9307 100644 --- a/emacs/notmuch-hello.el +++ b/emacs/notmuch-hello.el @@ -55,26 +55,6 @@ :type 'boolean :group 'notmuch) -(defcustom notmuch-hello-tag-list-make-query nil - "Function or string to generate queries for the all tags list. - -This variable controls which query results are shown for each tag -in the \"all tags\" list. If nil, it will use all messages with -that tag. If this is set to a string, it is used as a filter for -messages having that tag (equivalent to \"tag:TAG and (THIS-VARIABLE)\"). -Finally this can be a function that will be called for each tag and -should return a filter for that tag, or nil to hide the tag." - :type '(choice (const :tag "All messages" nil) -(const :tag "Unread messages" "tag:unread") -(const :tag "Custom filter" string) -(const :tag "Custom filter function" function)) - :group 'notmuch) - -(defcustom notmuch-hello-hide-tags nil - "List of tags to be hidden in the \"all tags\"-section." - :type '(repeat string) - :group 'notmuch) - (defface notmuch-hello-logo-background 'class color) (background dark)) @@ -123,6 +103,58 @@ Typically \",\" in the US and UK and \".\" in Europe." (defvar notmuch-hello-recent-searches nil) +(define-widget 'notmuch-hello-customized-section 'lazy + "Customize-type for notmuch-hello sections." + :tag "Customized section" + :type + (let ((opts +'((:title (string :tag "Title for this section")) + (:make-query (string :tag "Filter for each tag")) + (:make-count (string :tag "Different query to generate counts")) + (:hide-tags (repeat :tag "Tags that will be hidden" string)) + (:initially-hidden (boolean :tag "Hide this on startup?")) + (:hide-empty-tags (boolean :tag "Hide tags with no matching messages")) + (:hide-if-empty (boolean :tag "Hide if empty") +`(list :tag "" (const :tag "" lambda) (const :tag "" nil) + (list :tag "Options" +(const :tag "" apply) +(const :tag "" (quote notmuch-hello-insert-all-tags)) +(list :tag "" (const :tag "" quote) + (plist :options ,opts)) + +(defcustom notmuch-hello-sections + (list #'notmuch-hello-insert-header + #'notmuch-hello-insert-saved-searches + #'notmuch-hello-insert-search + #'notmuch-hello-insert-recent-searches + #'notmuch-hello-insert-alltags + #'notmuch-hello-insert-footer) + "Sections for notmuch-hello. + +Each entry of this list should be a function of no arguments +that should return if notmuch-hello-target is produced as part +of its output and nil otherwise. The functions will be run +to construct the content of the notmuch-hello buffer +in the order they appear in this list." + :group 'notmuch + :type + '(repeat +(choice (function-item notmuch-hello-insert-header) + (function-item notmuch-hello-insert-saved-searches) + (function-item notmuch-hello-insert-search) + (function-item notmuch-hello-insert-recent-searches) + (function-item notmuch-hello-insert-alltags) + (function-item notmuch-hello-insert-footer) + notmuch-hello-customized-section))) + +;; only defined to avoid compilation warnings about free variables +(defvar notmuch-hello-target nil) + +(defvar notmuch-hello-hidden-sections nil + "List of query sections whose contents are hidden") + +(defvar notmuch-hello-first-run t) + (defun notmuch-hello-remember-search (search) (if (not (member search notmuch-hello-recent-searches)) (push search notmuch-hello-recent-searches)) @@ -238,12 +270,40 @@ should be. Returns a cons cell `(tags-per-line width)'." (* tags-per-line (+ 9 1 tags-per-line -(defun notmuch-hello-insert-tags (tag-alist widest target) - (let* ((tags-and-width (notmuch-hello-tags-per-line widest)) +(defun notmuch-hello-query-entries (tag-alist &optional hide-empty) + "Compute list of counts and queries for TAG-ALIST. + +If HIDE-EMPTY is non-nil, entries with no matching messages will be +removed from the result." + (notmuch-remove-if-not + #'identity + (mapcar +(lambda (elem) + (let* ((name (car elem)) +(query-and-count (if (consp (cdr elem)) + ;; do we have a different query for the message count? +
[PATCH 0/2] emacs: User-defined sections in notmuch-hello
Unfortunately the customize-interface for more customized entries in notmuch-hello-sections looks a bit weird, but I couldn't figure out how to get rid of the empty lines produced by generating the lambda expression needed. To avoid unnecessary complexity in the code this patch removes aligning all the tag entries in different sections (e.g. saved searches and all tags) the same way. So if someone really thinks this was an important features, please yell loudly. Cheers, Daniel
[PATCH] test: revert non-intentional changes introduced in eb4cf465
eb4cf465 introduces changes which weren't part of the submitted patch (id:"87liwlip2j.fsf at gmail.com"), presumably made during resolving merge conflicts. The first one causes the title of a test to be printed a second time, in place of the correct title: PASS Message with .. in Message-Id: PASS Message with .. in Message-Id: instead of: PASS Message with .. in Message-Id: PASS Sending a message via (fake) SMTP The second one is simply the insertion of a line break, so no harm there. This commit reverts both changes, as they were clearly accidental. Signed-off-by: Pieter Praet --- test/emacs |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/test/emacs b/test/emacs index e59de47..53f455a 100755 --- a/test/emacs +++ b/test/emacs @@ -123,13 +123,13 @@ test_emacs '(notmuch-search "id:\"123..456 at example\"") output=$(notmuch search 'id:"123..456 at example"' | notmuch_search_sanitize) test_expect_equal "$output" "thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Message with .. in Message-Id (inbox search-add show-add)" +test_begin_subtest "Sending a message via (fake) SMTP" emacs_deliver_message \ 'Testing message sent via SMTP' \ 'This is a test that messages are sent via SMTP' \ '(message-goto-to) (kill-whole-line) (insert "To: user at example.com\n")' - sed \ -e s',^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' \ -e s',^Message-ID: <.*>$,Message-ID: ,' < sent_message >OUTPUT -- 1.7.4.1
[PATCH v3] test:Improve test behaviors when --root is used
On Tue, 28 Jun 2011 16:03:00 -0700, Carl Worth wrote: Non-text part: multipart/mixed Non-text part: multipart/signed > On Tue, 28 Jun 2011 16:11:32 -0600, Mark Anderson > wrote: > > Change add_email_corpus, emacs_deliver_message and tests to use > > $TEST_DIRECTORY instead of '..'. > ... > > Document -root option in README and update valgrind to work with > > -root. > > Thanks for the features, Mark. These should all be quite handy. Very useful indeed, great improvement Mark! > > This patch actually fixes what Austin pointed out. > > And thanks for the eagle-eyed review, Austin! > > This is pushed out now. Thanks Carl! However, it appears that (while resolving the merge conflicts?) you've accidentally introduced an [*] extra change in the Emacs tests, which causes PASS Message with .. in Message-Id: PASS Sending a message via (fake) SMTP to become PASS Message with .. in Message-Id: PASS Message with .. in Message-Id: I'll be following up with a patch. [*] Two extra changes actually, but the second one was merely the insertion of a line break. > -Carl > < Non-text part: application/pgp-signature > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch Peace -- Pieter
[PATCH 3/4] emacs: Add pseudo saved search to match mail that no saved search matches
On Wed, 29 Jun 2011 13:49:18 -0700, Jameson Graef Rollins wrote: Non-text part: multipart/signed > On Wed, 29 Jun 2011 20:15:22 +, Jani Nikula wrote: > > So to clarify, do you prefer having on/off switches, or just enabling > > this without customization at all? Personally I'd shy away from the > > latter, but I guess it depends on how useful vs. distracting people find > > this. > > My opinion is to just make it a feature without any customization at > all. I can see no harm in having these searches always available. That should clean up the code a bit as well. Then we're left with picking the sensible names. I just use "other" and "no tags" myself. Do you see value in e.g. surrounding them in brackets like "[other]" and "[no tags]" to signify they are special? Better ideas? I'm not a native speaker... And while at it, how about also making append the non-customizable, IMHO sensible, default for saved searches in patch 2/4? Jani
Re: Preventing the user shooting themself in the foot
On Thu, 30 Jun 2011 13:04:23 +1000, Brian May
wrote:
> Are there any keyboard bindings to go forwards to the next message or
> backwards to the last message without marking anything as archived?
'n' and 'p'. These are documented in the notmuch-show mode help ('?'
while in notmuch-show mode).
jamie.
pgpJynKbvxkcH.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Preventing the user shooting themself in the foot
On Thu, 30 Jun 2011 13:04:23 +1000, Brian May wrote:
> Are there any keyboard bindings to go forwards to the next message or
> backwards to the last message without marking anything as archived?
'n' and 'p'. These are documented in the notmuch-show mode help ('?'
while in notmuch-show mode).
jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL:
<http://notmuchmail.org/pipermail/notmuch/attachments/20110629/4cb021bc/attachment.pgp>
Preventing the user shooting themself in the foot
It's really dangerous to use the 'a' key in notmuch-mode in an inbox
thread which has multiple unread replies! Yes, the other unread replies
will still be tagged unread, but the user might not immediately be aware
of them. It would be really useful to have an optional warning ("More
unread messages in this thread, are you sure?") for this situation!
[PATCH 3/4] emacs: Add pseudo saved search to match mail that no saved search matches
On Wed, 29 Jun 2011 12:14:05 -0700, Jameson Graef Rollins wrote: Non-text part: multipart/signed > On Tue, 28 Jun 2011 07:31:31 +, Jani Nikula wrote: > > Add a pseudo saved search that matches all the mail that no other saved > > search matches. Add new customization option notmuch-saved-searches-nomatch > > to enable and name the pseudo saved search. > > Hi, Jani. I haven't looked too closely at these patches yet, although > they seem to look ok at first glance. However, I would like to argue > *against* using new customization variables for the names of certain > static saved searches. For instance I don't see the point of the > "notmuch-saved-searches-nomatch" and "notmuch-tags-nomatch" > customization variables. Do people *really* need to be able to > customize those names? Why not just pick a sensible name and go with > it? Hi, I didn't add the customization specifically to customize the name, but rather to be able to switch the feature on/off. I felt that people might want to customize that. And while at it, customizing the name in the same variable seemed like a good idea. It's probably not desirable to collide with whatever search/tag names people might use. So to clarify, do you prefer having on/off switches, or just enabling this without customization at all? Personally I'd shy away from the latter, but I guess it depends on how useful vs. distracting people find this. > Customization is great, but if there's too much the code and > customization UI become overly cluttered and hard to parse. Agreed. Jani
Re: Preventing the user shooting themself in the foot
On 30 June 2011 08:40, Carl Worth wrote: > The 'a' keybinding, (in turn), was designed for cases when you *know* > you don't want to read the rest of the thread. ... in which case it should also mark everything as read. IMHO. Are there any keyboard bindings to go forwards to the next message or backwards to the last message without marking anything as archived? Also, just something I have noticed it isn't really obvious that a thread has replies without scrolling down, and that takes time. Would be really good if there could be some big/highlighted visual indicator that there are still unread messages further down. -- Brian May ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Preventing the user shooting themself in the foot
I've spent embarrassingly little time in the emacs UI, so my opinions on this should be taken lightly, but I feel like all of the bindings are of the form "if W, do X and Y, otherwise do Z" and, as a result, I'm actively afraid of what's going to happen when I hit a key. I would much prefer bindings with simple, highly predictable behavior. I'm sure there's some workflow for which these contextual, compound bindings are fantastic, but other workflows wind up fighting against them. I don't have a specific proposal in mind, but Gmail's bindings seem like a good model to emulate (the actions, at least; I've never been too fond of the specific key choices). On Jun 29, 2011 6:40 PM, "Carl Worth" wrote: -- next part -- An HTML attachment was scrubbed... URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/45f1b984/attachment-0001.html>
[PATCH] new: Improved workaround for mistaken new directories
Currently, notmuch new assumes any directory with a database mtime of
0 is new, but we don't set the mtime until after processing messages
and subdirectories in that directory. Hence, anything that prevents
the mtime update (such as an interruption or the wall-clock logic
introduced in 8c39e8d6) will cause the next notmuch new to think the
directory is still new.
We work around this by setting the new directory's database mtime to
-1 before scanning anything in the new directory. This also obviates
the need for the workaround used in 8c39e8d6.
---
notmuch-new.c | 31 ---
1 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/notmuch-new.c b/notmuch-new.c
index c180591..7d17793 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -256,6 +256,25 @@ add_files_recursive (notmuch_database_t *notmuch,
new_directory = db_mtime ? FALSE : TRUE;
+/* XXX This is a temporary workaround. If we don't update the
+ * database mtime until after processing messages in this
+ * directory, then a 0 mtime is *not* sufficient to indicate that
+ * this directory has no messages or subdirs in the database (for
+ * example, if an earlier run skipped the mtime update because
+ * fs_mtime == stat_time, or was interrupted before updating the
+ * mtime at the end). To address this, we record a (bogus)
+ * non-zero value before processing any child messages so that a
+ * later run won't mistake this for a new directory (and, for
+ * example, fail to detect removed files and subdirs).
+ *
+ * A better solution would be for notmuch_database_get_directory
+ * to indicate if it really created a new directory or not, either
+ * by a new out-argument, or by recording this information and
+ * providing an accessor.
+ */
+if (new_directory)
+ notmuch_directory_set_mtime (directory, -1);
+
/* If the database knows about this directory, then we sort based
* on strcmp to match the database sorting. Otherwise, we can do
* inode-based sorting for faster filesystem operation. */
@@ -516,17 +535,7 @@ add_files_recursive (notmuch_database_t *notmuch,
* when we stat'ed the directory, we skip updating the mtime in
* the database because a message could be delivered later in this
* same second. This may lead to unnecessary re-scans, but it
- * avoids overlooking messages.
- *
- * XXX Bug workaround: If this is a new directory, we *must*
- * update the mtime; otherwise the next run will see the 0 mtime
- * and think this is still a new directory containing no files or
- * subdirs (which is unsound in general). If fs_mtime ==
- * stat_time, we set the database mtime to a bogus (but non-zero!)
- * value to force a rescan.
- */
-if (new_directory && fs_mtime == stat_time)
- fs_mtime = 1;
+ * avoids overlooking messages. */
if (! interrupted && fs_mtime != stat_time) {
status = notmuch_directory_set_mtime (directory, fs_mtime);
if (status && ret == NOTMUCH_STATUS_SUCCESS)
--
1.7.5.1
Re: Preventing the user shooting themself in the foot
I've spent embarrassingly little time in the emacs UI, so my opinions on this should be taken lightly, but I feel like all of the bindings are of the form "if W, do X and Y, otherwise do Z" and, as a result, I'm actively afraid of what's going to happen when I hit a key. I would much prefer bindings with simple, highly predictable behavior. I'm sure there's some workflow for which these contextual, compound bindings are fantastic, but other workflows wind up fighting against them. I don't have a specific proposal in mind, but Gmail's bindings seem like a good model to emulate (the actions, at least; I've never been too fond of the specific key choices). On Jun 29, 2011 6:40 PM, "Carl Worth" wrote: ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
notmuch Digest, Vol 20, Issue 57
On Wed, 29 Jun 2011 13:54:40 -0700, Jameson Graef Rollins wrote: > On Wed, 29 Jun 2011 14:21:11 -0600, Mark Anderson > wrote: > > I personally prefer --output=files remain as it was, with one file per > > mail (even though I submitted the patch to change it). I suggest that > > we could add another format to supply all files (perhaps > > --output=allfiles, or --output=dupfiles). I don't like my original > > suggestion of "filelists" because it implies a list of lists to me. A > > list of lists would correlate better to the number of messages which > > match the search terms, but doesn't correlate well to xargs input. > > What's wrong with just outputting all the files matching the search, > including duplicates? I can't think of any reason where one would want > to not include all files matching the search. I would be curious to > hear a use case there. For someone who is using gmail + offlineimap, labels in gmail become folders in maildir. The maildir structure can have a large number of copies of each email corresponding to the labels/tags which have been applied. To add a label/tag that is visible to the gmail interface, one should copy a file representing the message to the folder representing the gmail label, which will then sync to gmail. Copying more than one file for each message being labeled is more wasteful of time and storage. With all files returned, a simple xargs script to add a label by copying files will end up with many copies of the same file in the new directory. The consuming script could hunt for message-id's in files and uniquify, but since notmuch was doing that implicitly before, and it's fairly natural, it seems not a big deal to add. > Since I'm on this kick anyway, I'm going to keep pushing against further > customizations where there really isn't a need. With a common use case for the biggest email userbase which makes labels/tags natural, I think it is worth considering seriously. There are certainly other namesets which could be used to reprecent the two categories. I'm happy to use names that makes the 'allfiles' output the common case and the "one file/message" the longer string, but I think we should provide the "one file/message" output category. The 'allfiles' case is great for deleting all copies of an email, so I definitely want it to continue being available. -Mark
[PATCH 1/6] test: run emacs inside screen
Dmitry Kurochkin wrote:
> Before the change, emacs run in daemon mode without any visible
> buffers. Turns out that this affects emacs behavior in some
> cases. In particular, `window-end' function returns `point-max'
> instead of the last visible position. That makes it hard or
> impossible to implement some tests. The patch runs emacs in a
> detached screen(1) session. So that it works exactly as if it
> has a visible window.
>
> Note: screen terminates when emacs exits. So the patch does not
> introduce new "running processes left behind" issues.
> ---
> test/test-lib.sh | 10 --
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 079d7db..8d7c98d 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -873,10 +873,16 @@ EOF
> test_emacs () {
> if [ -z "$EMACS_SERVER" ]; then
> EMACS_SERVER="notmuch-test-suite-$$"
> - "$TMP_DIRECTORY/run_emacs" \
> - --daemon \
> + # start a detached screen session with an emacs server
> + screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
To avoid problems with ~/.screenrc or /etc/screenrc, you might want e.g.
env SYSSCREENRC=/dev/null SCREENRC=/dev/null screen -S ...
-jim
> + --no-window-system \
> --eval "(setq server-name \"$EMACS_SERVER\")" \
> + --eval '(server-start)' \
> --eval "(orphan-watchdog $$)" || return
> + # wait until the emacs server is up
> + until test_emacs '()'; do
> + sleep 1
> + done
> fi
>
> emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
> --
> 1.7.5.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] new: Improved workaround for mistaken new directories
On Wed, 29 Jun 2011 19:00:01 -0400, Austin Clements wrote: > We work around this by setting the new directory's database mtime to > -1 before scanning anything in the new directory. This also obviates > the need for the workaround used in 8c39e8d6. Thanks. This is pushed. > +/* XXX This is a temporary workaround. If we don't update the > + * database mtime until after processing messages in this ... > + * A better solution would be for notmuch_database_get_directory > + * to indicate if it really created a new directory or not, either > + * by a new out-argument, or by recording this information and > + * providing an accessor. A much better "XXX" comment! It tells the potential reader of the future what to do to be able to remove the comment. Thanks so much. -Carl -- carl.d.wo...@intel.com pgph1gdFIlFid.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] new: Improved workaround for mistaken new directories
On Wed, 29 Jun 2011 19:00:01 -0400, Austin Clements wrote: > We work around this by setting the new directory's database mtime to > -1 before scanning anything in the new directory. This also obviates > the need for the workaround used in 8c39e8d6. Thanks. This is pushed. > +/* XXX This is a temporary workaround. If we don't update the > + * database mtime until after processing messages in this ... > + * A better solution would be for notmuch_database_get_directory > + * to indicate if it really created a new directory or not, either > + * by a new out-argument, or by recording this information and > + * providing an accessor. A much better "XXX" comment! It tells the potential reader of the future what to do to be able to remove the comment. Thanks so much. -Carl -- carl.d.worth at intel.com -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/76c34e16/attachment.pgp>
[PATCH] new: Improved workaround for mistaken new directories
Currently, notmuch new assumes any directory with a database mtime of
0 is new, but we don't set the mtime until after processing messages
and subdirectories in that directory. Hence, anything that prevents
the mtime update (such as an interruption or the wall-clock logic
introduced in 8c39e8d6) will cause the next notmuch new to think the
directory is still new.
We work around this by setting the new directory's database mtime to
-1 before scanning anything in the new directory. This also obviates
the need for the workaround used in 8c39e8d6.
---
notmuch-new.c | 31 ---
1 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/notmuch-new.c b/notmuch-new.c
index c180591..7d17793 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -256,6 +256,25 @@ add_files_recursive (notmuch_database_t *notmuch,
new_directory = db_mtime ? FALSE : TRUE;
+/* XXX This is a temporary workaround. If we don't update the
+ * database mtime until after processing messages in this
+ * directory, then a 0 mtime is *not* sufficient to indicate that
+ * this directory has no messages or subdirs in the database (for
+ * example, if an earlier run skipped the mtime update because
+ * fs_mtime == stat_time, or was interrupted before updating the
+ * mtime at the end). To address this, we record a (bogus)
+ * non-zero value before processing any child messages so that a
+ * later run won't mistake this for a new directory (and, for
+ * example, fail to detect removed files and subdirs).
+ *
+ * A better solution would be for notmuch_database_get_directory
+ * to indicate if it really created a new directory or not, either
+ * by a new out-argument, or by recording this information and
+ * providing an accessor.
+ */
+if (new_directory)
+ notmuch_directory_set_mtime (directory, -1);
+
/* If the database knows about this directory, then we sort based
* on strcmp to match the database sorting. Otherwise, we can do
* inode-based sorting for faster filesystem operation. */
@@ -516,17 +535,7 @@ add_files_recursive (notmuch_database_t *notmuch,
* when we stat'ed the directory, we skip updating the mtime in
* the database because a message could be delivered later in this
* same second. This may lead to unnecessary re-scans, but it
- * avoids overlooking messages.
- *
- * XXX Bug workaround: If this is a new directory, we *must*
- * update the mtime; otherwise the next run will see the 0 mtime
- * and think this is still a new directory containing no files or
- * subdirs (which is unsound in general). If fs_mtime ==
- * stat_time, we set the database mtime to a bogus (but non-zero!)
- * value to force a rescan.
- */
-if (new_directory && fs_mtime == stat_time)
- fs_mtime = 1;
+ * avoids overlooking messages. */
if (! interrupted && fs_mtime != stat_time) {
status = notmuch_directory_set_mtime (directory, fs_mtime);
if (status && ret == NOTMUCH_STATUS_SUCCESS)
--
1.7.5.1
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: Preventing the user shooting themself in the foot
On Wed, 29 Jun 2011 20:42:01 +0100, Robin Green wrote:
> It's really dangerous to use the 'a' key in notmuch-mode in an inbox
> thread which has multiple unread replies! Yes, the other unread replies
> will still be tagged unread, but the user might not immediately be aware
> of them. It would be really useful to have an optional warning ("More
> unread messages in this thread, are you sure?") for this situation!
That was why I originally designed the space bar keybinding for reading
through each message in turn, (and then it archives the thread only when you
page past the last message).
The 'a' keybinding, (in turn), was designed for cases when you *know*
you don't want to read the rest of the thread.
That was the original idea behind these two, at least. In any case,
it's doing exactly what it's defined to do.
Perhaps you're just missing a key for archiving the current message (and
advancing to the next)?
I know there are various problems with the current keybindings—and
several obviously-missing operations. And it's funny how long I've put
up with shortcomings rather than just fixing them.
I'll be glad to look at proposals for improving things.
-Carl
--
carl.d.wo...@intel.com
pgp5blYqBDFEV.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Preventing the user shooting themself in the foot
On Wed, 29 Jun 2011 20:42:01 +0100, Robin Green wrote:
> It's really dangerous to use the 'a' key in notmuch-mode in an inbox
> thread which has multiple unread replies! Yes, the other unread replies
> will still be tagged unread, but the user might not immediately be aware
> of them. It would be really useful to have an optional warning ("More
> unread messages in this thread, are you sure?") for this situation!
That was why I originally designed the space bar keybinding for reading
through each message in turn, (and then it archives the thread only when you
page past the last message).
The 'a' keybinding, (in turn), was designed for cases when you *know*
you don't want to read the rest of the thread.
That was the original idea behind these two, at least. In any case,
it's doing exactly what it's defined to do.
Perhaps you're just missing a key for archiving the current message (and
advancing to the next)?
I know there are various problems with the current keybindings?and
several obviously-missing operations. And it's funny how long I've put
up with shortcomings rather than just fixing them.
I'll be glad to look at proposals for improving things.
-Carl
--
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL:
<http://notmuchmail.org/pipermail/notmuch/attachments/20110629/ce84a857/attachment.pgp>
Re: [PATCH] test: document test_expect_equal_file
On Wed, 29 Jun 2011 10:06:32 -0700, Jameson Graef Rollins wrote: > This test was not properly documented when it was originally added (my > bad). Thanks. Merged! -Carl -- carl.d.wo...@intel.com pgpbz7TFngMlS.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] test: document test_expect_equal_file
On Wed, 29 Jun 2011 10:06:32 -0700, Jameson Graef Rollins wrote: > This test was not properly documented when it was originally added (my > bad). Thanks. Merged! -Carl -- carl.d.worth at intel.com -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/bd90c076/attachment.pgp>
Re: [PATCH] lib/Makefile.local: remove leftover debugging output.
On Wed, 29 Jun 2011 07:42:02 -0300, da...@tethera.net wrote: > From: David Bremner > > The removed "echo $(libnotmuch_modules)" was strictly for debugging. Merged, thanks. -Carl -- carl.d.wo...@intel.com pgpbjPw7WN56c.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] lib/Makefile.local: remove leftover debugging output.
On Wed, 29 Jun 2011 07:42:02 -0300, david at tethera.net wrote: > From: David Bremner > > The removed "echo $(libnotmuch_modules)" was strictly for debugging. Merged, thanks. -Carl -- carl.d.worth at intel.com -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/255b9b38/attachment.pgp>
Re: [PATCH 1/2] new: Don't update DB mtime if FS mtime equals wall-clock time.
On Wed, 29 Jun 2011 03:10:54 -0400, Austin Clements wrote: > This fixes a race where multiple message deliveries in the same second > with an intervening notmuch new could result in messages being ignored > by notmuch (at least, until a later delivery forced a rescan). Excellent fix, Austin. I've merged this, (and the following commit to drop the ugly increment_mtime). I think that even fixes an outstanding portability bug (where someone previously sent patches to change our implementation of increment_mtime). It's sure nice to see notmuch become more robust all the time. -Carl -- carl.d.wo...@intel.com pgptjdcbKCIZa.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] new: Don't update DB mtime if FS mtime equals wall-clock time.
On Wed, 29 Jun 2011 03:10:54 -0400, Austin Clements wrote: > This fixes a race where multiple message deliveries in the same second > with an intervening notmuch new could result in messages being ignored > by notmuch (at least, until a later delivery forced a rescan). Excellent fix, Austin. I've merged this, (and the following commit to drop the ugly increment_mtime). I think that even fixes an outstanding portability bug (where someone previously sent patches to change our implementation of increment_mtime). It's sure nice to see notmuch become more robust all the time. -Carl -- carl.d.worth at intel.com -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/d46766a0/attachment.pgp>
Re: notmuch Digest, Vol 20, Issue 57
On Wed, 29 Jun 2011 13:54:40 -0700, Jameson Graef Rollins wrote: > On Wed, 29 Jun 2011 14:21:11 -0600, Mark Anderson wrote: > > I personally prefer --output=files remain as it was, with one file per > > mail (even though I submitted the patch to change it). I suggest that > > we could add another format to supply all files (perhaps > > --output=allfiles, or --output=dupfiles). I don't like my original > > suggestion of "filelists" because it implies a list of lists to me. A > > list of lists would correlate better to the number of messages which > > match the search terms, but doesn't correlate well to xargs input. > > What's wrong with just outputting all the files matching the search, > including duplicates? I can't think of any reason where one would want > to not include all files matching the search. I would be curious to > hear a use case there. For someone who is using gmail + offlineimap, labels in gmail become folders in maildir. The maildir structure can have a large number of copies of each email corresponding to the labels/tags which have been applied. To add a label/tag that is visible to the gmail interface, one should copy a file representing the message to the folder representing the gmail label, which will then sync to gmail. Copying more than one file for each message being labeled is more wasteful of time and storage. With all files returned, a simple xargs script to add a label by copying files will end up with many copies of the same file in the new directory. The consuming script could hunt for message-id's in files and uniquify, but since notmuch was doing that implicitly before, and it's fairly natural, it seems not a big deal to add. > Since I'm on this kick anyway, I'm going to keep pushing against further > customizations where there really isn't a need. With a common use case for the biggest email userbase which makes labels/tags natural, I think it is worth considering seriously. There are certainly other namesets which could be used to reprecent the two categories. I'm happy to use names that makes the 'allfiles' output the common case and the "one file/message" the longer string, but I think we should provide the "one file/message" output category. The 'allfiles' case is great for deleting all copies of an email, so I definitely want it to continue being available. -Mark ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/4] emacs: Add pseudo saved search to match mail that no saved search matches
On Wed, 29 Jun 2011 13:49:18 -0700, Jameson Graef Rollins wrote: Non-text part: multipart/signed > On Wed, 29 Jun 2011 20:15:22 +, Jani Nikula wrote: > > So to clarify, do you prefer having on/off switches, or just enabling > > this without customization at all? Personally I'd shy away from the > > latter, but I guess it depends on how useful vs. distracting people find > > this. > > My opinion is to just make it a feature without any customization at > all. I can see no harm in having these searches always available. That should clean up the code a bit as well. Then we're left with picking the sensible names. I just use "other" and "no tags" myself. Do you see value in e.g. surrounding them in brackets like "[other]" and "[no tags]" to signify they are special? Better ideas? I'm not a native speaker... And while at it, how about also making append the non-customizable, IMHO sensible, default for saved searches in patch 2/4? Jani ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/6] test: run emacs inside screen
On Wed, 29 Jun 2011 16:16:40 -0400, Jim Paris wrote:
> Dmitry Kurochkin wrote:
> > Before the change, emacs run in daemon mode without any visible
> > buffers. Turns out that this affects emacs behavior in some
> > cases. In particular, `window-end' function returns `point-max'
> > instead of the last visible position. That makes it hard or
> > impossible to implement some tests. The patch runs emacs in a
> > detached screen(1) session. So that it works exactly as if it
> > has a visible window.
> >
> > Note: screen terminates when emacs exits. So the patch does not
> > introduce new "running processes left behind" issues.
> > ---
> > test/test-lib.sh | 10 --
> > 1 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > index 079d7db..8d7c98d 100755
> > --- a/test/test-lib.sh
> > +++ b/test/test-lib.sh
> > @@ -873,10 +873,16 @@ EOF
> > test_emacs () {
> > if [ -z "$EMACS_SERVER" ]; then
> > EMACS_SERVER="notmuch-test-suite-$$"
> > - "$TMP_DIRECTORY/run_emacs" \
> > - --daemon \
> > + # start a detached screen session with an emacs server
> > + screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
>
> To avoid problems with ~/.screenrc or /etc/screenrc, you might want e.g.
> env SYSSCREENRC=/dev/null SCREENRC=/dev/null screen -S ...
>
Thanks for the suggestion, Jim. I sent a patch to fix that.
Regards,
Dmitry
> -jim
>
> > + --no-window-system \
> > --eval "(setq server-name \"$EMACS_SERVER\")" \
> > + --eval '(server-start)' \
> > --eval "(orphan-watchdog $$)" || return
> > + # wait until the emacs server is up
> > + until test_emacs '()'; do
> > + sleep 1
> > + done
> > fi
> >
> > emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
> > --
> > 1.7.5.4
> >
> > ___
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] test: avoid using screen(1) configuration files
Set SCREENRC and SYSSCREENRC environment variables to "/dev/null"
as suggested by Jim Paris to avoid potential problems with
screen(1) configuration files.
---
test/test-lib.sh |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 8d7c98d..7cd4832 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -50,6 +50,8 @@ TZ=UTC
TERM=dumb
export LANG LC_ALL PAGER TERM TZ
GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
+export SCREENRC=/dev/null
+export SYSSCREENRC=/dev/null
# Protect ourselves from common misconfiguration to export
# CDPATH into the environment
--
1.7.5.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] test: revert non-intentional changes introduced in eb4cf465
On Wed, 29 Jun 2011 22:23:47 +0200, Pieter Praet wrote: > eb4cf465 introduces changes which weren't part of the submitted > patch (id:"87liwlip2j@gmail.com"), presumably made during > resolving merge conflicts. Oops, you caught me. > This commit reverts both changes, as they were clearly accidental. Thanks. This is merged (automatically, so hopefully without errors!) and pushed out. -Carl -- carl.d.wo...@intel.com pgpL4htz9rUjs.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] test: revert non-intentional changes introduced in eb4cf465
On Wed, 29 Jun 2011 22:23:47 +0200, Pieter Praet wrote: > eb4cf465 introduces changes which weren't part of the submitted > patch (id:"87liwlip2j.fsf at gmail.com"), presumably made during > resolving merge conflicts. Oops, you caught me. > This commit reverts both changes, as they were clearly accidental. Thanks. This is merged (automatically, so hopefully without errors!) and pushed out. -Carl -- carl.d.worth at intel.com -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/5da48f05/attachment.pgp>
notmuch Digest, Vol 20, Issue 57
On Tue, 28 Jun 2011 16:53:30 -0700, Carl Worth wrote: > On Tue, 28 Jun 2011 16:38:30 -0600, Mark Anderson > wrote: > > I had briefly considered adding another output format "file", just to get a > > single file for each message in the db, but the file/files distinction > > feels a bit niggling. Perhaps it should be changed to "files" and > > "filelists" or something else more clear. > > Another option that would be general to several commands would be: > > notmuch search --output=files --exclude-duplicates > > Or alternately, --include-duplicates. That might be more useful for > "notmuch show" which is a case where users have previously asked for the > ability to ask for duplicate messages, (and where the duplicates are > squelched by default). > > Thoughts? I personally prefer --output=files remain as it was, with one file per mail (even though I submitted the patch to change it). I suggest that we could add another format to supply all files (perhaps --output=allfiles, or --output=dupfiles). I don't like my original suggestion of "filelists" because it implies a list of lists to me. A list of lists would correlate better to the number of messages which match the search terms, but doesn't correlate well to xargs input. I understand that we could use --include-duplicates, but I don't think there are currently any output specifers that actually have a plurality for a single message. If we had something like --output=from, or some other message attribute, then I think we would achieve more useful orthogonality from adding an argument similar to --include-duplicates. As it stands, it looks better to me to have a different --output specifier to represent the uncommon case of multiple outputs per search match. -Mark
Re: [PATCH] Fix folder: coherence issue
On Wed, 29 Jun 2011 14:04:45 -0600, Mark Anderson wrote: > Add removal of all ZXFOLDER terms to removal of all XFOLDER terms for > each message filename removal. > > The existing filename-list reindexing will put all the needed terms > back in. Test search-folder-coherence now passes. Thanks so much, Mark! I've now pushed the new test (followed by a quick fix to turn sleep into increment_mtime) and the fix. David, I recommend these three commits for the release branch. This is an important fix before we first make a release with the folder: feature. -Carl -- carl.d.wo...@intel.com pgpn3p3LxStJT.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Fix folder: coherence issue
On Wed, 29 Jun 2011 14:04:45 -0600, Mark Anderson wrote: > Add removal of all ZXFOLDER terms to removal of all XFOLDER terms for > each message filename removal. > > The existing filename-list reindexing will put all the needed terms > back in. Test search-folder-coherence now passes. Thanks so much, Mark! I've now pushed the new test (followed by a quick fix to turn sleep into increment_mtime) and the fix. David, I recommend these three commits for the release branch. This is an important fix before we first make a release with the folder: feature. -Carl -- carl.d.worth at intel.com -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/a7a01a06/attachment.pgp>
[PATCH] Fix folder: coherence issue
Add removal of all ZXFOLDER terms to removal of all XFOLDER terms for
each message filename removal.
The existing filename-list reindexing will put all the needed terms
back in. Test search-folder-coherence now passes.
Signed-off-by:Mark Anderson
---
Once I fixed the removal instead of the addition side, things went
smoothly.
lib/message.cc | 31 ---
1 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/lib/message.cc b/lib/message.cc
index 8b9c84f..d993cde 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -514,6 +514,8 @@ _notmuch_message_remove_filename (notmuch_message_t
*message,
const char *folder_prefix = _find_prefix ("folder");
int folder_prefix_len = strlen (folder_prefix);
void *local = talloc_new (message);
+char *zfolder_prefix = talloc_asprintf(local, "Z%s", folder_prefix);
+int zfolder_prefix_len = strlen (zfolder_prefix);
char *direntry;
notmuch_private_status_t private_status;
notmuch_status_t status;
@@ -530,9 +532,12 @@ _notmuch_message_remove_filename (notmuch_message_t
*message,
status = COERCE_STATUS (private_status,
"Unexpected error from
_notmuch_message_remove_term");
-/* Re-synchronize "folder:" terms for this message. This requires
- * first removing all "folder:" terms, then adding back terms for
- * all remaining filenames of the message. */
+/* Re-synchronize "folder:" terms for this message. This requires:
+ * 1. removing all "folder:" terms
+ * 2. removing all "folder:" stemmed terms
+ * 3. adding back terms for all remaining filenames of the message. */
+
+/* 1. removing all "folder:" terms */
while (1) {
i = message->doc.termlist_begin ();
i.skip_to (folder_prefix);
@@ -551,6 +556,26 @@ _notmuch_message_remove_filename (notmuch_message_t
*message,
}
}
+/* 2. removing all "folder:" stemmed terms */
+while (1) {
+ i = message->doc.termlist_begin ();
+ i.skip_to (zfolder_prefix);
+
+ /* Terminate loop when no terms remain with desired prefix. */
+ if (i == message->doc.termlist_end () ||
+ strncmp ((*i).c_str (), zfolder_prefix, zfolder_prefix_len))
+ {
+ break;
+ }
+
+ try {
+ message->doc.remove_term ((*i));
+ } catch (const Xapian::InvalidArgumentError) {
+ /* Ignore failure to remove non-existent term. */
+ }
+}
+
+/* 3. adding back terms for all remaining filenames of the message. */
i = message->doc.termlist_begin ();
i.skip_to (direntry_prefix);
--
1.7.4.1
Re: notmuch Digest, Vol 20, Issue 57
On Wed, 29 Jun 2011 14:21:11 -0600, Mark Anderson wrote: > I personally prefer --output=files remain as it was, with one file per > mail (even though I submitted the patch to change it). I suggest that > we could add another format to supply all files (perhaps > --output=allfiles, or --output=dupfiles). I don't like my original > suggestion of "filelists" because it implies a list of lists to me. A > list of lists would correlate better to the number of messages which > match the search terms, but doesn't correlate well to xargs input. What's wrong with just outputting all the files matching the search, including duplicates? I can't think of any reason where one would want to not include all files matching the search. I would be curious to hear a use case there. Since I'm on this kick anyway, I'm going to keep pushing against further customizations where there really isn't a need. jamie. pgp01go6Dr7ZN.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
notmuch Digest, Vol 20, Issue 57
On Wed, 29 Jun 2011 14:21:11 -0600, Mark Anderson wrote: > I personally prefer --output=files remain as it was, with one file per > mail (even though I submitted the patch to change it). I suggest that > we could add another format to supply all files (perhaps > --output=allfiles, or --output=dupfiles). I don't like my original > suggestion of "filelists" because it implies a list of lists to me. A > list of lists would correlate better to the number of messages which > match the search terms, but doesn't correlate well to xargs input. What's wrong with just outputting all the files matching the search, including duplicates? I can't think of any reason where one would want to not include all files matching the search. I would be curious to hear a use case there. Since I'm on this kick anyway, I'm going to keep pushing against further customizations where there really isn't a need. jamie. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/ee93de94/attachment.pgp>
Re: [PATCH 3/4] emacs: Add pseudo saved search to match mail that no saved search matches
On Wed, 29 Jun 2011 20:15:22 +, Jani Nikula wrote: > So to clarify, do you prefer having on/off switches, or just enabling > this without customization at all? Personally I'd shy away from the > latter, but I guess it depends on how useful vs. distracting people find > this. My opinion is to just make it a feature without any customization at all. I can see no harm in having these searches always available. jamie. pgpM72ifxmgxJ.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/4] emacs: Add pseudo saved search to match mail that no saved search matches
On Wed, 29 Jun 2011 20:15:22 +, Jani Nikula wrote: > So to clarify, do you prefer having on/off switches, or just enabling > this without customization at all? Personally I'd shy away from the > latter, but I guess it depends on how useful vs. distracting people find > this. My opinion is to just make it a feature without any customization at all. I can see no harm in having these searches always available. jamie. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/853213de/attachment.pgp>
Re: Preventing the user shooting themself in the foot
On Wed, 29 Jun 2011 20:42:01 +0100, Robin Green wrote:
> It's really dangerous to use the 'a' key in notmuch-mode in an inbox
> thread which has multiple unread replies! Yes, the other unread replies
> will still be tagged unread, but the user might not immediately be aware
> of them. It would be really useful to have an optional warning ("More
> unread messages in this thread, are you sure?") for this situation!
I think that's a bit extreme, but I agree that the default behavior of
the emacs key bindings are not good, particularly in regards to
archiving messages.
I should probably just send in all of my emacs UI tweaks as patches, but
here are a couple of functions/bindings I've added to my .emacs to make
the message processing more sane. The main new function is
notmuch-show-next-open-message-or-pop, which will jump to the next
message in the thread, or pop out of the thread if you're on the last
message. I then use this in my tag processing functions, such as
archiving and deleting.
hth.
jamie.
(defun notmuch-show-next-open-message-or-pop ()
"Show the next message or pop to parent search."
(interactive)
(let (r)
(while (and (setq r (notmuch-show-goto-message-next))
(not (notmuch-show-message-visible-p
(if r
(progn
(notmuch-show-mark-read)
(notmuch-show-message-adjust))
(let ((parent-buffer notmuch-show-parent-buffer))
(if parent-buffer
(progn
(kill-this-buffer)
(switch-to-buffer parent-buffer)
(forward-line 1)))
(define-key notmuch-show-mode-map "a"
(lambda ()
"Archive current message and advance."
(interactive)
(notmuch-show-remove-tag "inbox")
(notmuch-show-next-open-message-or-pop)))
(define-key notmuch-show-mode-map "d"
(lambda ()
"Delete current message and advance to next message."
(interactive)
(notmuch-show-add-tag "delete")
(notmuch-show-next-open-message-or-pop)))
pgpab7FObUqml.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Preventing the user shooting themself in the foot
On Wed, 29 Jun 2011 20:42:01 +0100, Robin Green wrote:
> It's really dangerous to use the 'a' key in notmuch-mode in an inbox
> thread which has multiple unread replies! Yes, the other unread replies
> will still be tagged unread, but the user might not immediately be aware
> of them. It would be really useful to have an optional warning ("More
> unread messages in this thread, are you sure?") for this situation!
I think that's a bit extreme, but I agree that the default behavior of
the emacs key bindings are not good, particularly in regards to
archiving messages.
I should probably just send in all of my emacs UI tweaks as patches, but
here are a couple of functions/bindings I've added to my .emacs to make
the message processing more sane. The main new function is
notmuch-show-next-open-message-or-pop, which will jump to the next
message in the thread, or pop out of the thread if you're on the last
message. I then use this in my tag processing functions, such as
archiving and deleting.
hth.
jamie.
(defun notmuch-show-next-open-message-or-pop ()
"Show the next message or pop to parent search."
(interactive)
(let (r)
(while (and (setq r (notmuch-show-goto-message-next))
(not (notmuch-show-message-visible-p
(if r
(progn
(notmuch-show-mark-read)
(notmuch-show-message-adjust))
(let ((parent-buffer notmuch-show-parent-buffer))
(if parent-buffer
(progn
(kill-this-buffer)
(switch-to-buffer parent-buffer)
(forward-line 1)))
(define-key notmuch-show-mode-map "a"
(lambda ()
"Archive current message and advance."
(interactive)
(notmuch-show-remove-tag "inbox")
(notmuch-show-next-open-message-or-pop)))
(define-key notmuch-show-mode-map "d"
(lambda ()
"Delete current message and advance to next message."
(interactive)
(notmuch-show-add-tag "delete")
(notmuch-show-next-open-message-or-pop)))
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL:
<http://notmuchmail.org/pipermail/notmuch/attachments/20110629/fc70ca80/attachment.pgp>
[PATCH 1/2] emacs: User-defined sections in notmuch-hello
This patch makes the notmuch-hello screen fully customizable by allowing the user to add and remove arbitrary sections. It also provides some convenience functions for constructing sections, e.g. showing the unread message count for each tag. This is done by specifying a list of functions that will be run when notmuch-hello is invoked. --- emacs/notmuch-hello.el | 553 +--- 1 files changed, 339 insertions(+), 214 deletions(-) diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el index 65fde75..e4c9307 100644 --- a/emacs/notmuch-hello.el +++ b/emacs/notmuch-hello.el @@ -55,26 +55,6 @@ :type 'boolean :group 'notmuch) -(defcustom notmuch-hello-tag-list-make-query nil - "Function or string to generate queries for the all tags list. - -This variable controls which query results are shown for each tag -in the \"all tags\" list. If nil, it will use all messages with -that tag. If this is set to a string, it is used as a filter for -messages having that tag (equivalent to \"tag:TAG and (THIS-VARIABLE)\"). -Finally this can be a function that will be called for each tag and -should return a filter for that tag, or nil to hide the tag." - :type '(choice (const :tag "All messages" nil) -(const :tag "Unread messages" "tag:unread") -(const :tag "Custom filter" string) -(const :tag "Custom filter function" function)) - :group 'notmuch) - -(defcustom notmuch-hello-hide-tags nil - "List of tags to be hidden in the \"all tags\"-section." - :type '(repeat string) - :group 'notmuch) - (defface notmuch-hello-logo-background 'class color) (background dark)) @@ -123,6 +103,58 @@ Typically \",\" in the US and UK and \".\" in Europe." (defvar notmuch-hello-recent-searches nil) +(define-widget 'notmuch-hello-customized-section 'lazy + "Customize-type for notmuch-hello sections." + :tag "Customized section" + :type + (let ((opts +'((:title (string :tag "Title for this section")) + (:make-query (string :tag "Filter for each tag")) + (:make-count (string :tag "Different query to generate counts")) + (:hide-tags (repeat :tag "Tags that will be hidden" string)) + (:initially-hidden (boolean :tag "Hide this on startup?")) + (:hide-empty-tags (boolean :tag "Hide tags with no matching messages")) + (:hide-if-empty (boolean :tag "Hide if empty") +`(list :tag "" (const :tag "" lambda) (const :tag "" nil) + (list :tag "Options" +(const :tag "" apply) +(const :tag "" (quote notmuch-hello-insert-all-tags)) +(list :tag "" (const :tag "" quote) + (plist :options ,opts)) + +(defcustom notmuch-hello-sections + (list #'notmuch-hello-insert-header + #'notmuch-hello-insert-saved-searches + #'notmuch-hello-insert-search + #'notmuch-hello-insert-recent-searches + #'notmuch-hello-insert-alltags + #'notmuch-hello-insert-footer) + "Sections for notmuch-hello. + +Each entry of this list should be a function of no arguments +that should return if notmuch-hello-target is produced as part +of its output and nil otherwise. The functions will be run +to construct the content of the notmuch-hello buffer +in the order they appear in this list." + :group 'notmuch + :type + '(repeat +(choice (function-item notmuch-hello-insert-header) + (function-item notmuch-hello-insert-saved-searches) + (function-item notmuch-hello-insert-search) + (function-item notmuch-hello-insert-recent-searches) + (function-item notmuch-hello-insert-alltags) + (function-item notmuch-hello-insert-footer) + notmuch-hello-customized-section))) + +;; only defined to avoid compilation warnings about free variables +(defvar notmuch-hello-target nil) + +(defvar notmuch-hello-hidden-sections nil + "List of query sections whose contents are hidden") + +(defvar notmuch-hello-first-run t) + (defun notmuch-hello-remember-search (search) (if (not (member search notmuch-hello-recent-searches)) (push search notmuch-hello-recent-searches)) @@ -238,12 +270,40 @@ should be. Returns a cons cell `(tags-per-line width)'." (* tags-per-line (+ 9 1 tags-per-line -(defun notmuch-hello-insert-tags (tag-alist widest target) - (let* ((tags-and-width (notmuch-hello-tags-per-line widest)) +(defun notmuch-hello-query-entries (tag-alist &optional hide-empty) + "Compute list of counts and queries for TAG-ALIST. + +If HIDE-EMPTY is non-nil, entries with no matching messages will be +removed from the result." + (notmuch-remove-if-not + #'identity + (mapcar +(lambda (elem) + (let* ((name (car elem)) +(query-and-count (if (consp (cdr elem)) + ;; do we have a different query for the message count? +
[PATCH 2/2] emacs: Tests for user-defined sections
--- test/emacs | 37 test/emacs.expected-output/notmuch-hello |3 +- .../notmuch-hello-new-section |4 ++ .../notmuch-hello-no-saved-searches|3 +- .../notmuch-hello-section-before | 18 + .../notmuch-hello-section-counts |5 +++ .../notmuch-hello-section-hidden-tag |4 ++ .../notmuch-hello-section-with-empty |4 ++ .../emacs.expected-output/notmuch-hello-with-empty |3 +- 9 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 test/emacs.expected-output/notmuch-hello-new-section create mode 100644 test/emacs.expected-output/notmuch-hello-section-before create mode 100644 test/emacs.expected-output/notmuch-hello-section-counts create mode 100644 test/emacs.expected-output/notmuch-hello-section-hidden-tag create mode 100644 test/emacs.expected-output/notmuch-hello-section-with-empty diff --git a/test/emacs b/test/emacs index e59de47..7d795d7 100755 --- a/test/emacs +++ b/test/emacs @@ -34,6 +34,43 @@ test_emacs '(let ((notmuch-saved-searches (test-output))' test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-no-saved-searches +test_begin_subtest "User defined section with inbox tag" +test_emacs "(let ((notmuch-hello-sections + (list (lambda () (notmuch-hello-insert-query-list + \"Test: \" '((\"inbox\" . \"tag:inbox\"))) + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-new-section + +test_begin_subtest "User defined section with empty, hidden entry" +test_emacs "(let ((notmuch-hello-sections + (list (lambda () (notmuch-hello-insert-query-list + \"Test-with-empty:\" + '((\"inbox\" . \"tag:inbox\") + (\"doesnotexist\" . \"tag:doesnotexist\")) + :hide-empty-tags t) + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-with-empty + +test_begin_subtest "User defined section, unread tag filtered out" +test_emacs "(let ((notmuch-hello-sections + (list (lambda () (notmuch-hello-insert-tags-section + :title \"Test-with-filtered: \" + :hide-tags '(\"unread\")) + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-hidden-tag + +test_begin_subtest "User defined section, different query for counts" +test_emacs "(let ((notmuch-hello-sections + (list (lambda () (notmuch-hello-insert-tags-section + :title \"Test-with-counts: \" + :make-count \"tag:signed\") + (notmuch-hello) + (test-output))" +test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-section-counts + test_begin_subtest "Basic notmuch-search view in emacs" test_emacs '(notmuch-search "tag:inbox") (notmuch-test-wait) diff --git a/test/emacs.expected-output/notmuch-hello b/test/emacs.expected-output/notmuch-hello index 64b7e42..9666327 100644 --- a/test/emacs.expected-output/notmuch-hello +++ b/test/emacs.expected-output/notmuch-hello @@ -6,9 +6,10 @@ Saved searches: [edit] Search: -[Show all tags] +All tags: [show] Type a search query and hit RET to view matching threads. Edit saved searches with the `edit' button. Hit RET or click on a saved search or tag name to view matching threads. `=' refreshes this screen. `s' jumps to the search box. `q' to quit. + diff --git a/test/emacs.expected-output/notmuch-hello-new-section b/test/emacs.expected-output/notmuch-hello-new-section new file mode 100644 index 000..be7b26a --- /dev/null +++ b/test/emacs.expected-output/notmuch-hello-new-section @@ -0,0 +1,4 @@ +Test: [hide] + + 50 inbox + diff --git a/test/emacs.expected-output/notmuch-hello-no-saved-searches b/test/emacs.expected-output/notmuch-hello-no-saved-searches index 7f8206a..744a8f1 100644 --- a/test/emacs.expected-output/notmuch-hello-no-saved-searches +++ b/test/emacs.expected-output/notmuch-hello-no-saved-searches @@ -2,9 +2,10 @@ Search: -[Show all tags] +All tags: [show] Type a search query and hit RET to view matching threads. Edit saved searches with the `edit' button. Hit RET or click on a saved search or tag name to view matching threads. `=' refreshes this screen. `s' jumps to the search box. `q' to quit. + diff --git a/test/emacs.
[PATCH 0/2] emacs: User-defined sections in notmuch-hello
Unfortunately the customize-interface for more customized entries in notmuch-hello-sections looks a bit weird, but I couldn't figure out how to get rid of the empty lines produced by generating the lambda expression needed. To avoid unnecessary complexity in the code this patch removes aligning all the tag entries in different sections (e.g. saved searches and all tags) the same way. So if someone really thinks this was an important features, please yell loudly. Cheers, Daniel ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] test: revert non-intentional changes introduced in eb4cf465
eb4cf465 introduces changes which weren't part of the submitted patch (id:"87liwlip2j@gmail.com"), presumably made during resolving merge conflicts. The first one causes the title of a test to be printed a second time, in place of the correct title: PASS Message with .. in Message-Id: PASS Message with .. in Message-Id: instead of: PASS Message with .. in Message-Id: PASS Sending a message via (fake) SMTP The second one is simply the insertion of a line break, so no harm there. This commit reverts both changes, as they were clearly accidental. Signed-off-by: Pieter Praet --- test/emacs |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/test/emacs b/test/emacs index e59de47..53f455a 100755 --- a/test/emacs +++ b/test/emacs @@ -123,13 +123,13 @@ test_emacs '(notmuch-search "id:\"123..456@example\"") output=$(notmuch search 'id:"123..456@example"' | notmuch_search_sanitize) test_expect_equal "$output" "thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Message with .. in Message-Id (inbox search-add show-add)" +test_begin_subtest "Sending a message via (fake) SMTP" emacs_deliver_message \ 'Testing message sent via SMTP' \ 'This is a test that messages are sent via SMTP' \ '(message-goto-to) (kill-whole-line) (insert "To: u...@example.com\n")' - sed \ -e s',^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' \ -e s',^Message-ID: <.*>$,Message-ID: ,' < sent_message >OUTPUT -- 1.7.4.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/6] test: run emacs inside screen
Dmitry Kurochkin wrote:
> Before the change, emacs run in daemon mode without any visible
> buffers. Turns out that this affects emacs behavior in some
> cases. In particular, `window-end' function returns `point-max'
> instead of the last visible position. That makes it hard or
> impossible to implement some tests. The patch runs emacs in a
> detached screen(1) session. So that it works exactly as if it
> has a visible window.
>
> Note: screen terminates when emacs exits. So the patch does not
> introduce new "running processes left behind" issues.
> ---
> test/test-lib.sh | 10 --
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 079d7db..8d7c98d 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -873,10 +873,16 @@ EOF
> test_emacs () {
> if [ -z "$EMACS_SERVER" ]; then
> EMACS_SERVER="notmuch-test-suite-$$"
> - "$TMP_DIRECTORY/run_emacs" \
> - --daemon \
> + # start a detached screen session with an emacs server
> + screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
To avoid problems with ~/.screenrc or /etc/screenrc, you might want e.g.
env SYSSCREENRC=/dev/null SCREENRC=/dev/null screen -S ...
-jim
> + --no-window-system \
> --eval "(setq server-name \"$EMACS_SERVER\")" \
> + --eval '(server-start)' \
> --eval "(orphan-watchdog $$)" || return
> + # wait until the emacs server is up
> + until test_emacs '()'; do
> + sleep 1
> + done
> fi
>
> emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
> --
> 1.7.5.4
>
> ___
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: notmuch Digest, Vol 20, Issue 57
On Tue, 28 Jun 2011 16:53:30 -0700, Carl Worth wrote: > On Tue, 28 Jun 2011 16:38:30 -0600, Mark Anderson wrote: > > I had briefly considered adding another output format "file", just to get a > > single file for each message in the db, but the file/files distinction > > feels a bit niggling. Perhaps it should be changed to "files" and > > "filelists" or something else more clear. > > Another option that would be general to several commands would be: > > notmuch search --output=files --exclude-duplicates > > Or alternately, --include-duplicates. That might be more useful for > "notmuch show" which is a case where users have previously asked for the > ability to ask for duplicate messages, (and where the duplicates are > squelched by default). > > Thoughts? I personally prefer --output=files remain as it was, with one file per mail (even though I submitted the patch to change it). I suggest that we could add another format to supply all files (perhaps --output=allfiles, or --output=dupfiles). I don't like my original suggestion of "filelists" because it implies a list of lists to me. A list of lists would correlate better to the number of messages which match the search terms, but doesn't correlate well to xargs input. I understand that we could use --include-duplicates, but I don't think there are currently any output specifers that actually have a plurality for a single message. If we had something like --output=from, or some other message attribute, then I think we would achieve more useful orthogonality from adding an argument similar to --include-duplicates. As it stands, it looks better to me to have a different --output specifier to represent the uncommon case of multiple outputs per search match. -Mark ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3] test:Improve test behaviors when --root is used
On Tue, 28 Jun 2011 16:03:00 -0700, Carl Worth wrote: Non-text part: multipart/mixed Non-text part: multipart/signed > On Tue, 28 Jun 2011 16:11:32 -0600, Mark Anderson wrote: > > Change add_email_corpus, emacs_deliver_message and tests to use > > $TEST_DIRECTORY instead of '..'. > ... > > Document -root option in README and update valgrind to work with > > -root. > > Thanks for the features, Mark. These should all be quite handy. Very useful indeed, great improvement Mark! > > This patch actually fixes what Austin pointed out. > > And thanks for the eagle-eyed review, Austin! > > This is pushed out now. Thanks Carl! However, it appears that (while resolving the merge conflicts?) you've accidentally introduced an [*] extra change in the Emacs tests, which causes PASS Message with .. in Message-Id: PASS Sending a message via (fake) SMTP to become PASS Message with .. in Message-Id: PASS Message with .. in Message-Id: I'll be following up with a patch. [*] Two extra changes actually, but the second one was merely the insertion of a line break. > -Carl > < Non-text part: application/pgp-signature > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch Peace -- Pieter ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Fix folder: coherence issue
Add removal of all ZXFOLDER terms to removal of all XFOLDER terms for
each message filename removal.
The existing filename-list reindexing will put all the needed terms
back in. Test search-folder-coherence now passes.
Signed-off-by:Mark Anderson
---
Once I fixed the removal instead of the addition side, things went
smoothly.
lib/message.cc | 31 ---
1 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/lib/message.cc b/lib/message.cc
index 8b9c84f..d993cde 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -514,6 +514,8 @@ _notmuch_message_remove_filename (notmuch_message_t
*message,
const char *folder_prefix = _find_prefix ("folder");
int folder_prefix_len = strlen (folder_prefix);
void *local = talloc_new (message);
+char *zfolder_prefix = talloc_asprintf(local, "Z%s", folder_prefix);
+int zfolder_prefix_len = strlen (zfolder_prefix);
char *direntry;
notmuch_private_status_t private_status;
notmuch_status_t status;
@@ -530,9 +532,12 @@ _notmuch_message_remove_filename (notmuch_message_t
*message,
status = COERCE_STATUS (private_status,
"Unexpected error from
_notmuch_message_remove_term");
-/* Re-synchronize "folder:" terms for this message. This requires
- * first removing all "folder:" terms, then adding back terms for
- * all remaining filenames of the message. */
+/* Re-synchronize "folder:" terms for this message. This requires:
+ * 1. removing all "folder:" terms
+ * 2. removing all "folder:" stemmed terms
+ * 3. adding back terms for all remaining filenames of the message. */
+
+/* 1. removing all "folder:" terms */
while (1) {
i = message->doc.termlist_begin ();
i.skip_to (folder_prefix);
@@ -551,6 +556,26 @@ _notmuch_message_remove_filename (notmuch_message_t
*message,
}
}
+/* 2. removing all "folder:" stemmed terms */
+while (1) {
+ i = message->doc.termlist_begin ();
+ i.skip_to (zfolder_prefix);
+
+ /* Terminate loop when no terms remain with desired prefix. */
+ if (i == message->doc.termlist_end () ||
+ strncmp ((*i).c_str (), zfolder_prefix, zfolder_prefix_len))
+ {
+ break;
+ }
+
+ try {
+ message->doc.remove_term ((*i));
+ } catch (const Xapian::InvalidArgumentError) {
+ /* Ignore failure to remove non-existent term. */
+ }
+}
+
+/* 3. adding back terms for all remaining filenames of the message. */
i = message->doc.termlist_begin ();
i.skip_to (direntry_prefix);
--
1.7.4.1
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/4] emacs: Add pseudo saved search to match mail that no saved search matches
On Wed, 29 Jun 2011 12:14:05 -0700, Jameson Graef Rollins wrote: Non-text part: multipart/signed > On Tue, 28 Jun 2011 07:31:31 +, Jani Nikula wrote: > > Add a pseudo saved search that matches all the mail that no other saved > > search matches. Add new customization option notmuch-saved-searches-nomatch > > to enable and name the pseudo saved search. > > Hi, Jani. I haven't looked too closely at these patches yet, although > they seem to look ok at first glance. However, I would like to argue > *against* using new customization variables for the names of certain > static saved searches. For instance I don't see the point of the > "notmuch-saved-searches-nomatch" and "notmuch-tags-nomatch" > customization variables. Do people *really* need to be able to > customize those names? Why not just pick a sensible name and go with > it? Hi, I didn't add the customization specifically to customize the name, but rather to be able to switch the feature on/off. I felt that people might want to customize that. And while at it, customizing the name in the same variable seemed like a good idea. It's probably not desirable to collide with whatever search/tag names people might use. So to clarify, do you prefer having on/off switches, or just enabling this without customization at all? Personally I'd shy away from the latter, but I guess it depends on how useful vs. distracting people find this. > Customization is great, but if there's too much the code and > customization UI become overly cluttered and hard to parse. Agreed. Jani ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/3] test: `notmuch-show-advance-and-archive' with invisible signature
On Wed, 29 Jun 2011 09:10:19 +0400, Dmitry Kurochkin wrote: > On Wed, 29 Jun 2011 05:48:50 +0400, Dmitry Kurochkin > wrote: > > Add Emacs test to check that `notmuch-show-advance-and-archive' > > works for the last message in thread with invisible signature. > > --- > > > > This patch series fixes the bug reported by Sebastien in [1]. I > > was able to reproduce it and confirm that the second patch from > > this series fixes the problem. Unfortunately, I can not explain > > why it fixes it. The patch uses a cleaner approach for visible > > text search. But the old approach should work fine as well. > > Apparently, it does not work when `invisible' property is not a > > single symbol but a list (which was changed in > > 95ef8da29439f2e79115c36ab4d2a80aef1a1462). I suspect that it is > > an Emacs bug. I plan to look at it later. > > > > Turns out that `point-invisible-p' is a function from notmuch-lib.el, I > did not realize that before. It implements a custom visibility check > which is incomplete and does not work correctly when `invisible' > property is a list. That is why the previous code (which used > `point-invisible-p') had the bug. I sent another patch that removes > `point-invisible-p' function. > > > Another issue is that the test does not demonstrate the bug. > > Again, I do not really know why. It passes both before and after > > the fix. Although if I run the test commands by hand I hit the > > bug. I guess it has something to do with emacs daemon mode when > > the buffer is not visible. I hope someone with a better elisp > > knowledge can tell what is going on and how to make the test > > work. > > > > Now it is clear where the bug was. Remaining question is how to test > it. > I have posted a new version of this patch series [1]. It fixes the test to actually demonstrate the bug. Regards, Dmitry [1] id:"1309376558-26284-1-git-send-email-dmitry.kuroch...@gmail.com" > Regards, > Dmitry > > > I believe patches 2 and 3 can be pushed after review even without > > a working test. > > > > Regards, > > Dmitry > > > > [1] id:"8739j5rn2d@cern.ch" > > > > test/emacs | 12 > > 1 files changed, 12 insertions(+), 0 deletions(-) > > > > diff --git a/test/emacs b/test/emacs > > index e59de47..65a96a5 100755 > > --- a/test/emacs > > +++ b/test/emacs > > @@ -347,4 +347,16 @@ test_emacs '(notmuch-show > > "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail. > > (test-visible-output)' > > test_expect_equal_file OUTPUT > > $EXPECTED/notmuch-show-thread-with-hidden-messages > > > > +test_begin_subtest 'notmuch-show-advance-and-archive with invisible > > signature' > > +message1='id:20091118010116.gc25...@dottiness.seas.harvard.edu' > > +message2='id:1258491078-29658-1-git-send-email-dotted...@dottedmag.net' > > +test_emacs "(notmuch-search \"$message1 or $message2\") > > + (notmuch-test-wait) > > + (notmuch-search-show-thread) > > + (notmuch-show-advance-and-archive) > > + (test-output)" > > +test_emacs "(notmuch-show \"$message2\") > > + (test-output \"EXPECTED\")" > > +test_expect_equal_file OUTPUT EXPECTED > > + > > test_done > > -- > > 1.7.5.4 > > ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 6/6] emacs: remove unused `point-invisible-p' function
`point-invisible-p' does not work correctly when `invisible' property is a list. There are standard `invisible-p' and related functions that should be used instead. --- emacs/notmuch-lib.el | 15 --- 1 files changed, 0 insertions(+), 15 deletions(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index f93c957..0f856bf 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -105,21 +105,6 @@ the user hasn't set this variable with the old or new value." ;; -;; XXX: This should be a generic function in emacs somewhere, not -;; here. -(defun point-invisible-p () - "Return whether the character at point is invisible. - -Here visibility is determined by `buffer-invisibility-spec' and -the invisible property of any overlays for point. It doesn't have -anything to do with whether point is currently being displayed -within the current window." - (let ((prop (get-char-property (point) 'invisible))) -(if (eq buffer-invisibility-spec t) - prop - (or (memq prop buffer-invisibility-spec) - (assq prop buffer-invisibility-spec) - (defun notmuch-remove-if-not (predicate list) "Return a copy of LIST with all items not satisfying PREDICATE removed." (let (out) -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 5/6] emacs: remove no longer used functions from notmuch-show.el
Remove `notmuch-show-move-past-invisible-backward' and `notmuch-show-move-past-invisible-forward' functions which are unused. --- emacs/notmuch-show.el |8 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index ad3cc7b..dcaea65 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -977,14 +977,6 @@ All currently available key bindings: (notmuch-show-move-to-message-top) t)) -(defun notmuch-show-move-past-invisible-forward () - (while (point-invisible-p) -(forward-char))) - -(defun notmuch-show-move-past-invisible-backward () - (while (point-invisible-p) -(backward-char))) - ;; Functions relating to the visibility of messages and their ;; components. -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 4/6] emacs: improve hidden signatures handling in notmuch-show-advance-and-archive
Use `previous-single-char-property-change' instead of going through each character by hand and testing it's visibility. This fixes `notmuch-show-advance-and-archive' to work for the last message in thread with hidden signature. --- emacs/notmuch-show.el | 17 + 1 files changed, 9 insertions(+), 8 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 6685717..ad3cc7b 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -1113,17 +1113,18 @@ thread, (remove the \"inbox\" tag from each message). Also kill this buffer, and display the next thread from the search from which this thread was originally shown." (interactive) - (let ((end-of-this-message (notmuch-show-message-bottom))) + (let* ((end-of-this-message (notmuch-show-message-bottom)) +(visible-end-of-this-message (1- end-of-this-message))) +(while (invisible-p visible-end-of-this-message) + (setq visible-end-of-this-message + (previous-single-char-property-change visible-end-of-this-message + 'invisible))) (cond ;; Ideally we would test `end-of-this-message' against the result ;; of `window-end', but that doesn't account for the fact that - ;; the end of the message might be hidden, so we have to actually - ;; go to the end, walk back over invisible text and then see if - ;; point is visible. - ((save-excursion - (goto-char (- end-of-this-message 1)) - (notmuch-show-move-past-invisible-backward) - (> (point) (window-end))) + ;; the end of the message might be hidden. + ((and visible-end-of-this-message + (> visible-end-of-this-message (window-end))) ;; The bottom of this message is not visible - scroll. (scroll-up nil)) -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/6] test: `notmuch-show-advance-and-archive' with invisible signature
Add Emacs test to check that `notmuch-show-advance-and-archive' works for the last message in thread with invisible signature. --- test/emacs | 14 ++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/test/emacs b/test/emacs index e59de47..bad1122 100755 --- a/test/emacs +++ b/test/emacs @@ -347,4 +347,18 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail. (test-visible-output)' test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages +test_begin_subtest 'notmuch-show-advance-and-archive with invisible signature' +message1='id:20091118010116.gc25...@dottiness.seas.harvard.edu' +message2='id:1258491078-29658-1-git-send-email-dotted...@dottedmag.net' +test_emacs "(notmuch-search \"$message1 or $message2\") + (notmuch-test-wait) + (notmuch-search-show-thread) + (goto-char (point-max)) + (redisplay) + (notmuch-show-advance-and-archive) + (test-output)" +test_emacs "(notmuch-show \"$message2\") + (test-output \"EXPECTED\")" +test_expect_equal_file OUTPUT EXPECTED + test_done -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/6] test: do not set frame width in emacs
No need for `set-frame-width' in emacs tests since it runs in screen now. --- test/test-lib.el |3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/test/test-lib.el b/test/test-lib.el index a783936..97ae593 100644 --- a/test/test-lib.el +++ b/test/test-lib.el @@ -20,9 +20,6 @@ ;; ;; Authors: Dmitry Kurochkin -;; avoid crazy 10-column default of --batch -(set-frame-width (window-frame (get-buffer-window)) 80) - ;; `read-file-name' by default uses `completing-read' function to read ;; user input. It does not respect `standard-input' variable which we ;; use in tests to provide user input. So replace it with a plain -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/6] test: run emacs inside screen
Before the change, emacs run in daemon mode without any visible
buffers. Turns out that this affects emacs behavior in some
cases. In particular, `window-end' function returns `point-max'
instead of the last visible position. That makes it hard or
impossible to implement some tests. The patch runs emacs in a
detached screen(1) session. So that it works exactly as if it
has a visible window.
Note: screen terminates when emacs exits. So the patch does not
introduce new "running processes left behind" issues.
---
test/test-lib.sh | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 079d7db..8d7c98d 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -873,10 +873,16 @@ EOF
test_emacs () {
if [ -z "$EMACS_SERVER" ]; then
EMACS_SERVER="notmuch-test-suite-$$"
- "$TMP_DIRECTORY/run_emacs" \
- --daemon \
+ # start a detached screen session with an emacs server
+ screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
+ --no-window-system \
--eval "(setq server-name \"$EMACS_SERVER\")" \
+ --eval '(server-start)' \
--eval "(orphan-watchdog $$)" || return
+ # wait until the emacs server is up
+ until test_emacs '()'; do
+ sleep 1
+ done
fi
emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
--
1.7.5.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Preventing the user shooting themself in the foot
It's really dangerous to use the 'a' key in notmuch-mode in an inbox
thread which has multiple unread replies! Yes, the other unread replies
will still be tagged unread, but the user might not immediately be aware
of them. It would be really useful to have an optional warning ("More
unread messages in this thread, are you sure?") for this situation!
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/4] emacs: Add pseudo saved search to match mail that no saved search matches
On Tue, 28 Jun 2011 07:31:31 +, Jani Nikula wrote: > Add a pseudo saved search that matches all the mail that no other saved > search matches. Add new customization option notmuch-saved-searches-nomatch > to enable and name the pseudo saved search. Hi, Jani. I haven't looked too closely at these patches yet, although they seem to look ok at first glance. However, I would like to argue *against* using new customization variables for the names of certain static saved searches. For instance I don't see the point of the "notmuch-saved-searches-nomatch" and "notmuch-tags-nomatch" customization variables. Do people *really* need to be able to customize those names? Why not just pick a sensible name and go with it? Customization is great, but if there's too much the code and customization UI become overly cluttered and hard to parse. jmho jamie. pgp1GOHu2YuTs.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/4] emacs: Add pseudo saved search to match mail that no saved search matches
On Tue, 28 Jun 2011 07:31:31 +, Jani Nikula wrote: > Add a pseudo saved search that matches all the mail that no other saved > search matches. Add new customization option notmuch-saved-searches-nomatch > to enable and name the pseudo saved search. Hi, Jani. I haven't looked too closely at these patches yet, although they seem to look ok at first glance. However, I would like to argue *against* using new customization variables for the names of certain static saved searches. For instance I don't see the point of the "notmuch-saved-searches-nomatch" and "notmuch-tags-nomatch" customization variables. Do people *really* need to be able to customize those names? Why not just pick a sensible name and go with it? Customization is great, but if there's too much the code and customization UI become overly cluttered and hard to parse. jmho jamie. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/0c20cc13/attachment.pgp>
[PATCH] test: document test_expect_equal_file
This test was not properly documented when it was originally added (my bad). --- test/README |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/test/README b/test/README index 8fbf78d..f9ac607 100644 --- a/test/README +++ b/test/README @@ -156,6 +156,13 @@ library for your script to use. will generate a failure and print the difference of the two strings. + test_expect_equal_file + + Identical to test_exepect_equal, except that and +are files instead of strings. This is a much more + robust method to compare formatted textual information, since it + also notices whitespace and closing newline differences. + test_expect_equal_failure This works similar to test_expect_equal (see above) but is used to -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] test: document test_expect_equal_file
This test was not properly documented when it was originally added (my bad). --- test/README |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/test/README b/test/README index 8fbf78d..f9ac607 100644 --- a/test/README +++ b/test/README @@ -156,6 +156,13 @@ library for your script to use. will generate a failure and print the difference of the two strings. + test_expect_equal_file + + Identical to test_exepect_equal, except that and +are files instead of strings. This is a much more + robust method to compare formatted textual information, since it + also notices whitespace and closing newline differences. + test_expect_equal_failure This works similar to test_expect_equal (see above) but is used to -- 1.7.5.4
[PATCH 1/3] test: `notmuch-show-advance-and-archive' with invisible signature
On Wed, 29 Jun 2011 05:48:50 +0400, Dmitry Kurochkin wrote: > Add Emacs test to check that `notmuch-show-advance-and-archive' > works for the last message in thread with invisible signature. > --- > > This patch series fixes the bug reported by Sebastien in [1]. I > was able to reproduce it and confirm that the second patch from > this series fixes the problem. Unfortunately, I can not explain > why it fixes it. The patch uses a cleaner approach for visible > text search. But the old approach should work fine as well. > Apparently, it does not work when `invisible' property is not a > single symbol but a list (which was changed in > 95ef8da29439f2e79115c36ab4d2a80aef1a1462). I suspect that it is > an Emacs bug. I plan to look at it later. > Turns out that `point-invisible-p' is a function from notmuch-lib.el, I did not realize that before. It implements a custom visibility check which is incomplete and does not work correctly when `invisible' property is a list. That is why the previous code (which used `point-invisible-p') had the bug. I sent another patch that removes `point-invisible-p' function. > Another issue is that the test does not demonstrate the bug. > Again, I do not really know why. It passes both before and after > the fix. Although if I run the test commands by hand I hit the > bug. I guess it has something to do with emacs daemon mode when > the buffer is not visible. I hope someone with a better elisp > knowledge can tell what is going on and how to make the test > work. > Now it is clear where the bug was. Remaining question is how to test it. Regards, Dmitry > I believe patches 2 and 3 can be pushed after review even without > a working test. > > Regards, > Dmitry > > [1] id:"8739j5rn2d.fsf at cern.ch" > > test/emacs | 12 > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/test/emacs b/test/emacs > index e59de47..65a96a5 100755 > --- a/test/emacs > +++ b/test/emacs > @@ -347,4 +347,16 @@ test_emacs '(notmuch-show > "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae at mail. > (test-visible-output)' > test_expect_equal_file OUTPUT > $EXPECTED/notmuch-show-thread-with-hidden-messages > > +test_begin_subtest 'notmuch-show-advance-and-archive with invisible > signature' > +message1='id:20091118010116.GC25380 at dottiness.seas.harvard.edu' > +message2='id:1258491078-29658-1-git-send-email-dottedmag at dottedmag.net' > +test_emacs "(notmuch-search \"$message1 or $message2\") > + (notmuch-test-wait) > + (notmuch-search-show-thread) > + (notmuch-show-advance-and-archive) > + (test-output)" > +test_emacs "(notmuch-show \"$message2\") > + (test-output \"EXPECTED\")" > +test_expect_equal_file OUTPUT EXPECTED > + > test_done > -- > 1.7.5.4 >
[PATCH] emacs: remove unused `point-invisible-p' function
`point-invisible-p' does not work correctly when `invisible' property is a list. There are standard `invisible-p' and related functions that should be used instead. --- emacs/notmuch-lib.el | 15 --- 1 files changed, 0 insertions(+), 15 deletions(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index f93c957..0f856bf 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -105,21 +105,6 @@ the user hasn't set this variable with the old or new value." ;; -;; XXX: This should be a generic function in emacs somewhere, not -;; here. -(defun point-invisible-p () - "Return whether the character at point is invisible. - -Here visibility is determined by `buffer-invisibility-spec' and -the invisible property of any overlays for point. It doesn't have -anything to do with whether point is currently being displayed -within the current window." - (let ((prop (get-char-property (point) 'invisible))) -(if (eq buffer-invisibility-spec t) - prop - (or (memq prop buffer-invisibility-spec) - (assq prop buffer-invisibility-spec) - (defun notmuch-remove-if-not (predicate list) "Return a copy of LIST with all items not satisfying PREDICATE removed." (let (out) -- 1.7.5.4
[PATCH 2/2] test/libnotmuch-abi: compare exported symbols, available symbols, and linker script.
From: David Bremner
This uses objdump and awk to grab the available "notmuch_" symbols
from the object files and all exported symbols from libnotmuch.so. The
symbols from the linker script are grabbed using sed. All three of
these sets of symbols should be equal.
---
test/libnotmuch-abi | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/test/libnotmuch-abi b/test/libnotmuch-abi
index a7467b8..0e6192b 100755
--- a/test/libnotmuch-abi
+++ b/test/libnotmuch-abi
@@ -23,4 +23,14 @@ mkdir -p fakedb/.notmuch
test_expect_success 'running exception test' run_exception_test
test_begin_subtest 'checking output'
test_expect_equal "$result" "$output"
+
+objdump -t $TEST_DIRECTORY/../lib/*.o | awk '$4 == ".text" && $6 ~ "^notmuch"
{print $6}' | sort | uniq > all-symbols.txt
+
+test_begin_subtest 'checking linker script'
+sed -n 's/^\s*\(notmuch_.*\);/\1/p' $TEST_DIRECTORY/../notmuch.sym | sort>
script-symbols.txt
+test_expect_equal_file all-symbols.txt script-symbols.txt
+
+test_begin_subtest 'comparing exported symbols'
+objdump -T $TEST_DIRECTORY/../lib/libnotmuch.so | awk '$4 == ".text" {print
$7}' | sort | uniq > lib-symbols.txt
+test_expect_equal_file all-symbols.txt lib-symbols.txt
test_done
--
1.7.5.4
[PATCH 1/2] test/symbol-hiding: rename to test/libnotmuch-abi, prepare to add more subtests
From: David Bremner
Test descriptions changed to accomodate more subtests.
The generic 'run_test' is renamed 'run_exception_test',
symbol-test(.cc) is renamed exception-test(.cc).
---
I'm not sure if it would be better to start a new test script rather
than this fuss about renaming, but "symbol-test.cc" is probably to
generic as a name anyway. Arguably exception-test.cc is pretty generic
too.
test/basic |2 +-
test/exception-test.cc | 17 +
test/libnotmuch-abi| 26 ++
test/notmuch-test |2 +-
test/symbol-hiding | 26 --
test/symbol-test.cc| 17 -
6 files changed, 45 insertions(+), 45 deletions(-)
create mode 100644 test/exception-test.cc
create mode 100755 test/libnotmuch-abi
delete mode 100755 test/symbol-hiding
delete mode 100644 test/symbol-test.cc
diff --git a/test/basic b/test/basic
index 33bf711..9b8542f 100755
--- a/test/basic
+++ b/test/basic
@@ -56,7 +56,7 @@ tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
available=$(ls -1 $TEST_DIRECTORY/ | \
sed -r -e
"/^(aggregate-results.sh|Makefile|Makefile.local|notmuch-test)/d" \
-e
"/^(README|test-lib.sh|test-lib.el|test-results|tmp.*|valgrind|corpus*)/d" \
- -e
"/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose|symbol-test.cc)/d"
\
+ -e
"/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose|exception-test.cc)/d"
\
-e "/^(test.expected-output|.*~)/d" \
-e "/^(gnupg-secret-key.asc)/d" \
-e "/^(gnupg-secret-key.NOTE)/d" \
diff --git a/test/exception-test.cc b/test/exception-test.cc
new file mode 100644
index 000..1de06ea
--- /dev/null
+++ b/test/exception-test.cc
@@ -0,0 +1,17 @@
+#include
+#include
+#include
+main (int argc, char **argv){
+
+notmuch_database_t *notmuch
+ = notmuch_database_open ("fakedb",
+NOTMUCH_DATABASE_MODE_READ_ONLY);
+
+ try{
+(void)new Xapian::WritableDatabase ("./nonexistant",
Xapian::DB_OPEN);
+ } catch (const Xapian::Error &error) {
+printf("caught %s\n",error.get_msg().c_str());
+return 0;
+ }
+ return 1;
+}
diff --git a/test/libnotmuch-abi b/test/libnotmuch-abi
new file mode 100755
index 000..a7467b8
--- /dev/null
+++ b/test/libnotmuch-abi
@@ -0,0 +1,26 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2011 David Bremner
+#
+
+test_description='libnotmuch symbol export and hiding'
+
+# This subtest tests whether hiding Xapian::Error symbols in libnotmuch
+# also hides them for other users of libxapian. This is motivated by
+# the discussion in http://gcc.gnu.org/wiki/Visibility'
+
+. ./test-lib.sh
+
+run_exception_test(){
+result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./exception-test 2>&1)
+}
+
+output="A Xapian exception occurred opening database: Couldn't stat
'fakedb/.notmuch/xapian'
+caught No chert database found at path \`./nonexistant'"
+
+g++ -o exception-test -I$TEST_DIRECTORY/../lib
$TEST_DIRECTORY/exception-test.cc -L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
+mkdir -p fakedb/.notmuch
+test_expect_success 'running exception test' run_exception_test
+test_begin_subtest 'checking output'
+test_expect_equal "$result" "$output"
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index fe85c6a..c7296fa 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -40,7 +40,7 @@ TESTS="
emacs-large-search-buffer
maildir-sync
crypto
- symbol-hiding
+ libnotmuch-abi
"
TESTS=${NOTMUCH_TESTS:=$TESTS}
diff --git a/test/symbol-hiding b/test/symbol-hiding
deleted file mode 100755
index d0b31ae..000
--- a/test/symbol-hiding
+++ /dev/null
@@ -1,26 +0,0 @@
-#!/usr/bin/env bash
-#
-# Copyright (c) 2011 David Bremner
-#
-
-# This test tests whether hiding Xapian::Error symbols in libnotmuch
-# also hides them for other users of libxapian. This is motivated by
-# the discussion in http://gcc.gnu.org/wiki/Visibility'
-
-test_description='exception symbol hiding'
-
-. ./test-lib.sh
-
-run_test(){
-result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./symbol-test 2>&1)
-}
-
-output="A Xapian exception occurred opening database: Couldn't stat
'fakedb/.notmuch/xapian'
-caught No chert database found at path \`./nonexistant'"
-
-g++ -o symbol-test -I$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test.cc
-L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
-mkdir -p fakedb/.notmuch
-test_expect_success 'running test' run_test
-test_begin_subtest 'checking output'
-test_expect_equal "$result" "$output"
-test_done
diff --git a/test/symbol-test.cc b/test/symbol-test.cc
deleted file mode 100644
index 1de06ea..000
--- a/test/symbol-test.cc
+++ /dev/null
@@ -1,17 +0,0 @@
-#include
-#include
-#include
-main (int argc, char **argv){
-
-notmuch_database_t *notmuch
- = notmuch_database_open ("fakedb",
-NOTMUCH_DATABASE_MODE_READ_ONLY);
-
bug in emacs-ui ?
Dmitry,
On Wed, 29 Jun 2011 05:53:37 +0400, Dmitry Kurochkin wrote:
> On Tue, 28 Jun 2011 21:38:17 +0400, Dmitry Kurochkin gmail.com> wrote:
> > Hi Jani.
> >
> > On Tue, 28 Jun 2011 10:35:48 + (UTC), Jani Nikula
> > wrote:
> > > Carl Worth writes:
> > >
> > > >
> > > > On Wed, 22 Jun 2011 08:50:23 +0200, Sebastien Binet
> > > > wrote:
> > > > > On Tue, 21 Jun 2011 15:09:20 -0700, Carl Worth wrote:
> > > > > > Perhaps this is an emacs bug?
> > > > > could be, but,
> > > > >
> > > > > > I'm using Debian's emacs 23.3.1 and have not encountered the problem
> > > > > > described.
> > > > > I don't see anything related to 'point', 'point-invisible-p' nor
> > > 'forward-char' in there:
> > > > > http://patch-tracker.debian.org/package/emacs23/23.3+1-1
> > > >
> > > > Perhaps I was confused. I thought 23.3.1 was a distinct upstream release
> > > > From 23.3, but maybe not, (emacs version numbers use 1-based indexing
> > > > perhaps?).
> > > >
> > > > > anyways... for the moment I'll just live with it...
> > > >
> > > > Sorry about that.
> > > >
> > > > > except if somebody has some insights on how to "catch" the 'End of
> > > > > buffer' error and add some more protective programming around the
> > > > > 'scroll-up' function ?
> > > >
> > > > My emacs lisp skills are primarily restricted to actual
> > > > experimentation. So if I can replicate the bug, I might be able to find
> > > > a fix. I'm not very good at doing theoretical fixes with emacs lisp
> > > > code.
> > >
> > > I bisected the problem to commit 95ef8da29439f2e79115c36ab4d2a80aef1a1462
> > > ("Fix
> > > hiding a message while some citations are shown in notmuch-show view").
> > > Reverting that fixes the problem for me. (If you don't want to resolve
> > > conflicts, also revert 432e091924c1d1d8950a44ca78bc5b9c5ade47e4 first.) I
> > > don't
> > > have the time just now to try to figure out the problem in that, but
> > > perhaps
> > > this will help others go forward.
> > >
> >
> > Thanks for tracking this. I will look into it.
> >
>
> I have posted a patch to fix this bug [1].
thanks a lot for figuring this one out.
I'll give it a try!
-s
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL:
<http://notmuchmail.org/pipermail/notmuch/attachments/20110629/6c21858b/attachment.pgp>
[PATCH 1/3] test: `notmuch-show-advance-and-archive' with invisible signature
On Wed, 29 Jun 2011 09:10:19 +0400, Dmitry Kurochkin wrote: > On Wed, 29 Jun 2011 05:48:50 +0400, Dmitry Kurochkin gmail.com> wrote: > > This patch series fixes the bug reported by Sebastien in [1]. I > > was able to reproduce it and confirm that the second patch from > > this series fixes the problem. Unfortunately, I can not explain > > why it fixes it. The patch uses a cleaner approach for visible > > text search. But the old approach should work fine as well. > > Apparently, it does not work when `invisible' property is not a > > single symbol but a list (which was changed in > > 95ef8da29439f2e79115c36ab4d2a80aef1a1462). I suspect that it is > > an Emacs bug. I plan to look at it later. > > > > Turns out that `point-invisible-p' is a function from notmuch-lib.el, I > did not realize that before. It implements a custom visibility check > which is incomplete and does not work correctly when `invisible' > property is a list. That is why the previous code (which used > `point-invisible-p') had the bug. I sent another patch that removes > `point-invisible-p' function. > > > Another issue is that the test does not demonstrate the bug. > > Again, I do not really know why. It passes both before and after > > the fix. Although if I run the test commands by hand I hit the > > bug. I guess it has something to do with emacs daemon mode when > > the buffer is not visible. I hope someone with a better elisp > > knowledge can tell what is going on and how to make the test > > work. > > > > Now it is clear where the bug was. Remaining question is how to test > it. Hi, I applied the series, and I can confirm it fixes the bug. Hiding of messages also seems to work as expected, including the un-hidden signatures, which is what the commit that introduced this bug originally fixed. Many thanks. I have no insights on the automated tests, though. Jani
[PATCH] lib/Makefile.local: remove leftover debugging output.
From: David Bremner The removed "echo $(libnotmuch_modules)" was strictly for debugging. Thanks to Austin Clements for the hint. --- lib/Makefile.local |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/lib/Makefile.local b/lib/Makefile.local index 6dd095c..fe42920 100644 --- a/lib/Makefile.local +++ b/lib/Makefile.local @@ -72,7 +72,6 @@ $(dir)/libnotmuch.a: $(libnotmuch_modules) $(call quiet,AR) rcs $@ $^ $(dir)/$(LIBNAME): $(libnotmuch_modules) notmuch.sym - echo $(libnotmuch_modules) $(call quiet,CXX $(CXXFLAGS)) $(libnotmuch_modules) $(FINAL_LIBNOTMUCH_LDFLAGS) $(LIBRARY_LINK_FLAG) -o $@ notmuch.sym: lib/notmuch.h -- 1.7.5.4
Re: [PATCH 1/2] new: Don't update DB mtime if FS mtime equals wall-clock time.
On Wed, 29 Jun 2011 06:47:37 -0700, Carl Worth wrote: > On Wed, 29 Jun 2011 03:10:54 -0400, Austin Clements wrote: > > + * XXX Bug workaround: If this is a new directory, we *must* > > + * update the mtime; otherwise the next run will see the 0 mtime ... > I like to reserve "XXX" as an indication that some further work is > necessary. Reading your other mail now, I see that there are bugs here and that you do want to eliminate the new_directory optimization. That wasn't clear to me from the comment above. So the XXX is probably fine, but could perhaps give a little more indication of what could be done to eliminate the bug. -Carl -- carl.d.wo...@intel.com pgpB6Vz2RHlIz.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] new: Don't update DB mtime if FS mtime equals wall-clock time.
On Wed, 29 Jun 2011 06:47:37 -0700, Carl Worth wrote: > On Wed, 29 Jun 2011 03:10:54 -0400, Austin Clements > wrote: > > + * XXX Bug workaround: If this is a new directory, we *must* > > + * update the mtime; otherwise the next run will see the 0 mtime ... > I like to reserve "XXX" as an indication that some further work is > necessary. Reading your other mail now, I see that there are bugs here and that you do want to eliminate the new_directory optimization. That wasn't clear to me from the comment above. So the XXX is probably fine, but could perhaps give a little more indication of what could be done to eliminate the bug. -Carl -- carl.d.worth at intel.com -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/4f487e2a/attachment.pgp>
Re: [PATCH 10/10] test: use emacsclient(1) for Emacs tests
On Wed, 29 Jun 2011 03:34:58 -0400, Austin Clements wrote: > Or, really, any file system, since notmuch doesn't currently take > advantage of sub-second mtimes. Oh, ouch. I hadn't realized that I'd codified the 1-second granularity mtime into the notmuch API. Oh well, we could extend that with a floating-point API as needed. > > Austin has a plan to fix the use of the mtime timestamp in notmuch, > > (never storing the current mtime in the database if it's the same as > > the current time), that should hopefully close this race window. > > Posted. Thanks! -Carl -- carl.d.wo...@intel.com pgp6UiYe7cvAC.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 10/10] test: use emacsclient(1) for Emacs tests
On Wed, 29 Jun 2011 03:34:58 -0400, Austin Clements wrote: > Or, really, any file system, since notmuch doesn't currently take > advantage of sub-second mtimes. Oh, ouch. I hadn't realized that I'd codified the 1-second granularity mtime into the notmuch API. Oh well, we could extend that with a floating-point API as needed. > > Austin has a plan to fix the use of the mtime timestamp in notmuch, > > (never storing the current mtime in the database if it's the same as > > the current time), that should hopefully close this race window. > > Posted. Thanks! -Carl -- carl.d.worth at intel.com -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/1e9ff4e7/attachment.pgp>
Re: [PATCH 1/2] new: Don't update DB mtime if FS mtime equals wall-clock time.
On Wed, 29 Jun 2011 03:10:54 -0400, Austin Clements wrote: > This fixes a race where multiple message deliveries in the same second > with an intervening notmuch new could result in messages being ignored > by notmuch (at least, until a later delivery forced a rescan). Thanks for the fix, Austin! > + * XXX Bug workaround: If this is a new directory, we *must* > + * update the mtime; otherwise the next run will see the 0 mtime > + * and think this is still a new directory containing no files or > + * subdirs (which is unsound in general). If fs_mtime == > + * stat_time, we set the database mtime to a bogus (but non-zero!) > + * value to force a rescan. I like to reserve "XXX" as an indication that some further work is necessary. That doesn't seem to be the case here, (instead, you seem to have thought this issue out quite fully). Other than that, I'm quite happy with the patch. -Carl -- carl.d.wo...@intel.com pgpkBjAvVzMIY.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] new: Don't update DB mtime if FS mtime equals wall-clock time.
On Wed, 29 Jun 2011 03:10:54 -0400, Austin Clements wrote: > This fixes a race where multiple message deliveries in the same second > with an intervening notmuch new could result in messages being ignored > by notmuch (at least, until a later delivery forced a rescan). Thanks for the fix, Austin! > + * XXX Bug workaround: If this is a new directory, we *must* > + * update the mtime; otherwise the next run will see the 0 mtime > + * and think this is still a new directory containing no files or > + * subdirs (which is unsound in general). If fs_mtime == > + * stat_time, we set the database mtime to a bogus (but non-zero!) > + * value to force a rescan. I like to reserve "XXX" as an indication that some further work is necessary. That doesn't seem to be the case here, (instead, you seem to have thought this issue out quite fully). Other than that, I'm quite happy with the patch. -Carl -- carl.d.worth at intel.com -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110629/6bc21b68/attachment.pgp>
bug in emacs-ui ?
On Tue, 28 Jun 2011 21:38:17 +0400, Dmitry Kurochkin wrote:
> Hi Jani.
>
> On Tue, 28 Jun 2011 10:35:48 + (UTC), Jani Nikula
> wrote:
> > Carl Worth writes:
> >
> > >
> > > On Wed, 22 Jun 2011 08:50:23 +0200, Sebastien Binet
> > > wrote:
> > > > On Tue, 21 Jun 2011 15:09:20 -0700, Carl Worth wrote:
> > > > > Perhaps this is an emacs bug?
> > > > could be, but,
> > > >
> > > > > I'm using Debian's emacs 23.3.1 and have not encountered the problem
> > > > > described.
> > > > I don't see anything related to 'point', 'point-invisible-p' nor
> > 'forward-char' in there:
> > > > http://patch-tracker.debian.org/package/emacs23/23.3+1-1
> > >
> > > Perhaps I was confused. I thought 23.3.1 was a distinct upstream release
> > > From 23.3, but maybe not, (emacs version numbers use 1-based indexing
> > > perhaps?).
> > >
> > > > anyways... for the moment I'll just live with it...
> > >
> > > Sorry about that.
> > >
> > > > except if somebody has some insights on how to "catch" the 'End of
> > > > buffer' error and add some more protective programming around the
> > > > 'scroll-up' function ?
> > >
> > > My emacs lisp skills are primarily restricted to actual
> > > experimentation. So if I can replicate the bug, I might be able to find
> > > a fix. I'm not very good at doing theoretical fixes with emacs lisp
> > > code.
> >
> > I bisected the problem to commit 95ef8da29439f2e79115c36ab4d2a80aef1a1462
> > ("Fix
> > hiding a message while some citations are shown in notmuch-show view").
> > Reverting that fixes the problem for me. (If you don't want to resolve
> > conflicts, also revert 432e091924c1d1d8950a44ca78bc5b9c5ade47e4 first.) I
> > don't
> > have the time just now to try to figure out the problem in that, but perhaps
> > this will help others go forward.
> >
>
> Thanks for tracking this. I will look into it.
>
I have posted a patch to fix this bug [1].
Regards,
Dmitry
[1] id:"1309312132-14564-2-git-send-email-dmitry.kurochkin at gmail.com"
> Regards,
> Dmitry
>
> > Jani
> >
> >
> > ___
> > notmuch mailing list
> > notmuch at notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/3] emacs: remove no longer used functions from notmuch-show.el
Remove `notmuch-show-move-past-invisible-backward' and `notmuch-show-move-past-invisible-forward' functions which are unused. --- emacs/notmuch-show.el |8 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index ad3cc7b..dcaea65 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -977,14 +977,6 @@ All currently available key bindings: (notmuch-show-move-to-message-top) t)) -(defun notmuch-show-move-past-invisible-forward () - (while (point-invisible-p) -(forward-char))) - -(defun notmuch-show-move-past-invisible-backward () - (while (point-invisible-p) -(backward-char))) - ;; Functions relating to the visibility of messages and their ;; components. -- 1.7.5.4
[PATCH 2/3] emacs: improve hidden signatures handling in notmuch-show-advance-and-archive
Use `previous-single-char-property-change' instead of going through each character by hand and testing it's visibility. This fixes `notmuch-show-advance-and-archive' to work for the last message in thread with hidden signature. --- emacs/notmuch-show.el | 17 + 1 files changed, 9 insertions(+), 8 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 6685717..ad3cc7b 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -1113,17 +1113,18 @@ thread, (remove the \"inbox\" tag from each message). Also kill this buffer, and display the next thread from the search from which this thread was originally shown." (interactive) - (let ((end-of-this-message (notmuch-show-message-bottom))) + (let* ((end-of-this-message (notmuch-show-message-bottom)) +(visible-end-of-this-message (1- end-of-this-message))) +(while (invisible-p visible-end-of-this-message) + (setq visible-end-of-this-message + (previous-single-char-property-change visible-end-of-this-message + 'invisible))) (cond ;; Ideally we would test `end-of-this-message' against the result ;; of `window-end', but that doesn't account for the fact that - ;; the end of the message might be hidden, so we have to actually - ;; go to the end, walk back over invisible text and then see if - ;; point is visible. - ((save-excursion - (goto-char (- end-of-this-message 1)) - (notmuch-show-move-past-invisible-backward) - (> (point) (window-end))) + ;; the end of the message might be hidden. + ((and visible-end-of-this-message + (> visible-end-of-this-message (window-end))) ;; The bottom of this message is not visible - scroll. (scroll-up nil)) -- 1.7.5.4
[PATCH 1/3] test: `notmuch-show-advance-and-archive' with invisible signature
Add Emacs test to check that `notmuch-show-advance-and-archive' works for the last message in thread with invisible signature. --- This patch series fixes the bug reported by Sebastien in [1]. I was able to reproduce it and confirm that the second patch from this series fixes the problem. Unfortunately, I can not explain why it fixes it. The patch uses a cleaner approach for visible text search. But the old approach should work fine as well. Apparently, it does not work when `invisible' property is not a single symbol but a list (which was changed in 95ef8da29439f2e79115c36ab4d2a80aef1a1462). I suspect that it is an Emacs bug. I plan to look at it later. Another issue is that the test does not demonstrate the bug. Again, I do not really know why. It passes both before and after the fix. Although if I run the test commands by hand I hit the bug. I guess it has something to do with emacs daemon mode when the buffer is not visible. I hope someone with a better elisp knowledge can tell what is going on and how to make the test work. I believe patches 2 and 3 can be pushed after review even without a working test. Regards, Dmitry [1] id:"8739j5rn2d.fsf at cern.ch" test/emacs | 12 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/test/emacs b/test/emacs index e59de47..65a96a5 100755 --- a/test/emacs +++ b/test/emacs @@ -347,4 +347,16 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae at mail. (test-visible-output)' test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages +test_begin_subtest 'notmuch-show-advance-and-archive with invisible signature' +message1='id:20091118010116.GC25380 at dottiness.seas.harvard.edu' +message2='id:1258491078-29658-1-git-send-email-dottedmag at dottedmag.net' +test_emacs "(notmuch-search \"$message1 or $message2\") + (notmuch-test-wait) + (notmuch-search-show-thread) + (notmuch-show-advance-and-archive) + (test-output)" +test_emacs "(notmuch-show \"$message2\") + (test-output \"EXPECTED\")" +test_expect_equal_file OUTPUT EXPECTED + test_done -- 1.7.5.4
[PATCH 2/2] test/libnotmuch-abi: compare exported symbols, available symbols, and linker script.
From: David Bremner
This uses objdump and awk to grab the available "notmuch_" symbols
from the object files and all exported symbols from libnotmuch.so. The
symbols from the linker script are grabbed using sed. All three of
these sets of symbols should be equal.
---
test/libnotmuch-abi | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/test/libnotmuch-abi b/test/libnotmuch-abi
index a7467b8..0e6192b 100755
--- a/test/libnotmuch-abi
+++ b/test/libnotmuch-abi
@@ -23,4 +23,14 @@ mkdir -p fakedb/.notmuch
test_expect_success 'running exception test' run_exception_test
test_begin_subtest 'checking output'
test_expect_equal "$result" "$output"
+
+objdump -t $TEST_DIRECTORY/../lib/*.o | awk '$4 == ".text" && $6 ~ "^notmuch"
{print $6}' | sort | uniq > all-symbols.txt
+
+test_begin_subtest 'checking linker script'
+sed -n 's/^\s*\(notmuch_.*\);/\1/p' $TEST_DIRECTORY/../notmuch.sym | sort>
script-symbols.txt
+test_expect_equal_file all-symbols.txt script-symbols.txt
+
+test_begin_subtest 'comparing exported symbols'
+objdump -T $TEST_DIRECTORY/../lib/libnotmuch.so | awk '$4 == ".text" {print
$7}' | sort | uniq > lib-symbols.txt
+test_expect_equal_file all-symbols.txt lib-symbols.txt
test_done
--
1.7.5.4
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] test/symbol-hiding: rename to test/libnotmuch-abi, prepare to add more subtests
From: David Bremner
Test descriptions changed to accomodate more subtests.
The generic 'run_test' is renamed 'run_exception_test',
symbol-test(.cc) is renamed exception-test(.cc).
---
I'm not sure if it would be better to start a new test script rather
than this fuss about renaming, but "symbol-test.cc" is probably to
generic as a name anyway. Arguably exception-test.cc is pretty generic
too.
test/basic |2 +-
test/exception-test.cc | 17 +
test/libnotmuch-abi| 26 ++
test/notmuch-test |2 +-
test/symbol-hiding | 26 --
test/symbol-test.cc| 17 -
6 files changed, 45 insertions(+), 45 deletions(-)
create mode 100644 test/exception-test.cc
create mode 100755 test/libnotmuch-abi
delete mode 100755 test/symbol-hiding
delete mode 100644 test/symbol-test.cc
diff --git a/test/basic b/test/basic
index 33bf711..9b8542f 100755
--- a/test/basic
+++ b/test/basic
@@ -56,7 +56,7 @@ tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
available=$(ls -1 $TEST_DIRECTORY/ | \
sed -r -e
"/^(aggregate-results.sh|Makefile|Makefile.local|notmuch-test)/d" \
-e
"/^(README|test-lib.sh|test-lib.el|test-results|tmp.*|valgrind|corpus*)/d" \
- -e
"/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose|symbol-test.cc)/d"
\
+ -e
"/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose|exception-test.cc)/d"
\
-e "/^(test.expected-output|.*~)/d" \
-e "/^(gnupg-secret-key.asc)/d" \
-e "/^(gnupg-secret-key.NOTE)/d" \
diff --git a/test/exception-test.cc b/test/exception-test.cc
new file mode 100644
index 000..1de06ea
--- /dev/null
+++ b/test/exception-test.cc
@@ -0,0 +1,17 @@
+#include
+#include
+#include
+main (int argc, char **argv){
+
+notmuch_database_t *notmuch
+ = notmuch_database_open ("fakedb",
+NOTMUCH_DATABASE_MODE_READ_ONLY);
+
+ try{
+(void)new Xapian::WritableDatabase ("./nonexistant",
Xapian::DB_OPEN);
+ } catch (const Xapian::Error &error) {
+printf("caught %s\n",error.get_msg().c_str());
+return 0;
+ }
+ return 1;
+}
diff --git a/test/libnotmuch-abi b/test/libnotmuch-abi
new file mode 100755
index 000..a7467b8
--- /dev/null
+++ b/test/libnotmuch-abi
@@ -0,0 +1,26 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2011 David Bremner
+#
+
+test_description='libnotmuch symbol export and hiding'
+
+# This subtest tests whether hiding Xapian::Error symbols in libnotmuch
+# also hides them for other users of libxapian. This is motivated by
+# the discussion in http://gcc.gnu.org/wiki/Visibility'
+
+. ./test-lib.sh
+
+run_exception_test(){
+result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./exception-test 2>&1)
+}
+
+output="A Xapian exception occurred opening database: Couldn't stat
'fakedb/.notmuch/xapian'
+caught No chert database found at path \`./nonexistant'"
+
+g++ -o exception-test -I$TEST_DIRECTORY/../lib
$TEST_DIRECTORY/exception-test.cc -L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
+mkdir -p fakedb/.notmuch
+test_expect_success 'running exception test' run_exception_test
+test_begin_subtest 'checking output'
+test_expect_equal "$result" "$output"
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index fe85c6a..c7296fa 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -40,7 +40,7 @@ TESTS="
emacs-large-search-buffer
maildir-sync
crypto
- symbol-hiding
+ libnotmuch-abi
"
TESTS=${NOTMUCH_TESTS:=$TESTS}
diff --git a/test/symbol-hiding b/test/symbol-hiding
deleted file mode 100755
index d0b31ae..000
--- a/test/symbol-hiding
+++ /dev/null
@@ -1,26 +0,0 @@
-#!/usr/bin/env bash
-#
-# Copyright (c) 2011 David Bremner
-#
-
-# This test tests whether hiding Xapian::Error symbols in libnotmuch
-# also hides them for other users of libxapian. This is motivated by
-# the discussion in http://gcc.gnu.org/wiki/Visibility'
-
-test_description='exception symbol hiding'
-
-. ./test-lib.sh
-
-run_test(){
-result=$(LD_LIBRARY_PATH=$TEST_DIRECTORY/../lib ./symbol-test 2>&1)
-}
-
-output="A Xapian exception occurred opening database: Couldn't stat
'fakedb/.notmuch/xapian'
-caught No chert database found at path \`./nonexistant'"
-
-g++ -o symbol-test -I$TEST_DIRECTORY/../lib $TEST_DIRECTORY/symbol-test.cc
-L$TEST_DIRECTORY/../lib -lnotmuch -lxapian
-mkdir -p fakedb/.notmuch
-test_expect_success 'running test' run_test
-test_begin_subtest 'checking output'
-test_expect_equal "$result" "$output"
-test_done
diff --git a/test/symbol-test.cc b/test/symbol-test.cc
deleted file mode 100644
index 1de06ea..000
--- a/test/symbol-test.cc
+++ /dev/null
@@ -1,17 +0,0 @@
-#include
-#include
-#include
-main (int argc, char **argv){
-
-notmuch_database_t *notmuch
- = notmuch_database_open ("fakedb",
-NOTMUCH_DATABASE_MODE_READ_ONLY);
[PATCH] lib/Makefile.local: remove leftover debugging output.
From: David Bremner The removed "echo $(libnotmuch_modules)" was strictly for debugging. Thanks to Austin Clements for the hint. --- lib/Makefile.local |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/lib/Makefile.local b/lib/Makefile.local index 6dd095c..fe42920 100644 --- a/lib/Makefile.local +++ b/lib/Makefile.local @@ -72,7 +72,6 @@ $(dir)/libnotmuch.a: $(libnotmuch_modules) $(call quiet,AR) rcs $@ $^ $(dir)/$(LIBNAME): $(libnotmuch_modules) notmuch.sym - echo $(libnotmuch_modules) $(call quiet,CXX $(CXXFLAGS)) $(libnotmuch_modules) $(FINAL_LIBNOTMUCH_LDFLAGS) $(LIBRARY_LINK_FLAG) -o $@ notmuch.sym: lib/notmuch.h -- 1.7.5.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 10/10] test: use emacsclient(1) for Emacs tests
On Tue, Jun 28, 2011 at 8:15 PM, Carl Worth wrote: > That means that if the emacs_deliver_message code is fast enough, (less > than 1 second), and if the test suite is being run on a lame filesystem > without sub-second timestamp granularity (including ext3fs) that the > "notmuch new" invocation will not see the mail[*]. Or, really, any file system, since notmuch doesn't currently take advantage of sub-second mtimes. > [*] And, yes, this does mean that there are race conditions under which > mail can be delivered and notmuch won't see it, (unless subsequent mails > are delivered to the same directory). Austin has a plan to fix the use > of the mtime timestamp in notmuch, (never storing the current mtime in > the database if it's the same as the current time), that should > hopefully close this race window. Posted.
[PATCH 2/2] test: Nix increment_mtime.
With the fix for the mtime race, this workaround is no longer
necessary.
---
test/maildir-sync |8
test/multipart|1 -
test/new |9 -
test/search-by-folder |2 --
test/test-lib.sh | 15 ---
5 files changed, 0 insertions(+), 35 deletions(-)
diff --git a/test/maildir-sync b/test/maildir-sync
index c99dbec..a60854f 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -23,7 +23,6 @@ output=$(notmuch search subject:"Adding S flag" |
notmuch_search_sanitize)
output+="
"
mv "${gen_msg_filename}" "${gen_msg_filename}S"
-increment_mtime "$(dirname "${gen_msg_filename}")"
output+=$(NOTMUCH_NEW)
output+="
"
@@ -66,7 +65,6 @@ test_expect_equal "$output" '[[[{"id": "adding-replied-tag at
notmuch-test-suite",
test_expect_success 'notmuch reply works with renamed file (without notmuch
new)' 'notmuch reply id:${gen_msg_id}'
test_begin_subtest "notmuch new detects no file rename after tag->flag
synchronization"
-increment_mtime "$(dirname ${gen_msg_filename})"
output=$(NOTMUCH_NEW)
test_expect_equal "$output" "No new mail."
@@ -77,7 +75,6 @@ output=$(cd "$MAIL_DIR/cur"; ls message-to-move*)
test_expect_equal "$output" "message-to-move-to-cur:2,S"
test_begin_subtest "No rename should be detected by notmuch new"
-increment_mtime "$MAIL_DIR/cur"
output=$(NOTMUCH_NEW)
test_expect_equal "$output" "No new mail."
# (*) If notmuch new was not run we've got "Processed 1 file in almost
@@ -97,7 +94,6 @@ output=$(notmuch search subject:"Removing S flag" |
notmuch_search_sanitize)
output+="
"
mv "${gen_msg_filename}" "${gen_msg_filename%S}"
-increment_mtime "$(dirname "${gen_msg_filename}")"
output+=$(NOTMUCH_NEW)
output+="
"
@@ -110,7 +106,6 @@ test_begin_subtest "Removing info from filename leaves tags
unchanged"
add_message [subject]='"Message to lose maildir info"'
[filename]='message-to-lose-maildir-info' [dir]=cur
notmuch tag -unread subject:"Message to lose maildir info"
mv "$MAIL_DIR/cur/message-to-lose-maildir-info:2,S"
"$MAIL_DIR/cur/message-without-maildir-info"
-increment_mtime "$MAIL_DIR/cur"
output=$(NOTMUCH_NEW)
output+="
"
@@ -134,7 +129,6 @@ mv $MAIL_DIR/cur/adding-replied-tag:2,RS
$MAIL_DIR/cur/adding-replied-tag:2,S
mv $MAIL_DIR/cur/adding-s-flag:2,S $MAIL_DIR/cur/adding-s-flag:2,
mv $MAIL_DIR/cur/adding-with-s-flag:2,S $MAIL_DIR/cur/adding-with-s-flag:2,RS
mv $MAIL_DIR/cur/message-to-move-to-cur:2,S
$MAIL_DIR/cur/message-to-move-to-cur:2,DS
-increment_mtime $MAIL_DIR/cur
notmuch dump dump.txt
NOTMUCH_NEW >/dev/null
notmuch restore dump.txt
@@ -144,7 +138,6 @@ test_expect_equal "$output" "$expected"
test_begin_subtest 'Adding flags to duplicate message tags the mail'
add_message [subject]='"Duplicated message"' [dir]=cur
[filename]='duplicated-message:2,'
cp "$MAIL_DIR/cur/duplicated-message:2,"
"$MAIL_DIR/cur/duplicated-message-copy:2,RS"
-increment_mtime $MAIL_DIR/cur
NOTMUCH_NEW > output
notmuch search subject:"Duplicated message" | notmuch_search_sanitize >> output
test_expect_equal "$(< output)" "No new mail.
@@ -152,7 +145,6 @@ thread:XXX 2001-01-05 [1/1] Notmuch Test Suite;
Duplicated message (inbox repl
test_begin_subtest "Adding duplicate message without flags does not remove
tags"
cp "$MAIL_DIR/cur/duplicated-message-copy:2,RS"
"$MAIL_DIR/cur/duplicated-message-another-copy:2,"
-increment_mtime $MAIL_DIR/cur
NOTMUCH_NEW > output
notmuch search subject:"Duplicated message" | notmuch_search_sanitize >> output
test_expect_equal "$(< output)" "No new mail.
diff --git a/test/multipart b/test/multipart
index 4d577f8..22c42c6 100755
--- a/test/multipart
+++ b/test/multipart
@@ -88,7 +88,6 @@ Content-Transfer-Encoding: base64
7w0K
--==-=-=--
EOF
-increment_mtime "$MAIL_DIR"
notmuch new > /dev/null
test_begin_subtest "--format=text --part=0, full message"
diff --git a/test/new b/test/new
index 1b7296e..49f390d 100755
--- a/test/new
+++ b/test/new
@@ -52,10 +52,8 @@ generate_message
tmp_msg_filename=tmp/"$gen_msg_filename"
mkdir -p "$(dirname "$tmp_msg_filename")"
mv "$gen_msg_filename" "$tmp_msg_filename"
-increment_mtime "${MAIL_DIR}"
notmuch new > /dev/null
mv "$tmp_msg_filename" "$gen_msg_filename"
-increment_mtime "${MAIL_DIR}"
output=$(NOTMUCH_NEW)
test_expect_equal "$output" "Added 1 new message to the database."
@@ -65,7 +63,6 @@ test_begin_subtest "Renamed message"
generate_message
notmuch new > /dev/null
mv "$gen_msg_filename" "${gen_msg_filename}"-renamed
-increment_mtime "${MAIL_DIR}"
output=$(NOTMUCH_NEW)
test_expect_equal "$output" "No new mail. Detected 1 file rename."
@@ -73,7 +70,6 @@ test_expect_equal "$output" "No new mail. Detected 1 file
rename."
test_begin_subtest "Deleted message"
rm "${gen_msg_filename}"-renamed
-increment_mtime "${MAIL_DIR}"
output=$(NOTMUCH_NEW)
test_expect_equal "$output" "No new mail. Removed 1 message."
@@ -87,7 +83,6 @@ generate_message [dir]=dir
notmuch new > /dev/null
mv "${MAIL_DIR}"/dir "${
[PATCH 1/2] new: Don't update DB mtime if FS mtime equals wall-clock time.
This fixes a race where multiple message deliveries in the same second
with an intervening notmuch new could result in messages being ignored
by notmuch (at least, until a later delivery forced a rescan).
Because mtimes only have second granularity, later deliveries in the
same second won't change the directory mtime, and hence won't trigger
notmuch new to rescan the directory. This situation can only occur
when notmuch new is being run at the same second as the directory's
modification time, so simply don't update the saved mtime in this
case.
This very race happens all over the test suite, and is currently
compensated for with increment_mtime (and, occasionally, luck). With
this change, increment_mtime becomes unnecessary.
---
notmuch-new.c | 20 +++-
1 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/notmuch-new.c b/notmuch-new.c
index 0fa2a3c..c180591 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -213,6 +213,7 @@ _entries_resemble_maildir (struct dirent **entries, int
count)
* information is lost from the database).
*
* o Tell the database to update its time of 'path' to 'fs_mtime'
+ * if fs_mtime isn't the current wall-clock time.
*/
static notmuch_status_t
add_files_recursive (notmuch_database_t *notmuch,
@@ -230,6 +231,7 @@ add_files_recursive (notmuch_database_t *notmuch,
notmuch_directory_t *directory;
notmuch_filenames_t *db_files = NULL;
notmuch_filenames_t *db_subdirs = NULL;
+time_t stat_time;
struct stat st;
notmuch_bool_t is_maildir, new_directory;
const char **tag;
@@ -239,6 +241,7 @@ add_files_recursive (notmuch_database_t *notmuch,
path, strerror (errno));
return NOTMUCH_STATUS_FILE_ERROR;
}
+stat_time = time (NULL);
/* This is not an error since we may have recursed based on a
* symlink to a regular file, not a directory, and we don't know
@@ -509,7 +512,22 @@ add_files_recursive (notmuch_database_t *notmuch,
notmuch_filenames_move_to_next (db_subdirs);
}
-if (! interrupted) {
+/* If the directory's mtime is the same as the wall-clock time
+ * when we stat'ed the directory, we skip updating the mtime in
+ * the database because a message could be delivered later in this
+ * same second. This may lead to unnecessary re-scans, but it
+ * avoids overlooking messages.
+ *
+ * XXX Bug workaround: If this is a new directory, we *must*
+ * update the mtime; otherwise the next run will see the 0 mtime
+ * and think this is still a new directory containing no files or
+ * subdirs (which is unsound in general). If fs_mtime ==
+ * stat_time, we set the database mtime to a bogus (but non-zero!)
+ * value to force a rescan.
+ */
+if (new_directory && fs_mtime == stat_time)
+ fs_mtime = 1;
+if (! interrupted && fs_mtime != stat_time) {
status = notmuch_directory_set_mtime (directory, fs_mtime);
if (status && ret == NOTMUCH_STATUS_SUCCESS)
ret = status;
--
1.7.5.1
[PATCH v6 00/17] Fix 'notmuch new' atomicity issues
Just found one more atomicity bug in notmuch-new. I would prefer to not update this patch series yet again, but rather to follow up with a separate fix for this. The full bug is described below, but the gist is that how add_files_recursive computes new_directory from the mtime isn't sound. The simplest fix is to remove the new_directory optimization, but then we wouldn't scan files in inode order. The best fix is probably to add an out-argument to notmuch_database_get_directory that indicates if the directory really was just created in the database (and hence has no files or subdirs). Unfortunately, that requires an API change. If that's undesirable, an alternate would be to record that bit in the notmuch_directory_t and add some notmuch_directory_is_new API that returns it. Thoughts? Bug description: add_files_recursive determines whether a directory is "new" by inspecting the database mtime returned by notmuch_directory_get_mtime. However, if there's an interruption after adding messages/subdirectories from a new directory but before updating the directory's mtime, it will be considered a new directory again on the next run. As a result, on the next run, db_files and db_subdirs will not be loaded from the database (even though they *wouldn't* be NULL). As a result, we'll miss removing messages or entire subdirectories that have been deleted from the "new" directory. (We'll also re-add the messages in this directory to the database rather than skipping them, which will throw off the notmuch new statistics, but that's fine.) This didn't show up in the atomicity test because throwing off anything besides the statistics requires *two* interruptions. In fact, I don't even know how I would write an automated test for this.
[PATCH 10/10] test: use emacsclient(1) for Emacs tests
On Tue, 28 Jun 2011 15:14:04 -0700, Carl Worth wrote: > On Tue, 28 Jun 2011 08:45:11 +0400, Dmitry Kurochkin gmail.com> wrote: > > Before the change, every Emacs test ran in a separate Emacs > > instance. Starting Emacs many times wastes considerable time and > > it gets worse as the test suite grows. The patch solves this by > > using a single Emacs server and emacsclient(1) to run multiple > > tests. > > Great, great stuff, Dmitry! > > I've pushed everything earlier than this patch in this series. > > And I'd be fine pushing this one as well, (if Austin cares strongly > about not polling, I'll invite him to improve things from here). > > Except that things don't actually work for me with this patch applied. > > I'm not getting consistent results from the test suite, (I have seen > both "2 tests failed" and "5 tests failed"). Here are the failures from > a recent run with 5 failures: > > FAIL decryption, --format=text > FAIL decryption, --format=json > FAIL decryption, --format=json, --part=4 > FAIL decrypt attachment (--part=5 --format=raw) > FAIL decryption failure with missing key > > In each case the actual output was either empty, or an empty JSON > array. In some cases notmuch also gave an error: > > Error: search term did not match precisely one message. > > Are you not getting these same failures? > Ouch. I never saw these. Do you get these in crypto tests only? Regards, Dmitry > Let me know what else I can do to investigate this. > > -Carl > > -- > carl.d.worth at intel.com
Re: [PATCH 1/3] test: `notmuch-show-advance-and-archive' with invisible signature
On Wed, 29 Jun 2011 09:10:19 +0400, Dmitry Kurochkin wrote: > On Wed, 29 Jun 2011 05:48:50 +0400, Dmitry Kurochkin > wrote: > > This patch series fixes the bug reported by Sebastien in [1]. I > > was able to reproduce it and confirm that the second patch from > > this series fixes the problem. Unfortunately, I can not explain > > why it fixes it. The patch uses a cleaner approach for visible > > text search. But the old approach should work fine as well. > > Apparently, it does not work when `invisible' property is not a > > single symbol but a list (which was changed in > > 95ef8da29439f2e79115c36ab4d2a80aef1a1462). I suspect that it is > > an Emacs bug. I plan to look at it later. > > > > Turns out that `point-invisible-p' is a function from notmuch-lib.el, I > did not realize that before. It implements a custom visibility check > which is incomplete and does not work correctly when `invisible' > property is a list. That is why the previous code (which used > `point-invisible-p') had the bug. I sent another patch that removes > `point-invisible-p' function. > > > Another issue is that the test does not demonstrate the bug. > > Again, I do not really know why. It passes both before and after > > the fix. Although if I run the test commands by hand I hit the > > bug. I guess it has something to do with emacs daemon mode when > > the buffer is not visible. I hope someone with a better elisp > > knowledge can tell what is going on and how to make the test > > work. > > > > Now it is clear where the bug was. Remaining question is how to test > it. Hi, I applied the series, and I can confirm it fixes the bug. Hiding of messages also seems to work as expected, including the un-hidden signatures, which is what the commit that introduced this bug originally fixed. Many thanks. I have no insights on the automated tests, though. Jani ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
