[PATCH] emacs: show: make id links respect window

2012-12-19 Thread Mark Walters
There is a bug in current notmuch: if you have multiple windows in one
frame and click an id button link in a show buffer that does not
contain point then the link is opened in the window containing point.

This reads the mouse event to make sure that the correct window is
used for the link.
---

I think this is a bug but that could be debated. It is particularly
easy to trigger with notmuch pick because that uses the split pane
while focus usually remains in the `pick' pane rather than the `show'
pane.

The lisp is not pretty but seems to work.

Best wishes

Mark




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

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5751d98..5664ea3 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1077,6 +1077,11 @@ buttons for a corresponding notmuch search."
(make-text-button (first link) (second link)
  'action `(lambda (arg)
 (notmuch-show ,(third link)))
+ 'mouse-action `(lambda (arg)
+  (let* ((event last-input-event)
+ (window (car (cadr event
+(select-window window)
+(notmuch-show ,(third link
  'follow-link t
  'help-echo "Mouse-1, RET: search for this message"
  'face goto-address-mail-face)
-- 
1.7.9.1



[PATCH] test: Adding non-maildir tags does not move message from new to cur

2012-12-19 Thread Michal Sojka
Hello Jani,

On Sat, Dec 08 2012, Jani Nikula wrote:
> On Sat, 08 Dec 2012, david at tethera.net wrote:
>> From: Michal Sojka 
>>
>> Some MUA's like mutt show the difference between "new" emails living in 
>> maildir
>> directory new/, and "old" emails living in maildir directory cur/. However
>> notmuch tag unconditionally moves selected messages from new/ to cur/, even 
>> if
>> no maildir synchronized tag is changed.
>>
>> While maildir specification forbids messages with tags living in new/, there 
>> is
>> no need to move messages to cur/ when no maildir synchronized tag is changed.
>> Thus notmuch can remain transparent with respect to other MUA's.
>>
>> [ Edited commit log to better describe the intended changes, and tag the
>>   test as broken until the actual changes are implemented -- Louis Rilling ]
>>
>> Signed-off-by: Louis Rilling 
>>
>> [ Converted to use test_subtest_known_broken, David Bremner ]
>> ---
>>
>> Do we agree that the behaviour of moving messages to ./cur on tagging
>> is broken? If so, maybe it's worth tidying up and applying this.  The
>> use of cd and ls strikes me as slightly suspect, but I welcome other
>> opinions.
>
> I think I would narrow down the special case a bit: I think messages in
> ./new that have no maildir flags, and have no ":2," in the end of the
> filename, and and the tag change(s) will not affect maildir flags,
> should stay in ./new. Files in ./new should not have ":2," or maildir
> flags, and I see no reason to support having them there.
>
> Thus any messages in ./new that do have maildir flags, or have ":2," in
> the end of the filename should probably be moved to ./cur, even if the
> tag change(s) do not affect maildir flags. The patch in this thread
> fails here. It also changes the behaviour for messages in ./cur by not
> appending ":2," to them.

I agree with you. In
id:1355952747-27350-1-git-send-email-sojkam1 at fel.cvut.cz I sent the
tests for the cases descried above as well as the updated patch for tag
to maildir synchronization.

> As to the test, I think it should do something along the lines of (based
> on search-output test):
>
> notmuch search --output=files subject:"Message to stay in new" | sed -e 
> "s,$MAIL_DIR,MAIL_DIR," >OUTPUT
> cat  MAIL_DIR/new/message-to-stay-in-new
> EOF
> test_expect_equal_file OUTPUT EXPECTED

With this you test what notmuch thinks about the file names of messages,
not whether the files have actually been renamed. For this reason I kept
the previous way of testing in the new patches.

Cheers,
-Michal


[PATCH 3/3] tags_to_maildir_flags: Don't rename if no flags change

2012-12-19 Thread Michal Sojka
From: Louis Rilling 

notmuch_message_tags_to_maildir_flags() unconditionally moves messages from
maildir directory "new/" to maildir directory "cur/", which makes messages lose
their "new" status in the MUA. However some users want to keep this "new"
status after, for instance, an auto-tagging of new messages.

However, as Austin mentioned and according to the maildir specification,
messages living in "new/" are not allowed to have flags, even if mutt allows it
to happen. For this reason, this patch prevents moving messages from "new/" to
"cur/", only if no flags have to be changed. It's hopefully enough to satisfy
mutt (and maybe other MUAs showing the "new" status) users checking the "new"
status.

Changelog:
* v2: Fix bool type as well as NULL returned despite having no errors (Austin
  Clements)
* v4: Tag the related test (contributed by Michal Sojka) as working

Signed-off-by: Louis Rilling 

[Condition for keeping messages in new/ was extended to satisfy all
 tests from the previous patch. -Michal Sojka]
---
 lib/message.cc|   13 -
 test/maildir-sync |1 -
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 320901f..87369bb 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1195,7 +1195,9 @@ _get_maildir_flag_actions (notmuch_message_t *message,
  * compute the new maildir filename.
  *
  * If the existing filename is in the directory "new", the new
- * filename will be in the directory "cur".
+ * filename will be in the directory "cur", except for the case when
+ * no flags are changed and the existing filename does not contain
+ * maildir info (starting with ",2:").
  *
  * After a sequence of ":2," in the filename, any subsequent
  * single-character flags will be added or removed according to the
@@ -1218,6 +1220,7 @@ _new_maildir_filename (void *ctx,
 char *filename_new, *dir;
 char flag_map[128];
 int flags_in_map = 0;
+notmuch_bool_t flags_changed = FALSE;
 unsigned int i;
 char *s;

@@ -1258,6 +1261,7 @@ _new_maildir_filename (void *ctx,
if (flag_map[flag] == 0) {
flag_map[flag] = 1;
flags_in_map++;
+   flags_changed = TRUE;
}
 }

@@ -1266,9 +1270,16 @@ _new_maildir_filename (void *ctx,
if (flag_map[flag]) {
flag_map[flag] = 0;
flags_in_map--;
+   flags_changed = TRUE;
}
 }

+/* Messages in new/ without maildir info can be kept in new/ if no
+ * flags have changed. */
+dir = (char *) _filename_is_in_maildir (filename);
+if (dir && STRNCMP_LITERAL (dir, "new/") == 0 && !*info && !flags_changed)
+   return talloc_strdup (ctx, filename);
+
 filename_new = (char *) talloc_size (ctx,
 info - filename +
 strlen (":2,") + flags_in_map + 1);
diff --git a/test/maildir-sync b/test/maildir-sync
index b2ac89f..33d2c58 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -84,7 +84,6 @@ test_expect_equal "$output" "No new mail."
 # creating all necessary database state for those directories.

 test_begin_subtest "Adding non-maildir tags does not move message from new to 
cur"
-test_subtest_known_broken
 add_message [subject]='"Message to stay in new"' \
 [date]='"Sat, 01 Jan 2000 12:00:00 -"' \
 [filename]='message-to-stay-in-new' [dir]=new
-- 
1.7.10.4



[PATCH 2/3] test: Add some missing maildir synchronization tests

2012-12-19 Thread Michal Sojka
As mentioned by Jani Nikula in id:87vcccp4y3.fsf at nikula.org, some cases
of maildir synchronization are not covered by our tests. Let's add the
missing tests.
---
 test/maildir-sync |   12 
 1 file changed, 12 insertions(+)

diff --git a/test/maildir-sync b/test/maildir-sync
index 6165782..b2ac89f 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -92,6 +92,18 @@ notmuch tag +donotmove subject:"Message to stay in new"
 output=$(cd "$MAIL_DIR"; ls */message-to-stay-in-new*)
 test_expect_equal "$output" "new/message-to-stay-in-new"

+test_begin_subtest "Message in cur lacking maildir info gets one on any tag 
change"
+add_message [filename]='message-to-get-maildir-info' [dir]=cur
+notmuch tag +anytag id:$gen_msg_id
+output=$(cd "$MAIL_DIR"; ls */message-to-get-maildir-info*)
+test_expect_equal "$output" "cur/message-to-get-maildir-info:2,"
+
+test_begin_subtest "Message in new with maildir info is moved to cur on any 
tag change"
+add_message [filename]='message-with-info-to-be-moved-to-cur:2,' [dir]=new
+notmuch tag +anytag id:$gen_msg_id
+output=$(cd "$MAIL_DIR"; ls */message-with-info-to-be-moved-to-cur*)
+test_expect_equal "$output" "cur/message-with-info-to-be-moved-to-cur:2,"
+
 test_begin_subtest "Removing 'S' flag from existing filename adds 'unread' tag"
 add_message [subject]='"Removing S flag"' [filename]='removing-s-flag:2,S' 
[dir]=cur
 output=$(notmuch search subject:"Removing S flag" | notmuch_search_sanitize)
-- 
1.7.10.4



[PATCH 1/3] test: Adding non-maildir tags does not move message from new to cur

2012-12-19 Thread Michal Sojka
From: Michal Sojka 

Some MUA's like mutt show the difference between "new" emails living in maildir
directory new/, and "old" emails living in maildir directory cur/. However
notmuch tag unconditionally moves selected messages from new/ to cur/, even if
no maildir synchronized tag is changed.

While maildir specification forbids messages with tags living in new/, there is
no need to move messages to cur/ when no maildir synchronized tag is changed.
Thus notmuch can remain transparent with respect to other MUA's.

[ Edited commit log to better describe the intended changes, and tag the
  test as broken until the actual changes are implemented -- Louis Rilling ]

Signed-off-by: Louis Rilling 

[ Converted to use test_subtest_known_broken, David Bremner ]
---
 test/maildir-sync |9 +
 1 file changed, 9 insertions(+)

diff --git a/test/maildir-sync b/test/maildir-sync
index 0fc742a..6165782 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -83,6 +83,15 @@ test_expect_equal "$output" "No new mail."
 # creating new directories in the mail store, then it should be
 # creating all necessary database state for those directories.

+test_begin_subtest "Adding non-maildir tags does not move message from new to 
cur"
+test_subtest_known_broken
+add_message [subject]='"Message to stay in new"' \
+[date]='"Sat, 01 Jan 2000 12:00:00 -"' \
+[filename]='message-to-stay-in-new' [dir]=new
+notmuch tag +donotmove subject:"Message to stay in new"
+output=$(cd "$MAIL_DIR"; ls */message-to-stay-in-new*)
+test_expect_equal "$output" "new/message-to-stay-in-new"
+
 test_begin_subtest "Removing 'S' flag from existing filename adds 'unread' tag"
 add_message [subject]='"Removing S flag"' [filename]='removing-s-flag:2,S' 
[dir]=cur
 output=$(notmuch search subject:"Removing S flag" | notmuch_search_sanitize)
-- 
1.7.10.4



[PATCH 0/3] Adding non-maildir tags does not move message from new to cur

2012-12-19 Thread Michal Sojka
Hello,

patches in this series should supersede the patches in
id:1316039001-32602-5-git-send-email-l.rilling at av7.net. They address
the comments from Jani Nikula and add tests for the behavior that was
unintentionally changed by the previous version of the patch.

Louis Rilling (1):
  tags_to_maildir_flags: Don't rename if no flags change

Michal Sojka (2):
  test: Adding non-maildir tags does not move message from new to cur
  test: Add some missing maildir synchronization tests

 lib/message.cc|   13 -
 test/maildir-sync |   20 
 2 files changed, 32 insertions(+), 1 deletion(-)

-- 
1.7.10.4



[DRAFT PATCH] emacs: Eliminate buffer invisibility specs from show and wash

2012-12-19 Thread Tomi Ollila
On Tue, Dec 18 2012, Austin Clements  wrote:

> Previously, all visibility in show buffers for headers, message
> bodies, and washed text was specified by generating one or more
> symbols for each region and creating overlays with their 'invisible
> property set to carefully crafted combinations of these symbols.
> Visibility was controlled not by modifying the overlays directly, but
> by adding and removing the generated symbols from a gigantic buffer
> invisibilty spec.
>
> This has myriad negative consequences.  It's slow because Emacs'
> display engine has to traverse the buffer invisibility list for every
> overlay and, since every overlay has its own symbol, this makes
> rendering O(N^2) in the number of overlays.  It composes poorly
> because symbol-type 'invisible properties are taken from the highest
> priority overlay over a given character (which is often ambiguous!),
> rather than being gathered from all overlays over a character.  As a
> result, we have to include symbols related to message hiding in the
> wash code lest the wash overlays un-hide parts of hidden messages.  It
> also requires various workarounds for isearch to properly open
> overlays, to set up buffer-invisibility-spec for
> remove-from-invisibility-spec to work right, and to explicitly refresh
> the display after updating the buffer invisibility spec.
>
> None of this is necessary.
>
> This patch converts show and wash to use simple boolean 'invisible
> properties and to not use the buffer invisibility spec.  Rather than
> adding and removing generated symbols from the invisibility spec, the
> code now directly toggles the 'invisible property of the appropriate
> overlay.  This speeds up rendering because the display engine only has
> to check the boolean values of the overlays over a character.  It
> composes nicely because text will be invisible if *any* overlay over
> it has 'invisible t, which means we can overlap invisibility overlays
> with abandon.  We no longer need any of the workarounds mentioned
> above.  And it fixes a minor bug for free: now, when isearch opens a
> washed region, the button text will update to say "Click/Enter to
> hide" rather than remaining unchanged.
> ---
>
> Mark's part toggling code got me thinking about how needlessly
> complicated our other show-mode invisibility code was.  This patch is
> a shot at fixing that.  It will require a bit of reworking after part
> toggling goes in (owning to the aforementioned non-composability, part
> toggling needs to be intimately aware of wash and message hiding).
> However, I think this patch should wait until after the release
> freeze; this code is fragile (though much less so after this patch),
> so I'd rather it soak for a release than cause user-visible bugs for
> no user-visible gain.

What is the current thought of getting this (and series of
id:1355858880-16329-1-git-send-email-markwalters1009 at gmail.com )
pushed ? The patches doesn't look bad to me and these seems to
work very well... 

Tomi


>
>  emacs/notmuch-show.el |   42 +++--
>  emacs/notmuch-wash.el |   97 
> ++---
>  2 files changed, 17 insertions(+), 122 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 7d9f8a9..4bdd5af 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -872,27 +872,8 @@ message at DEPTH in the current thread."
>message-start message-end
>content-start content-end
>headers-start headers-end
> -  body-start body-end
> -  (headers-invis-spec (notmuch-show-make-symbol "header"))
> -  (message-invis-spec (notmuch-show-make-symbol "message"))
>(bare-subject (notmuch-show-strip-re (plist-get headers :Subject
>  
> -;; Set `buffer-invisibility-spec' to `nil' (a list), otherwise
> -;; removing items from `buffer-invisibility-spec' (which is what
> -;; `notmuch-show-headers-visible' and
> -;; `notmuch-show-message-visible' do) is a no-op and has no
> -;; effect. This caused threads with only matching messages to have
> -;; those messages hidden initially because
> -;; `buffer-invisibility-spec' stayed `t'.
> -;;
> -;; This needs to be set here (rather than just above the call to
> -;; `notmuch-show-headers-visible') because some of the part
> -;; rendering or body washing functions
> -;; (e.g. `notmuch-wash-text/plain-citations') manipulate
> -;; `buffer-invisibility-spec').
> -(when (eq buffer-invisibility-spec t)
> -  (setq buffer-invisibility-spec nil))
> -
>  (setq message-start (point-marker))
>  
>  (notmuch-show-insert-headerline headers
> @@ -904,9 +885,6 @@ message at DEPTH in the current thread."
>  
>  (setq content-start (point-marker))
>  
> -(plist-put msg :headers-invis-spec headers-invis-spec)
> -(plist-put msg :message-invis-spec message-invis-spec)
> -
>  ;; Set `headers-start' to point after the 'Subject:' header 

[PATCH v5 0/4] Use invisibility to toggle display of all parts including multipart

2012-12-19 Thread Austin Clements
Quoth Mark Walters on Dec 18 at  7:27 pm:
> This is an alternative version of
> id:1355781287-6010-1-git-send-email-markwalters1009 at gmail.com based on
> top of Austin's patch at
> id:1355812810-32747-1-git-send-email-amdragon at mit.edu
> 
> Austin's patch significantly simplifies the invisibility handling
> taking this series down from 90/27 to 68/26 in diffstat terms.
> 
> In general terms Austin's approach has to be the right thing to do:
> what we want to do just before the freeze is less clear. My view is
> that we should go with Austin's approach now so that at least any bugs
> we get from it and (more likely) from this series apply to master as
> well. 
> 
> I am posting this series to make it easier for people to judge the two
> approaches when finished (ie with part invisibility too).
> 
> I attach a trimmed diff from v4 below the diffstat (note this was a
> diff with -U10 which I have trimmed so it is purely for information)
> 
> Note we no longer need patch 4/5 because in this approach the overlays
> do not need to know about other overlays.
> 
> Best wishes
> 
> Mark

LGTM.


Re: [DRAFT PATCH] emacs: Eliminate buffer invisibility specs from show and wash

2012-12-19 Thread Tomi Ollila
On Tue, Dec 18 2012, Austin Clements amdra...@mit.edu wrote:

 Previously, all visibility in show buffers for headers, message
 bodies, and washed text was specified by generating one or more
 symbols for each region and creating overlays with their 'invisible
 property set to carefully crafted combinations of these symbols.
 Visibility was controlled not by modifying the overlays directly, but
 by adding and removing the generated symbols from a gigantic buffer
 invisibilty spec.

 This has myriad negative consequences.  It's slow because Emacs'
 display engine has to traverse the buffer invisibility list for every
 overlay and, since every overlay has its own symbol, this makes
 rendering O(N^2) in the number of overlays.  It composes poorly
 because symbol-type 'invisible properties are taken from the highest
 priority overlay over a given character (which is often ambiguous!),
 rather than being gathered from all overlays over a character.  As a
 result, we have to include symbols related to message hiding in the
 wash code lest the wash overlays un-hide parts of hidden messages.  It
 also requires various workarounds for isearch to properly open
 overlays, to set up buffer-invisibility-spec for
 remove-from-invisibility-spec to work right, and to explicitly refresh
 the display after updating the buffer invisibility spec.

 None of this is necessary.

 This patch converts show and wash to use simple boolean 'invisible
 properties and to not use the buffer invisibility spec.  Rather than
 adding and removing generated symbols from the invisibility spec, the
 code now directly toggles the 'invisible property of the appropriate
 overlay.  This speeds up rendering because the display engine only has
 to check the boolean values of the overlays over a character.  It
 composes nicely because text will be invisible if *any* overlay over
 it has 'invisible t, which means we can overlap invisibility overlays
 with abandon.  We no longer need any of the workarounds mentioned
 above.  And it fixes a minor bug for free: now, when isearch opens a
 washed region, the button text will update to say Click/Enter to
 hide rather than remaining unchanged.
 ---

 Mark's part toggling code got me thinking about how needlessly
 complicated our other show-mode invisibility code was.  This patch is
 a shot at fixing that.  It will require a bit of reworking after part
 toggling goes in (owning to the aforementioned non-composability, part
 toggling needs to be intimately aware of wash and message hiding).
 However, I think this patch should wait until after the release
 freeze; this code is fragile (though much less so after this patch),
 so I'd rather it soak for a release than cause user-visible bugs for
 no user-visible gain.

What is the current thought of getting this (and series of
id:1355858880-16329-1-git-send-email-markwalters1...@gmail.com )
pushed ? The patches doesn't look bad to me and these seems to
work very well... 

Tomi



  emacs/notmuch-show.el |   42 +++--
  emacs/notmuch-wash.el |   97 
 ++---
  2 files changed, 17 insertions(+), 122 deletions(-)

 diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
 index 7d9f8a9..4bdd5af 100644
 --- a/emacs/notmuch-show.el
 +++ b/emacs/notmuch-show.el
 @@ -872,27 +872,8 @@ message at DEPTH in the current thread.
message-start message-end
content-start content-end
headers-start headers-end
 -  body-start body-end
 -  (headers-invis-spec (notmuch-show-make-symbol header))
 -  (message-invis-spec (notmuch-show-make-symbol message))
(bare-subject (notmuch-show-strip-re (plist-get headers :Subject
  
 -;; Set `buffer-invisibility-spec' to `nil' (a list), otherwise
 -;; removing items from `buffer-invisibility-spec' (which is what
 -;; `notmuch-show-headers-visible' and
 -;; `notmuch-show-message-visible' do) is a no-op and has no
 -;; effect. This caused threads with only matching messages to have
 -;; those messages hidden initially because
 -;; `buffer-invisibility-spec' stayed `t'.
 -;;
 -;; This needs to be set here (rather than just above the call to
 -;; `notmuch-show-headers-visible') because some of the part
 -;; rendering or body washing functions
 -;; (e.g. `notmuch-wash-text/plain-citations') manipulate
 -;; `buffer-invisibility-spec').
 -(when (eq buffer-invisibility-spec t)
 -  (setq buffer-invisibility-spec nil))
 -
  (setq message-start (point-marker))
  
  (notmuch-show-insert-headerline headers
 @@ -904,9 +885,6 @@ message at DEPTH in the current thread.
  
  (setq content-start (point-marker))
  
 -(plist-put msg :headers-invis-spec headers-invis-spec)
 -(plist-put msg :message-invis-spec message-invis-spec)
 -
  ;; Set `headers-start' to point after the 'Subject:' header to be
  ;; compatible with the existing implementation. This just sets it
  ;; 

[PATCH 2/3] test: Add some missing maildir synchronization tests

2012-12-19 Thread Michal Sojka
As mentioned by Jani Nikula in id:87vcccp4y3@nikula.org, some cases
of maildir synchronization are not covered by our tests. Let's add the
missing tests.
---
 test/maildir-sync |   12 
 1 file changed, 12 insertions(+)

diff --git a/test/maildir-sync b/test/maildir-sync
index 6165782..b2ac89f 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -92,6 +92,18 @@ notmuch tag +donotmove subject:Message to stay in new
 output=$(cd $MAIL_DIR; ls */message-to-stay-in-new*)
 test_expect_equal $output new/message-to-stay-in-new
 
+test_begin_subtest Message in cur lacking maildir info gets one on any tag 
change
+add_message [filename]='message-to-get-maildir-info' [dir]=cur
+notmuch tag +anytag id:$gen_msg_id
+output=$(cd $MAIL_DIR; ls */message-to-get-maildir-info*)
+test_expect_equal $output cur/message-to-get-maildir-info:2,
+
+test_begin_subtest Message in new with maildir info is moved to cur on any 
tag change
+add_message [filename]='message-with-info-to-be-moved-to-cur:2,' [dir]=new
+notmuch tag +anytag id:$gen_msg_id
+output=$(cd $MAIL_DIR; ls */message-with-info-to-be-moved-to-cur*)
+test_expect_equal $output cur/message-with-info-to-be-moved-to-cur:2,
+
 test_begin_subtest Removing 'S' flag from existing filename adds 'unread' tag
 add_message [subject]='Removing S flag' [filename]='removing-s-flag:2,S' 
[dir]=cur
 output=$(notmuch search subject:Removing S flag | notmuch_search_sanitize)
-- 
1.7.10.4

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


[PATCH 1/3] test: Adding non-maildir tags does not move message from new to cur

2012-12-19 Thread Michal Sojka
From: Michal Sojka so...@os.inf.tu-dresden.de

Some MUA's like mutt show the difference between new emails living in maildir
directory new/, and old emails living in maildir directory cur/. However
notmuch tag unconditionally moves selected messages from new/ to cur/, even if
no maildir synchronized tag is changed.

While maildir specification forbids messages with tags living in new/, there is
no need to move messages to cur/ when no maildir synchronized tag is changed.
Thus notmuch can remain transparent with respect to other MUA's.

[ Edited commit log to better describe the intended changes, and tag the
  test as broken until the actual changes are implemented -- Louis Rilling ]

Signed-off-by: Louis Rilling l.rill...@av7.net

[ Converted to use test_subtest_known_broken, David Bremner ]
---
 test/maildir-sync |9 +
 1 file changed, 9 insertions(+)

diff --git a/test/maildir-sync b/test/maildir-sync
index 0fc742a..6165782 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -83,6 +83,15 @@ test_expect_equal $output No new mail.
 # creating new directories in the mail store, then it should be
 # creating all necessary database state for those directories.
 
+test_begin_subtest Adding non-maildir tags does not move message from new to 
cur
+test_subtest_known_broken
+add_message [subject]='Message to stay in new' \
+[date]='Sat, 01 Jan 2000 12:00:00 -' \
+[filename]='message-to-stay-in-new' [dir]=new
+notmuch tag +donotmove subject:Message to stay in new
+output=$(cd $MAIL_DIR; ls */message-to-stay-in-new*)
+test_expect_equal $output new/message-to-stay-in-new
+
 test_begin_subtest Removing 'S' flag from existing filename adds 'unread' tag
 add_message [subject]='Removing S flag' [filename]='removing-s-flag:2,S' 
[dir]=cur
 output=$(notmuch search subject:Removing S flag | notmuch_search_sanitize)
-- 
1.7.10.4

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


[PATCH 3/3] tags_to_maildir_flags: Don't rename if no flags change

2012-12-19 Thread Michal Sojka
From: Louis Rilling l.rill...@av7.net

notmuch_message_tags_to_maildir_flags() unconditionally moves messages from
maildir directory new/ to maildir directory cur/, which makes messages lose
their new status in the MUA. However some users want to keep this new
status after, for instance, an auto-tagging of new messages.

However, as Austin mentioned and according to the maildir specification,
messages living in new/ are not allowed to have flags, even if mutt allows it
to happen. For this reason, this patch prevents moving messages from new/ to
cur/, only if no flags have to be changed. It's hopefully enough to satisfy
mutt (and maybe other MUAs showing the new status) users checking the new
status.

Changelog:
* v2: Fix bool type as well as NULL returned despite having no errors (Austin
  Clements)
* v4: Tag the related test (contributed by Michal Sojka) as working

Signed-off-by: Louis Rilling l.rill...@av7.net

[Condition for keeping messages in new/ was extended to satisfy all
 tests from the previous patch. -Michal Sojka]
---
 lib/message.cc|   13 -
 test/maildir-sync |1 -
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/lib/message.cc b/lib/message.cc
index 320901f..87369bb 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1195,7 +1195,9 @@ _get_maildir_flag_actions (notmuch_message_t *message,
  * compute the new maildir filename.
  *
  * If the existing filename is in the directory new, the new
- * filename will be in the directory cur.
+ * filename will be in the directory cur, except for the case when
+ * no flags are changed and the existing filename does not contain
+ * maildir info (starting with ,2:).
  *
  * After a sequence of :2, in the filename, any subsequent
  * single-character flags will be added or removed according to the
@@ -1218,6 +1220,7 @@ _new_maildir_filename (void *ctx,
 char *filename_new, *dir;
 char flag_map[128];
 int flags_in_map = 0;
+notmuch_bool_t flags_changed = FALSE;
 unsigned int i;
 char *s;
 
@@ -1258,6 +1261,7 @@ _new_maildir_filename (void *ctx,
if (flag_map[flag] == 0) {
flag_map[flag] = 1;
flags_in_map++;
+   flags_changed = TRUE;
}
 }
 
@@ -1266,9 +1270,16 @@ _new_maildir_filename (void *ctx,
if (flag_map[flag]) {
flag_map[flag] = 0;
flags_in_map--;
+   flags_changed = TRUE;
}
 }
 
+/* Messages in new/ without maildir info can be kept in new/ if no
+ * flags have changed. */
+dir = (char *) _filename_is_in_maildir (filename);
+if (dir  STRNCMP_LITERAL (dir, new/) == 0  !*info  !flags_changed)
+   return talloc_strdup (ctx, filename);
+
 filename_new = (char *) talloc_size (ctx,
 info - filename +
 strlen (:2,) + flags_in_map + 1);
diff --git a/test/maildir-sync b/test/maildir-sync
index b2ac89f..33d2c58 100755
--- a/test/maildir-sync
+++ b/test/maildir-sync
@@ -84,7 +84,6 @@ test_expect_equal $output No new mail.
 # creating all necessary database state for those directories.
 
 test_begin_subtest Adding non-maildir tags does not move message from new to 
cur
-test_subtest_known_broken
 add_message [subject]='Message to stay in new' \
 [date]='Sat, 01 Jan 2000 12:00:00 -' \
 [filename]='message-to-stay-in-new' [dir]=new
-- 
1.7.10.4

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


[PATCH 0/3] Adding non-maildir tags does not move message from new to cur

2012-12-19 Thread Michal Sojka
Hello,

patches in this series should supersede the patches in
id:1316039001-32602-5-git-send-email-l.rill...@av7.net. They address
the comments from Jani Nikula and add tests for the behavior that was
unintentionally changed by the previous version of the patch.

Louis Rilling (1):
  tags_to_maildir_flags: Don't rename if no flags change

Michal Sojka (2):
  test: Adding non-maildir tags does not move message from new to cur
  test: Add some missing maildir synchronization tests

 lib/message.cc|   13 -
 test/maildir-sync |   20 
 2 files changed, 32 insertions(+), 1 deletion(-)

-- 
1.7.10.4

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


Re: [PATCH] test: Adding non-maildir tags does not move message from new to cur

2012-12-19 Thread Michal Sojka
Hello Jani,

On Sat, Dec 08 2012, Jani Nikula wrote:
 On Sat, 08 Dec 2012, da...@tethera.net wrote:
 From: Michal Sojka so...@os.inf.tu-dresden.de

 Some MUA's like mutt show the difference between new emails living in 
 maildir
 directory new/, and old emails living in maildir directory cur/. However
 notmuch tag unconditionally moves selected messages from new/ to cur/, even 
 if
 no maildir synchronized tag is changed.

 While maildir specification forbids messages with tags living in new/, there 
 is
 no need to move messages to cur/ when no maildir synchronized tag is changed.
 Thus notmuch can remain transparent with respect to other MUA's.

 [ Edited commit log to better describe the intended changes, and tag the
   test as broken until the actual changes are implemented -- Louis Rilling ]

 Signed-off-by: Louis Rilling l.rill...@av7.net

 [ Converted to use test_subtest_known_broken, David Bremner ]
 ---

 Do we agree that the behaviour of moving messages to ./cur on tagging
 is broken? If so, maybe it's worth tidying up and applying this.  The
 use of cd and ls strikes me as slightly suspect, but I welcome other
 opinions.

 I think I would narrow down the special case a bit: I think messages in
 ./new that have no maildir flags, and have no :2, in the end of the
 filename, and and the tag change(s) will not affect maildir flags,
 should stay in ./new. Files in ./new should not have :2, or maildir
 flags, and I see no reason to support having them there.

 Thus any messages in ./new that do have maildir flags, or have :2, in
 the end of the filename should probably be moved to ./cur, even if the
 tag change(s) do not affect maildir flags. The patch in this thread
 fails here. It also changes the behaviour for messages in ./cur by not
 appending :2, to them.

I agree with you. In
id:1355952747-27350-1-git-send-email-sojk...@fel.cvut.cz I sent the
tests for the cases descried above as well as the updated patch for tag
to maildir synchronization.

 As to the test, I think it should do something along the lines of (based
 on search-output test):

 notmuch search --output=files subject:Message to stay in new | sed -e 
 s,$MAIL_DIR,MAIL_DIR, OUTPUT
 cat EOF EXPECTED
 MAIL_DIR/new/message-to-stay-in-new
 EOF
 test_expect_equal_file OUTPUT EXPECTED

With this you test what notmuch thinks about the file names of messages,
not whether the files have actually been renamed. For this reason I kept
the previous way of testing in the new patches.

Cheers,
-Michal
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] emacs: show: make id links respect window

2012-12-19 Thread Mark Walters
There is a bug in current notmuch: if you have multiple windows in one
frame and click an id button link in a show buffer that does not
contain point then the link is opened in the window containing point.

This reads the mouse event to make sure that the correct window is
used for the link.
---

I think this is a bug but that could be debated. It is particularly
easy to trigger with notmuch pick because that uses the split pane
while focus usually remains in the `pick' pane rather than the `show'
pane.

The lisp is not pretty but seems to work.

Best wishes

Mark




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

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5751d98..5664ea3 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1077,6 +1077,11 @@ buttons for a corresponding notmuch search.
(make-text-button (first link) (second link)
  'action `(lambda (arg)
 (notmuch-show ,(third link)))
+ 'mouse-action `(lambda (arg)
+  (let* ((event last-input-event)
+ (window (car (cadr event
+(select-window window)
+(notmuch-show ,(third link
  'follow-link t
  'help-echo Mouse-1, RET: search for this message
  'face goto-address-mail-face)
-- 
1.7.9.1

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