[notmuch] [PATCH] Simplify "unread" tag handling in emacs UI.

[Carl Worth]

On Thu, 25 Feb 2010 11:23:40 +0100, "Sebastian Spaeth"  wrote:
> Well, it is fatal in the sense that it makes that function useless. But
> it's not as bad as it is currently unused.

Right. I didn't want to push something out with a known-broken function.

I would have rather the patch just remove the function in that case,
(but the fact that someone noticed the broken function meant that
someone actually *was* using it and removing it wouldn't be nice
either).

> I still think it's worth taking this patch and fixing it then.

Done and pushed out now.

> - split into a "notmuch-show-next-message"
>   and a notmuch-show-show-next-message (what a naming!) function. 
>   One would merely skip to the next and one actually show and mark
>   unread.

That's the approach I took, with naming of
notmuch-show-next-message-without-marking-read. And yes, the naming we
have for our emacs functions is awful. I really dislike "notmuch-show"
as a prefix because it puts a verb in the prefix, (where I'd instead
like to just have a verb for the actual functionality of the function).

But we have to have at least *something* to separate the search-results
and message-viewing modes, (since we have commands like "archive" in
both of them). The lack of nice scoping here is really awkward.

-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

[notmuch] [PATCH] Simplify "unread" tag handling in emacs UI.

[Sebastian Spaeth]

On Wed, 24 Feb 2010 14:20:27 -0500, Jameson Rollins  wrote:
> On Wed, 24 Feb 2010 10:26:47 -0800, Carl Worth  wrote:

> > > It then checks the unread status in order to decide whether to proceed
> > > to the next again. So with your patch notmuch-show-next-unread-message
> > > will skip through all messages in a thread thinking they are all read
> > > (and actually marking all as read).
> > 
> > ...that seems like a fatal bug in this script. Thanks for noting that
> > Sebastian.
> 
> I certainly don't see it as fatal, but it is something we should
> resolve.  I think the simplification that the patch provides is worth
> it.

Well, it is fatal in the sense that it makes that function useless. But
it's not as bad as it is currently unused. You will only notice it if you
make the "skip to next unread" (interactive) and bind it to a key (which
is what I have done). I still think it's worth taking this patch and fixing it 
then.

The solutions I can see are: 
- split into a "notmuch-show-next-message"
  and a notmuch-show-show-next-message (what a naming!) function. 
  One would merely skip to the next and one actually show and mark unread.
- Introduce a second argument option whether we should mark as unread or not.
- rework "notmuch-show-next-unread-message" to not make use of
  "notmuch-show-next-message" but e.g. to just use
  "notmuch-show-find-next-message" or so.

Does this make any sense?

> I'm seeing the notmuch-show-next-unread-message as a non-interactive
> function that's not currently called by any other functions, and is
> therefore not being used.  Sebastian, are you using that in a private
> function, or am I misreading the code?

Touch?. As explained above. I made it interactive and bound it to 'n'.

Sebastian

Re: [notmuch] [PATCH] Simplify unread tag handling in emacs UI.

[Sebastian Spaeth]

On Wed, 24 Feb 2010 14:20:27 -0500, Jameson Rollins 
jroll...@finestructure.net wrote:
 On Wed, 24 Feb 2010 10:26:47 -0800, Carl Worth cwo...@cworth.org wrote:

   It then checks the unread status in order to decide whether to proceed
   to the next again. So with your patch notmuch-show-next-unread-message
   will skip through all messages in a thread thinking they are all read
   (and actually marking all as read).
  
  ...that seems like a fatal bug in this script. Thanks for noting that
  Sebastian.
 
 I certainly don't see it as fatal, but it is something we should
 resolve.  I think the simplification that the patch provides is worth
 it.

Well, it is fatal in the sense that it makes that function useless. But
it's not as bad as it is currently unused. You will only notice it if you
make the skip to next unread (interactive) and bind it to a key (which
is what I have done). I still think it's worth taking this patch and fixing it 
then.

The solutions I can see are: 
- split into a notmuch-show-next-message
  and a notmuch-show-show-next-message (what a naming!) function. 
  One would merely skip to the next and one actually show and mark unread.
- Introduce a second argument option whether we should mark as unread or not.
- rework notmuch-show-next-unread-message to not make use of
  notmuch-show-next-message but e.g. to just use
  notmuch-show-find-next-message or so.

Does this make any sense?

 I'm seeing the notmuch-show-next-unread-message as a non-interactive
 function that's not currently called by any other functions, and is
 therefore not being used.  Sebastian, are you using that in a private
 function, or am I misreading the code?

Touché. As explained above. I made it interactive and bound it to 'n'.

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

[notmuch] [PATCH] Simplify "unread" tag handling in emacs UI.

[Jameson Rollins]

On Wed, 24 Feb 2010 10:26:47 -0800, Carl Worth  wrote:
> On Wed, 17 Feb 2010 14:33:11 +0100, "Sebastian Spaeth"  SSpaeth.de> wrote:
> > On Tue, 19 Jan 2010 17:54:16 -0500, Jameson Rollins  > finestructure.net> wrote:
> > > This patch is intended to greatly simplify the handling of the
> > > "unread" tag in the emacs UI.  This patch adds a new function
> > > 'notmuch-show-mark-read', that removes the "unread" tag in
> > > notmuch-show-mode.  This function is then executed as a
> > > notmuch-show-hook, and by notmuch-show-next-message.  All of the
> > > functions that explicitly marked messages as unread are removed or
> > > renamed.

> Thanks for contributing the patch. This exact feature, (removing all
> commands with "and mark read" in their names), has been on my todo list
> for too long, and I'm anxious to remove it from that. But...
>
> > It then checks the unread status in order to decide whether to proceed
> > to the next again. So with your patch notmuch-show-next-unread-message
> > will skip through all messages in a thread thinking they are all read
> > (and actually marking all as read).
> 
> ...that seems like a fatal bug in this script. Thanks for noting that
> Sebastian.

I certainly don't see it as fatal, but it is something we should
resolve.  I think the simplification that the patch provides is worth
it.

I'm seeing the notmuch-show-next-unread-message as a non-interactive
function that's not currently called by any other functions, and is
therefore not being used.  Sebastian, are you using that in a private
function, or am I misreading the code?

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 

[notmuch] [PATCH] Simplify "unread" tag handling in emacs UI.

[Carl Worth]

On Wed, 17 Feb 2010 14:33:11 +0100, "Sebastian Spaeth"  wrote:
> On Tue, 19 Jan 2010 17:54:16 -0500, Jameson Rollins  finestructure.net> wrote:
> > This patch is intended to greatly simplify the handling of the
> > "unread" tag in the emacs UI.  This patch adds a new function
> > 'notmuch-show-mark-read', that removes the "unread" tag in
> > notmuch-show-mode.  This function is then executed as a
> > notmuch-show-hook, and by notmuch-show-next-message.  All of the
> > functions that explicitly marked messages as unread are removed or
> > renamed.

Hi Jameson,

Thanks for contributing the patch. This exact feature, (removing all
commands with "and mark read" in their names), has been on my todo list
for too long, and I'm anxious to remove it from that. But...

> It then checks the unread status in order to decide whether to proceed
> to the next again. So with your patch notmuch-show-next-unread-message
> will skip through all messages in a thread thinking they are all read
> (and actually marking all as read).

...that seems like a fatal bug in this script. Thanks for noting that
Sebastian.

I'm very interested in augmenting the test suite such that we could
write emacs-based tests that would capture this bug. It seems that with
the --batch, --load, and --funcall options it shouldn't be too hard to
write a modular function exercising emacs code and then execute it
no-interactively. Then we could either inspect the state of the database
to ensure the operation worked as desired, or else have the test
function write a buffer out to a file and test that the file has the
desired contents.

So that's something I'll be working on soon. And of course, I'll always
be glad to accept any help.

-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Re: [notmuch] [PATCH] Simplify unread tag handling in emacs UI.

[Carl Worth]

On Wed, 17 Feb 2010 14:33:11 +0100, Sebastian Spaeth sebast...@sspaeth.de 
wrote:
 On Tue, 19 Jan 2010 17:54:16 -0500, Jameson Rollins 
 jroll...@finestructure.net wrote:
  This patch is intended to greatly simplify the handling of the
  unread tag in the emacs UI.  This patch adds a new function
  'notmuch-show-mark-read', that removes the unread tag in
  notmuch-show-mode.  This function is then executed as a
  notmuch-show-hook, and by notmuch-show-next-message.  All of the
  functions that explicitly marked messages as unread are removed or
  renamed.

Hi Jameson,

Thanks for contributing the patch. This exact feature, (removing all
commands with and mark read in their names), has been on my todo list
for too long, and I'm anxious to remove it from that. But...

 It then checks the unread status in order to decide whether to proceed
 to the next again. So with your patch notmuch-show-next-unread-message
 will skip through all messages in a thread thinking they are all read
 (and actually marking all as read).

...that seems like a fatal bug in this script. Thanks for noting that
Sebastian.

I'm very interested in augmenting the test suite such that we could
write emacs-based tests that would capture this bug. It seems that with
the --batch, --load, and --funcall options it shouldn't be too hard to
write a modular function exercising emacs code and then execute it
no-interactively. Then we could either inspect the state of the database
to ensure the operation worked as desired, or else have the test
function write a buffer out to a file and test that the file has the
desired contents.

So that's something I'll be working on soon. And of course, I'll always
be glad to accept any help.

-Carl


pgpTZCas7ktU0.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [notmuch] [PATCH] Simplify unread tag handling in emacs UI.

[Jameson Rollins]

On Wed, 24 Feb 2010 10:26:47 -0800, Carl Worth cwo...@cworth.org wrote:
 On Wed, 17 Feb 2010 14:33:11 +0100, Sebastian Spaeth sebast...@sspaeth.de 
 wrote:
  On Tue, 19 Jan 2010 17:54:16 -0500, Jameson Rollins 
  jroll...@finestructure.net wrote:
   This patch is intended to greatly simplify the handling of the
   unread tag in the emacs UI.  This patch adds a new function
   'notmuch-show-mark-read', that removes the unread tag in
   notmuch-show-mode.  This function is then executed as a
   notmuch-show-hook, and by notmuch-show-next-message.  All of the
   functions that explicitly marked messages as unread are removed or
   renamed.

 Thanks for contributing the patch. This exact feature, (removing all
 commands with and mark read in their names), has been on my todo list
 for too long, and I'm anxious to remove it from that. But...

  It then checks the unread status in order to decide whether to proceed
  to the next again. So with your patch notmuch-show-next-unread-message
  will skip through all messages in a thread thinking they are all read
  (and actually marking all as read).
 
 ...that seems like a fatal bug in this script. Thanks for noting that
 Sebastian.

I certainly don't see it as fatal, but it is something we should
resolve.  I think the simplification that the patch provides is worth
it.

I'm seeing the notmuch-show-next-unread-message as a non-interactive
function that's not currently called by any other functions, and is
therefore not being used.  Sebastian, are you using that in a private
function, or am I misreading the code?

jamie.


pgpGEJw3EvCA3.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[notmuch] [PATCH] Simplify "unread" tag handling in emacs UI.

[Sebastian Spaeth]

On Tue, 19 Jan 2010 17:54:16 -0500, Jameson Rollins  wrote:
> This patch is intended to greatly simplify the handling of the
> "unread" tag in the emacs UI.  This patch adds a new function
> 'notmuch-show-mark-read', that removes the "unread" tag in
> notmuch-show-mode.  This function is then executed as a
> notmuch-show-hook, and by notmuch-show-next-message.  All of the
> functions that explicitly marked messages as unread are removed or
> renamed.

Hi Jameson,

I've been using this for quite some time now and I am pretty happy with
it. However, there is a problem with this approach as this renders the
function notmuch-show-next-unread-message useless. 

This proceeds to the next message, using
notmuch-show-next-message, which your patch marks automatically as read.

It then checks the unread status in order to decide whether to proceed
to the next again. So with your patch notmuch-show-next-unread-message
will skip through all messages in a thread thinking they are all read
(and actually marking all as read).

Sebastian

[notmuch] [PATCH] Simplify "unread" tag handling in emacs UI.

[Jameson Rollins]

On Wed, 17 Feb 2010 14:33:11 +0100, "Sebastian Spaeth"  wrote:
> I've been using this for quite some time now and I am pretty happy with
> it. However, there is a problem with this approach as this renders the
> function notmuch-show-next-unread-message useless. 
> 
> This proceeds to the next message, using
> notmuch-show-next-message, which your patch marks automatically as read.
> 
> It then checks the unread status in order to decide whether to proceed
> to the next again. So with your patch notmuch-show-next-unread-message
> will skip through all messages in a thread thinking they are all read
> (and actually marking all as read).

Hey, Sebastian.  I'm glad you've been finding the patch useful.  I'd be
happy to look at improving it, or of course accepting patch.  I was
hoping Carl might process these patch first, in which case it would be
easier to know what any improvements might look like.  I'll send a
separate email to ping Carl about that.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 

Re: [notmuch] [PATCH] Simplify unread tag handling in emacs UI.

[Sebastian Spaeth]

On Tue, 19 Jan 2010 17:54:16 -0500, Jameson Rollins 
jroll...@finestructure.net wrote:
 This patch is intended to greatly simplify the handling of the
 unread tag in the emacs UI.  This patch adds a new function
 'notmuch-show-mark-read', that removes the unread tag in
 notmuch-show-mode.  This function is then executed as a
 notmuch-show-hook, and by notmuch-show-next-message.  All of the
 functions that explicitly marked messages as unread are removed or
 renamed.

Hi Jameson,

I've been using this for quite some time now and I am pretty happy with
it. However, there is a problem with this approach as this renders the
function notmuch-show-next-unread-message useless. 

This proceeds to the next message, using
notmuch-show-next-message, which your patch marks automatically as read.

It then checks the unread status in order to decide whether to proceed
to the next again. So with your patch notmuch-show-next-unread-message
will skip through all messages in a thread thinking they are all read
(and actually marking all as read).

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

Re: [notmuch] [PATCH] Simplify unread tag handling in emacs UI.

[Jameson Rollins]

On Wed, 17 Feb 2010 14:33:11 +0100, Sebastian Spaeth sebast...@sspaeth.de 
wrote:
 I've been using this for quite some time now and I am pretty happy with
 it. However, there is a problem with this approach as this renders the
 function notmuch-show-next-unread-message useless. 
 
 This proceeds to the next message, using
 notmuch-show-next-message, which your patch marks automatically as read.
 
 It then checks the unread status in order to decide whether to proceed
 to the next again. So with your patch notmuch-show-next-unread-message
 will skip through all messages in a thread thinking they are all read
 (and actually marking all as read).

Hey, Sebastian.  I'm glad you've been finding the patch useful.  I'd be
happy to look at improving it, or of course accepting patch.  I was
hoping Carl might process these patch first, in which case it would be
easier to know what any improvements might look like.  I'll send a
separate email to ping Carl about that.

jamie.


pgpUtKyn99VO1.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[notmuch] [PATCH] Simplify unread tag handling in emacs UI.

[Jameson Rollins]

This patch is intended to greatly simplify the handling of the
unread tag in the emacs UI.  This patch adds a new function
'notmuch-show-mark-read', that removes the unread tag in
notmuch-show-mode.  This function is then executed as a
notmuch-show-hook, and by notmuch-show-next-message.  All of the
functions that explicitly marked messages as unread are removed or
renamed.

The idea here is that the user should never have to worry about
manipulating the unread tag.  The tag should persist until the user
actually views a message, at which point it should be automatically
removed.  This patch simplifies the notmuch.el quite a bit, removing a
lot of somewhat redundant functions, and reducing clutter in the name
and key-mapping space.
---
 notmuch.el |   95 +++
 1 files changed, 31 insertions(+), 64 deletions(-)

diff --git a/notmuch.el b/notmuch.el
index 97914f2..2da3c06 100644
--- a/notmuch.el
+++ b/notmuch.el
@@ -69,15 +69,12 @@
 (define-key map v 'notmuch-show-view-all-mime-parts)
 (define-key map - 'notmuch-show-remove-tag)
 (define-key map + 'notmuch-show-add-tag)
-(define-key map X 'notmuch-show-mark-read-then-archive-then-exit)
 (define-key map x 'notmuch-show-archive-thread-then-exit)
-(define-key map A 'notmuch-show-mark-read-then-archive-thread)
 (define-key map a 'notmuch-show-archive-thread)
 (define-key map p 'notmuch-show-previous-message)
-(define-key map N 'notmuch-show-mark-read-then-next-open-message)
 (define-key map n 'notmuch-show-next-message)
 (define-key map (kbd DEL) 'notmuch-show-rewind)
-(define-key map   'notmuch-show-advance-marking-read-and-archiving)
+(define-key map   'notmuch-show-advance-and-archive)
 map)
   Keymap for \notmuch show\ buffers.)
 (fset 'notmuch-show-mode-map notmuch-show-mode-map)
@@ -226,13 +223,22 @@ Unlike builtin `previous-line' this version accepts no 
arguments.
 (cons (notmuch-show-get-message-id) nil)))
  (notmuch-show-set-tags (sort (set-difference tags toremove :test 
'string=) 'string))
 
-(defun notmuch-show-archive-thread-maybe-mark-read (markread)
+(defun notmuch-show-archive-thread ()
+  Archive each message in thread, then show next thread from search.
+
+Archive each message currently shown by removing the \inbox\
+tag from each. Then kill this buffer and show the next thread
+from the search from which this thread was originally shown.
+
+Note: This command is safe from any race condition of new messages
+being delivered to the same thread. It does not archive the
+entire thread, but only the messages shown in the current
+buffer.
+  (interactive)
   (save-excursion
 (goto-char (point-min))
 (while (not (eobp))
-  (if markread
- (notmuch-show-remove-tag unread inbox)
-   (notmuch-show-remove-tag inbox))
+  (notmuch-show-remove-tag inbox)
   (if (not (eobp))
  (forward-char))
   (if (not (re-search-forward notmuch-show-message-begin-regexp nil t))
@@ -245,47 +251,12 @@ Unlike builtin `previous-line' this version accepts no 
arguments.
  (forward-line)
  (notmuch-search-show-thread)
 
-(defun notmuch-show-mark-read-then-archive-thread ()
-  Remove unread tags from thread, then archive and show next thread.
-
-Archive each message currently shown by removing the \unread\
-and \inbox\ tag from each. Then kill this buffer and show the
-next thread from the search from which this thread was originally
-shown.
-
-Note: This command is safe from any race condition of new messages
-being delivered to the same thread. It does not archive the
-entire thread, but only the messages shown in the current
-buffer.
-  (interactive)
-  (notmuch-show-archive-thread-maybe-mark-read t))
-
-(defun notmuch-show-archive-thread ()
-  Archive each message in thread, then show next thread from search.
-
-Archive each message currently shown by removing the \inbox\
-tag from each. Then kill this buffer and show the next thread
-from the search from which this thread was originally shown.
-
-Note: This command is safe from any race condition of new messages
-being delivered to the same thread. It does not archive the
-entire thread, but only the messages shown in the current
-buffer.
-  (interactive)
-  (notmuch-show-archive-thread-maybe-mark-read nil))
-
 (defun notmuch-show-archive-thread-then-exit ()
   Archive each message in thread, then exit back to search results.
   (interactive)
   (notmuch-show-archive-thread)
   (kill-this-buffer))
 
-(defun notmuch-show-mark-read-then-archive-then-exit ()
-  Remove unread tags from thread, then archive and exit to search results.
-  (interactive)
-  (notmuch-show-mark-read-then-archive-thread)
-  (kill-this-buffer))
-
 (defun notmuch-show-view-raw-message ()
   View the raw email of the current message.
   (interactive)
@@ -440,7 +411,8 @@ Returns nil if already on the last message in the buffer.
 (while