unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility
@ 2012-01-25  5:25 Pieter Praet
  2012-01-25  6:35 ` David Edmondson
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Pieter Praet @ 2012-01-25  5:25 UTC (permalink / raw)
  To: Notmuch Mail

* emacs/notmuch-show.el (notmuch-show-open-or-close-all):
  Rename to `notmuch-show-toggle-all-messages', and make it toggle
  visibility of all messages based on the visibility of the current
  message, instead of setting visibility based on whether or not a
  prefix arg was supplied.

Same functionality, less effort (reaching for 'C-u' is a pain)...

---
 emacs/notmuch-show.el |   22 ++++++++++++----------
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e6a5b31..2d17f74 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1050,8 +1050,8 @@ thread id.  If a prefix is given, crypto processing is toggled."
 	(define-key map "p" 'notmuch-show-previous-open-message)
 	(define-key map (kbd "DEL") 'notmuch-show-rewind)
 	(define-key map " " 'notmuch-show-advance-and-archive)
-	(define-key map (kbd "M-RET") 'notmuch-show-open-or-close-all)
 	(define-key map (kbd "RET") 'notmuch-show-toggle-message)
+	(define-key map (kbd "M-RET") 'notmuch-show-toggle-all-messages)
 	(define-key map "#" 'notmuch-show-print-message)
 	map)
       "Keymap for \"notmuch show\" buffers.")
@@ -1502,16 +1502,18 @@ the result."
      (not (plist-get props :message-visible))))
   (force-window-update))
 
-(defun notmuch-show-open-or-close-all ()
-  "Set the visibility all of the messages in the current thread.
-By default make all of the messages visible. With a prefix
-argument, hide all of the messages."
+(defun notmuch-show-toggle-all-messages ()
+  "Toggle the visibility of all messages in the current thread.
+If the current message is visible, hide all messages -- and vice versa."
   (interactive)
-  (save-excursion
-    (goto-char (point-min))
-    (loop do (notmuch-show-message-visible (notmuch-show-get-message-properties)
-					   (not current-prefix-arg))
-	  until (not (notmuch-show-goto-message-next))))
+  (let ((toggle (notmuch-show-message-visible-p)))
+    (save-excursion
+      (goto-char (point-min))
+      (loop do (notmuch-show-message-visible
+		(notmuch-show-get-message-properties)
+		(not toggle))
+	    until (not (notmuch-show-goto-message-next)))))
+  (recenter-top-bottom 1)
   (force-window-update))
 
 (defun notmuch-show-next-button ()
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility
  2012-01-25  5:25 [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility Pieter Praet
@ 2012-01-25  6:35 ` David Edmondson
  2012-01-25 13:03   ` Tomi Ollila
  2012-01-26 13:02   ` Pieter Praet
  2012-01-26 15:58 ` Tomi Ollila
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 22+ messages in thread
From: David Edmondson @ 2012-01-25  6:35 UTC (permalink / raw)
  To: Pieter Praet, Notmuch Mail

[-- Attachment #1: Type: text/plain, Size: 3103 bytes --]

On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet <pieter@praet.org> wrote:
> * emacs/notmuch-show.el (notmuch-show-open-or-close-all):
>   Rename to `notmuch-show-toggle-all-messages', and make it toggle
>   visibility of all messages based on the visibility of the current
>   message, instead of setting visibility based on whether or not a
>   prefix arg was supplied.
> 
> Same functionality, less effort (reaching for 'C-u' is a pain)...

-1.

The behaviour you've provided is not what I want, from two perspectives:
        - currently it's clear what will happen when I use M-RET or
          C-uM-RET without me having to think about whether the cursor
          is over an open message,
        - often I'll be reading an open message and I want to open all
          of the rest to look at some context. That's a little more
          awkward after this change.
> 
> ---
>  emacs/notmuch-show.el |   22 ++++++++++++----------
>  1 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index e6a5b31..2d17f74 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1050,8 +1050,8 @@ thread id.  If a prefix is given, crypto processing is toggled."
>  	(define-key map "p" 'notmuch-show-previous-open-message)
>  	(define-key map (kbd "DEL") 'notmuch-show-rewind)
>  	(define-key map " " 'notmuch-show-advance-and-archive)
> -	(define-key map (kbd "M-RET") 'notmuch-show-open-or-close-all)
>  	(define-key map (kbd "RET") 'notmuch-show-toggle-message)
> +	(define-key map (kbd "M-RET") 'notmuch-show-toggle-all-messages)
>  	(define-key map "#" 'notmuch-show-print-message)
>  	map)
>        "Keymap for \"notmuch show\" buffers.")
> @@ -1502,16 +1502,18 @@ the result."
>       (not (plist-get props :message-visible))))
>    (force-window-update))
>  
> -(defun notmuch-show-open-or-close-all ()
> -  "Set the visibility all of the messages in the current thread.
> -By default make all of the messages visible. With a prefix
> -argument, hide all of the messages."
> +(defun notmuch-show-toggle-all-messages ()
> +  "Toggle the visibility of all messages in the current thread.
> +If the current message is visible, hide all messages -- and vice versa."
>    (interactive)
> -  (save-excursion
> -    (goto-char (point-min))
> -    (loop do (notmuch-show-message-visible (notmuch-show-get-message-properties)
> -					   (not current-prefix-arg))
> -	  until (not (notmuch-show-goto-message-next))))
> +  (let ((toggle (notmuch-show-message-visible-p)))
> +    (save-excursion
> +      (goto-char (point-min))
> +      (loop do (notmuch-show-message-visible
> +		(notmuch-show-get-message-properties)
> +		(not toggle))
> +	    until (not (notmuch-show-goto-message-next)))))
> +  (recenter-top-bottom 1)
>    (force-window-update))
>  
>  (defun notmuch-show-next-button ()
> -- 
> 1.7.8.1
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility
  2012-01-25  6:35 ` David Edmondson
@ 2012-01-25 13:03   ` Tomi Ollila
  2012-01-26 13:02   ` Pieter Praet
  1 sibling, 0 replies; 22+ messages in thread
From: Tomi Ollila @ 2012-01-25 13:03 UTC (permalink / raw)
  To: David Edmondson, Pieter Praet, Notmuch Mail

On Wed, 25 Jan 2012 06:35:33 +0000, David Edmondson <dme@dme.org> wrote:
> On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet <pieter@praet.org> wrote:
> > * emacs/notmuch-show.el (notmuch-show-open-or-close-all):
> >   Rename to `notmuch-show-toggle-all-messages', and make it toggle
> >   visibility of all messages based on the visibility of the current
> >   message, instead of setting visibility based on whether or not a
> >   prefix arg was supplied.
> > 
> > Same functionality, less effort (reaching for 'C-u' is a pain)...
> 
> -1.
> 
> The behaviour you've provided is not what I want, from two perspectives:
>         - currently it's clear what will happen when I use M-RET or
>           C-uM-RET without me having to think about whether the cursor
>           is over an open message,
>         - often I'll be reading an open message and I want to open all
>           of the rest to look at some context. That's a little more
>           awkward after this change.

Yes, let's just switch. c-u M-RET to expand and M-RET to collapse (!!!)

(Needless to say I've never expanded but often collapsed ;)

Tomi

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility
  2012-01-25  6:35 ` David Edmondson
  2012-01-25 13:03   ` Tomi Ollila
@ 2012-01-26 13:02   ` Pieter Praet
  2012-01-26 15:04     ` David Edmondson
  1 sibling, 1 reply; 22+ messages in thread
From: Pieter Praet @ 2012-01-26 13:02 UTC (permalink / raw)
  To: David Edmondson, Notmuch Mail

On Wed, 25 Jan 2012 06:35:33 +0000, David Edmondson <dme@dme.org> wrote:
> On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet <pieter@praet.org> wrote:
> > * emacs/notmuch-show.el (notmuch-show-open-or-close-all):
> >   Rename to `notmuch-show-toggle-all-messages', and make it toggle
> >   visibility of all messages based on the visibility of the current
> >   message, instead of setting visibility based on whether or not a
> >   prefix arg was supplied.
> > 
> > Same functionality, less effort (reaching for 'C-u' is a pain)...
> 
> -1.
> 
> The behaviour you've provided is not what I want, from two perspectives:
>         - currently it's clear what will happen when I use M-RET or
>           C-uM-RET without me having to think about whether the cursor
>           is over an open message,
>         - often I'll be reading an open message and I want to open all
>           of the rest to look at some context. That's a little more
>           awkward after this change.

I may be missing something, but wouldn't both issues be solved by simply
pressing M-RET a second time?  I've been using this for a little while
now, and IMO it makes navigating through long and diverging threads a lot
faster, much like zooming in/out on an outline.

How about if C-u M-RET behaved as usual ?


> > 
> > ---
> >  emacs/notmuch-show.el |   22 ++++++++++++----------
> >  1 files changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> > index e6a5b31..2d17f74 100644
> > --- a/emacs/notmuch-show.el
> > +++ b/emacs/notmuch-show.el
> > @@ -1050,8 +1050,8 @@ thread id.  If a prefix is given, crypto processing is toggled."
> >  	(define-key map "p" 'notmuch-show-previous-open-message)
> >  	(define-key map (kbd "DEL") 'notmuch-show-rewind)
> >  	(define-key map " " 'notmuch-show-advance-and-archive)
> > -	(define-key map (kbd "M-RET") 'notmuch-show-open-or-close-all)
> >  	(define-key map (kbd "RET") 'notmuch-show-toggle-message)
> > +	(define-key map (kbd "M-RET") 'notmuch-show-toggle-all-messages)
> >  	(define-key map "#" 'notmuch-show-print-message)
> >  	map)
> >        "Keymap for \"notmuch show\" buffers.")
> > @@ -1502,16 +1502,18 @@ the result."
> >       (not (plist-get props :message-visible))))
> >    (force-window-update))
> >  
> > -(defun notmuch-show-open-or-close-all ()
> > -  "Set the visibility all of the messages in the current thread.
> > -By default make all of the messages visible. With a prefix
> > -argument, hide all of the messages."
> > +(defun notmuch-show-toggle-all-messages ()
> > +  "Toggle the visibility of all messages in the current thread.
> > +If the current message is visible, hide all messages -- and vice versa."
> >    (interactive)
> > -  (save-excursion
> > -    (goto-char (point-min))
> > -    (loop do (notmuch-show-message-visible (notmuch-show-get-message-properties)
> > -					   (not current-prefix-arg))
> > -	  until (not (notmuch-show-goto-message-next))))
> > +  (let ((toggle (notmuch-show-message-visible-p)))
> > +    (save-excursion
> > +      (goto-char (point-min))
> > +      (loop do (notmuch-show-message-visible
> > +		(notmuch-show-get-message-properties)
> > +		(not toggle))
> > +	    until (not (notmuch-show-goto-message-next)))))
> > +  (recenter-top-bottom 1)
> >    (force-window-update))
> >  
> >  (defun notmuch-show-next-button ()
> > -- 
> > 1.7.8.1
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch


Peace

-- 
Pieter

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility
  2012-01-26 13:02   ` Pieter Praet
@ 2012-01-26 15:04     ` David Edmondson
  0 siblings, 0 replies; 22+ messages in thread
From: David Edmondson @ 2012-01-26 15:04 UTC (permalink / raw)
  To: Pieter Praet, Notmuch Mail

[-- Attachment #1: Type: text/plain, Size: 511 bytes --]

On Thu, 26 Jan 2012 14:02:15 +0100, Pieter Praet <pieter@praet.org> wrote:
> I may be missing something, but wouldn't both issues be solved by simply
> pressing M-RET a second time?  I've been using this for a little while
> now, and IMO it makes navigating through long and diverging threads a lot
> faster, much like zooming in/out on an outline.
> 
> How about if C-u M-RET behaved as usual ?

Okay, I used it for a day and I'm happy that the original patch is
good.

+2 (which leaves me at +1).

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility
  2012-01-25  5:25 [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility Pieter Praet
  2012-01-25  6:35 ` David Edmondson
@ 2012-01-26 15:58 ` Tomi Ollila
  2012-02-13 10:51 ` Dmitry Kurochkin
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Tomi Ollila @ 2012-01-26 15:58 UTC (permalink / raw)
  To: Pieter Praet, Notmuch Mail


+1

Tomi

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility
  2012-01-25  5:25 [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility Pieter Praet
  2012-01-25  6:35 ` David Edmondson
  2012-01-26 15:58 ` Tomi Ollila
@ 2012-02-13 10:51 ` Dmitry Kurochkin
  2012-02-22 18:41   ` Pieter Praet
  2012-02-22 18:43 ` [PATCH v2 1/7] test: emacs: new tests "notmuch-show: {show, hide} message headers" Pieter Praet
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Dmitry Kurochkin @ 2012-02-13 10:51 UTC (permalink / raw)
  To: Pieter Praet, Notmuch Mail

Hi Pieter.

On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet <pieter@praet.org> wrote:
> * emacs/notmuch-show.el (notmuch-show-open-or-close-all):
>   Rename to `notmuch-show-toggle-all-messages', and make it toggle
>   visibility of all messages based on the visibility of the current
>   message, instead of setting visibility based on whether or not a
>   prefix arg was supplied.
> 
> Same functionality, less effort (reaching for 'C-u' is a pain)...
> 
> ---
>  emacs/notmuch-show.el |   22 ++++++++++++----------
>  1 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index e6a5b31..2d17f74 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1050,8 +1050,8 @@ thread id.  If a prefix is given, crypto processing is toggled."
>  	(define-key map "p" 'notmuch-show-previous-open-message)
>  	(define-key map (kbd "DEL") 'notmuch-show-rewind)
>  	(define-key map " " 'notmuch-show-advance-and-archive)
> -	(define-key map (kbd "M-RET") 'notmuch-show-open-or-close-all)
>  	(define-key map (kbd "RET") 'notmuch-show-toggle-message)
> +	(define-key map (kbd "M-RET") 'notmuch-show-toggle-all-messages)

Should the function name include "visible" or "visibility" to make it
clear what is toggled?  E.g. `notmuch-show-toggle-visibility-all'.

Also, consider changing "all-messages" to just "all" or "thread".  That
seems to be more consistent with other functions.

>  	(define-key map "#" 'notmuch-show-print-message)
>  	map)
>        "Keymap for \"notmuch show\" buffers.")
> @@ -1502,16 +1502,18 @@ the result."
>       (not (plist-get props :message-visible))))
>    (force-window-update))
>  
> -(defun notmuch-show-open-or-close-all ()
> -  "Set the visibility all of the messages in the current thread.
> -By default make all of the messages visible. With a prefix
> -argument, hide all of the messages."
> +(defun notmuch-show-toggle-all-messages ()
> +  "Toggle the visibility of all messages in the current thread.
> +If the current message is visible, hide all messages -- and vice versa."
>    (interactive)
> -  (save-excursion
> -    (goto-char (point-min))
> -    (loop do (notmuch-show-message-visible (notmuch-show-get-message-properties)
> -					   (not current-prefix-arg))
> -	  until (not (notmuch-show-goto-message-next))))
> +  (let ((toggle (notmuch-show-message-visible-p)))

Please rename "toggle" to "visible-p".  That would make it more clear
what the variable means, and is consistent with
`notmuch-show-message-visible-p'.

> +    (save-excursion
> +      (goto-char (point-min))
> +      (loop do (notmuch-show-message-visible
> +		(notmuch-show-get-message-properties)
> +		(not toggle))
> +	    until (not (notmuch-show-goto-message-next)))))

A new `notmuch-show-mapc' function was introduced in a recent commit.
Please use it here instead of a custom loop.

> +  (recenter-top-bottom 1)

There was no `recenter-top-bottom' call before.  Why is it needed now?
It seems like an independent change and, if it is needed, would be
better as a separate patch.  At the very least, it's worth mentioning in
the preamble and perhaps in a comment.

Regards,
  Dmitry

>    (force-window-update))
>  
>  (defun notmuch-show-next-button ()
> -- 
> 1.7.8.1
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility
  2012-02-13 10:51 ` Dmitry Kurochkin
@ 2012-02-22 18:41   ` Pieter Praet
  0 siblings, 0 replies; 22+ messages in thread
From: Pieter Praet @ 2012-02-22 18:41 UTC (permalink / raw)
  To: Dmitry Kurochkin, Notmuch Mail

On Mon, 13 Feb 2012 14:51:17 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Hi Pieter.
> 
> On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet <pieter@praet.org> wrote:
> > * emacs/notmuch-show.el (notmuch-show-open-or-close-all):
> >   Rename to `notmuch-show-toggle-all-messages', and make it toggle
> >   visibility of all messages based on the visibility of the current
> >   message, instead of setting visibility based on whether or not a
> >   prefix arg was supplied.
> > 
> > Same functionality, less effort (reaching for 'C-u' is a pain)...
> > 
> > ---
> >  emacs/notmuch-show.el |   22 ++++++++++++----------
> >  1 files changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> > index e6a5b31..2d17f74 100644
> > --- a/emacs/notmuch-show.el
> > +++ b/emacs/notmuch-show.el
> > @@ -1050,8 +1050,8 @@ thread id.  If a prefix is given, crypto processing is toggled."
> >  	(define-key map "p" 'notmuch-show-previous-open-message)
> >  	(define-key map (kbd "DEL") 'notmuch-show-rewind)
> >  	(define-key map " " 'notmuch-show-advance-and-archive)
> > -	(define-key map (kbd "M-RET") 'notmuch-show-open-or-close-all)
> >  	(define-key map (kbd "RET") 'notmuch-show-toggle-message)
> > +	(define-key map (kbd "M-RET") 'notmuch-show-toggle-all-messages)
> 
> Should the function name include "visible" or "visibility" to make it
> clear what is toggled?  E.g. `notmuch-show-toggle-visibility-all'.
> 
> Also, consider changing "all-messages" to just "all" or "thread".  That
> seems to be more consistent with other functions.
>

Good point, but we also have `notmuch-show-toggle-message' and
`notmuch-show-toggle-headers', so `notmuch-show-toggle-visibility-all'
would imply that both messages as well as headers are toggled.

Also, `notmuch-show-toggle-visibility-thread' sounds to me like
it would toggle the thread itself instead of the messages of
which it is composed, so my personal expectation would be that
it just blanks the entire buffer. (what's in a name....)

How about renaming the relevant functions like so:
- `notmuch-show-toggle-headers'    -> `notmuch-show-toggle-visibility-headers'
- `notmuch-show-toggle-message'    -> `notmuch-show-toggle-visibility-message'
- `notmuch-show-open-or-close-all' -> `notmuch-show-toggle-visibility-messages'

> >  	(define-key map "#" 'notmuch-show-print-message)
> >  	map)
> >        "Keymap for \"notmuch show\" buffers.")
> > @@ -1502,16 +1502,18 @@ the result."
> >       (not (plist-get props :message-visible))))
> >    (force-window-update))
> >  
> > -(defun notmuch-show-open-or-close-all ()
> > -  "Set the visibility all of the messages in the current thread.
> > -By default make all of the messages visible. With a prefix
> > -argument, hide all of the messages."
> > +(defun notmuch-show-toggle-all-messages ()
> > +  "Toggle the visibility of all messages in the current thread.
> > +If the current message is visible, hide all messages -- and vice versa."
> >    (interactive)
> > -  (save-excursion
> > -    (goto-char (point-min))
> > -    (loop do (notmuch-show-message-visible (notmuch-show-get-message-properties)
> > -					   (not current-prefix-arg))
> > -	  until (not (notmuch-show-goto-message-next))))
> > +  (let ((toggle (notmuch-show-message-visible-p)))
> 
> Please rename "toggle" to "visible-p".  That would make it more clear
> what the variable means, and is consistent with
> `notmuch-show-message-visible-p'.
>

AFAIK the '-p' suffix is "reserved" for predicate functions, and
using it for a variable could be confusing.  But I'm not aware of
any guidelines on indicating the variable type except when it
stores one or more functions [1,2]...

Perhaps we could call it `visible-bool' ?

Anyways, I've gone with your suggestion: `visible-p' it is...

> > +    (save-excursion
> > +      (goto-char (point-min))
> > +      (loop do (notmuch-show-message-visible
> > +		(notmuch-show-get-message-properties)
> > +		(not toggle))
> > +	    until (not (notmuch-show-goto-message-next)))))
> 
> A new `notmuch-show-mapc' function was introduced in a recent commit.
> Please use it here instead of a custom loop.
>

Nice!

> > +  (recenter-top-bottom 1)
> 
> There was no `recenter-top-bottom' call before.  Why is it needed now?
> It seems like an independent change and, if it is needed, would be
> better as a separate patch.  At the very least, it's worth mentioning in
> the preamble and perhaps in a comment.
>

It ensures that the message being uncollapsed is put properly in view
(instead of starting somewhere in the middle of the buffer) whilst also
making it obvious that/if/when there's previous messages in the thread
(due to its argument being 1 instead of 0).

I thought about using `notmuch-show-message-adjust' instead, but that
obscures the fact that there's previous messages.

As it's quite essential in making the function DTRT, I've opted to
clarify it in a comment as well as the commit message instead of
splitting it out into a separate patch.

> Regards,
>   Dmitry
> 
> >    (force-window-update))
> >  
> >  (defun notmuch-show-next-button ()
> > -- 
> > 1.7.8.1
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch

So... I've expanded the test suite to cover everything I might be
breaking, renamed the toggle functions to be consistent, and addressed
all your comments in some way or another.  I've also thrown in a bonus
patch which is *not* meant to be applied (WIP, should eventually provide
functionality similar to `notmuch-search-filter{,-by-tag}').

Patches follow.


Peace

-- 
Pieter

[1] http://www.gnu.org/software/emacs/manual/html_node/elisp/Coding-Conventions.html
[2] http://www.gnu.org/software/emacs/manual/html_node/elisp/Hooks.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 1/7] test: emacs: new tests "notmuch-show: {show, hide} message headers"
  2012-01-25  5:25 [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility Pieter Praet
                   ` (2 preceding siblings ...)
  2012-02-13 10:51 ` Dmitry Kurochkin
@ 2012-02-22 18:43 ` Pieter Praet
  2012-10-20 20:40   ` David Bremner
  2012-02-22 18:43 ` [PATCH v2 2/7] test: emacs: new tests "notmuch-show: {, un}collapse all messages in thread" Pieter Praet
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Pieter Praet @ 2012-02-22 18:43 UTC (permalink / raw)
  To: Notmuch Mail

* test/emacs:

  - New subtest "notmuch-show: show message headers":
    Setting `notmuch-message-headers-visible' to t causes all headers
    defined in `notmuch-message-headers' to be shown.

  - New subtest "notmuch-show: hide message headers":
    Setting `notmuch-message-headers-visible' to nil causes all headers
    defined in `notmuch-message-headers' to be hidden.
    ("Subject:" may be an exception;  See the use of `headers-start' in
    `notmuch-show-insert-msg')

  - New subtest "notmuch-show: hide message headers (w/ notmuch-show-toggle-headers)":
    Setting `notmuch-message-headers-visible' to t causes all headers
    defined in `notmuch-message-headers' to be shown, but they can be
    hidden for the current message by running `notmuch-show-toggle-headers'.
---
 test/emacs                                         |   25 ++++++++++++++++++++
 .../notmuch-show-message-with-headers-hidden       |   22 +++++++++++++++++
 .../notmuch-show-message-with-headers-visible      |   25 ++++++++++++++++++++
 3 files changed, 72 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs.expected-output/notmuch-show-message-with-headers-hidden
 create mode 100644 test/emacs.expected-output/notmuch-show-message-with-headers-visible

diff --git a/test/emacs b/test/emacs
index b74cfa9..f9ea1c3 100755
--- a/test/emacs
+++ b/test/emacs
@@ -383,6 +383,31 @@ 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: show message headers"
+test_emacs \
+	'(let ((notmuch-message-headers '\''("Subject" "To" "Cc" "Date"))
+	       (notmuch-message-headers-visible t))
+	   (notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
+	   (test-visible-output))'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-message-with-headers-visible
+
+test_begin_subtest "notmuch-show: hide message headers"
+test_emacs \
+	'(let ((notmuch-message-headers '\''("Subject" "To" "Cc" "Date"))
+	       (notmuch-message-headers-visible nil))
+	   (notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
+	   (test-visible-output))'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-message-with-headers-hidden
+
+test_begin_subtest "notmuch-show: hide message headers (w/ notmuch-show-toggle-headers)"
+test_emacs \
+	'(let ((notmuch-message-headers '\''("Subject" "To" "Cc" "Date"))
+	       (notmuch-message-headers-visible t))
+	   (notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
+	   (notmuch-show-toggle-headers)
+	   (test-visible-output))'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-message-with-headers-hidden
+
 test_begin_subtest "Stashing in notmuch-show"
 add_message '[date]="Sat, 01 Jan 2000 12:00:00 -0000"' \
     '[from]="Some One <someone@somewhere.org>"' \
diff --git a/test/emacs.expected-output/notmuch-show-message-with-headers-hidden b/test/emacs.expected-output/notmuch-show-message-with-headers-hidden
new file mode 100644
index 0000000..9d7f91b
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-show-message-with-headers-hidden
@@ -0,0 +1,22 @@
+Jan Janak <jan@ryngle.com> (2009-11-17) (inbox unread)
+Subject: [notmuch] What a great idea!
+ Jan Janak <jan@ryngle.com> (2009-11-17) (inbox)
+
+ On Tue, Nov 17, 2009 at 11:35 PM, Jan Janak <jan at ryngle.com> wrote:
+ > Hello,
+ >
+ > First of all, notmuch is a wonderful idea, both the cmdline tool and
+ [ 2 more citation lines. Click/Enter to show. ]
+ >
+ > Have you considered sending an announcement to the org-mode mailing list?
+ > http://org-mode.org
+
+ Sorry, wrong URL, the correct one is: http://orgmode.org
+
+ > Various ways of searching/referencing emails from emacs were discussed
+ > there several times and none of them were as elegant as notmuch (not
+ > even close). Maybe notmuch would attract some of the developers
+ > there..
+
+   -- Jan
+ Carl Worth <cworth@cworth.org> (2009-11-18) (inbox unread)
diff --git a/test/emacs.expected-output/notmuch-show-message-with-headers-visible b/test/emacs.expected-output/notmuch-show-message-with-headers-visible
new file mode 100644
index 0000000..8efbd60
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-show-message-with-headers-visible
@@ -0,0 +1,25 @@
+Jan Janak <jan@ryngle.com> (2009-11-17) (inbox unread)
+Subject: [notmuch] What a great idea!
+ Jan Janak <jan@ryngle.com> (2009-11-17) (inbox)
+ Subject: [notmuch] What a great idea!
+ To: notmuch@notmuchmail.org
+ Date: Tue, 17 Nov 2009 23:38:47 +0100
+
+ On Tue, Nov 17, 2009 at 11:35 PM, Jan Janak <jan at ryngle.com> wrote:
+ > Hello,
+ >
+ > First of all, notmuch is a wonderful idea, both the cmdline tool and
+ [ 2 more citation lines. Click/Enter to show. ]
+ >
+ > Have you considered sending an announcement to the org-mode mailing list?
+ > http://org-mode.org
+
+ Sorry, wrong URL, the correct one is: http://orgmode.org
+
+ > Various ways of searching/referencing emails from emacs were discussed
+ > there several times and none of them were as elegant as notmuch (not
+ > even close). Maybe notmuch would attract some of the developers
+ > there..
+
+   -- Jan
+ Carl Worth <cworth@cworth.org> (2009-11-18) (inbox unread)
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 2/7] test: emacs: new tests "notmuch-show: {, un}collapse all messages in thread"
  2012-01-25  5:25 [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility Pieter Praet
                   ` (3 preceding siblings ...)
  2012-02-22 18:43 ` [PATCH v2 1/7] test: emacs: new tests "notmuch-show: {show, hide} message headers" Pieter Praet
@ 2012-02-22 18:43 ` Pieter Praet
  2012-02-22 18:43 ` [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers' Pieter Praet
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Pieter Praet @ 2012-02-22 18:43 UTC (permalink / raw)
  To: Notmuch Mail

* test/emacs:

  - New subtest "notmuch-show: collapse all messages in thread":
    `notmuch-show-open-or-close-all' with prefix arg ("C-u M-RET")
    collapses all messages in thread.

  - New subtest "notmuch-show: uncollapse all messages in thread":
    `notmuch-show-open-or-close-all' without prefix arg ("M-RET")
    uncollapses all messages in thread.
---
 test/emacs                                         |   13 +++
 ...notmuch-show-thread-with-all-messages-collapsed |    4 +
 ...tmuch-show-thread-with-all-messages-uncollapsed |   79 ++++++++++++++++++++
 3 files changed, 96 insertions(+), 0 deletions(-)
 create mode 100644 test/emacs.expected-output/notmuch-show-thread-with-all-messages-collapsed
 create mode 100644 test/emacs.expected-output/notmuch-show-thread-with-all-messages-uncollapsed

diff --git a/test/emacs b/test/emacs
index f9ea1c3..7d6e6ee 100755
--- a/test/emacs
+++ b/test/emacs
@@ -408,6 +408,19 @@ test_emacs \
 	   (test-visible-output))'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-message-with-headers-hidden
 
+test_begin_subtest "notmuch-show: collapse all messages in thread"
+test_emacs '(notmuch-show "id:f35dbb950911171435ieecd458o853c873e35f4be95@mail.gmail.com")
+	(let ((current-prefix-arg t))
+	  (notmuch-show-open-or-close-all)
+	  (test-visible-output))'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-all-messages-collapsed
+
+test_begin_subtest "notmuch-show: uncollapse all messages in thread"
+test_emacs '(notmuch-show "id:f35dbb950911171435ieecd458o853c873e35f4be95@mail.gmail.com")
+	(notmuch-show-open-or-close-all)
+	(test-visible-output)'
+test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-all-messages-uncollapsed
+
 test_begin_subtest "Stashing in notmuch-show"
 add_message '[date]="Sat, 01 Jan 2000 12:00:00 -0000"' \
     '[from]="Some One <someone@somewhere.org>"' \
diff --git a/test/emacs.expected-output/notmuch-show-thread-with-all-messages-collapsed b/test/emacs.expected-output/notmuch-show-thread-with-all-messages-collapsed
new file mode 100644
index 0000000..73b0e60
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-show-thread-with-all-messages-collapsed
@@ -0,0 +1,4 @@
+Jan Janak <jan@ryngle.com> (2009-11-17) (inbox)
+Subject: [notmuch] What a great idea!
+ Jan Janak <jan@ryngle.com> (2009-11-17) (inbox)
+ Carl Worth <cworth@cworth.org> (2009-11-18) (inbox unread)
diff --git a/test/emacs.expected-output/notmuch-show-thread-with-all-messages-uncollapsed b/test/emacs.expected-output/notmuch-show-thread-with-all-messages-uncollapsed
new file mode 100644
index 0000000..bd5598e
--- /dev/null
+++ b/test/emacs.expected-output/notmuch-show-thread-with-all-messages-uncollapsed
@@ -0,0 +1,79 @@
+Jan Janak <jan@ryngle.com> (2009-11-17) (inbox)
+Subject: [notmuch] What a great idea!
+To: notmuch@notmuchmail.org
+Date: Tue, 17 Nov 2009 23:35:30 +0100
+
+Hello,
+
+First of all, notmuch is a wonderful idea, both the cmdline tool and
+the emacs interface! Thanks a lot for writing it, I was really excited
+when I read the announcement today.
+
+Have you considered sending an announcement to the org-mode mailing list?
+http://org-mode.org
+
+Various ways of searching/referencing emails from emacs were discussed
+there several times and none of them were as elegant as notmuch (not
+even close). Maybe notmuch would attract some of the developers
+there..
+
+   -- Jan
+ Jan Janak <jan@ryngle.com> (2009-11-17) (inbox)
+ Subject: [notmuch] What a great idea!
+ To: notmuch@notmuchmail.org
+ Date: Tue, 17 Nov 2009 23:38:47 +0100
+
+ On Tue, Nov 17, 2009 at 11:35 PM, Jan Janak <jan at ryngle.com> wrote:
+ > Hello,
+ >
+ > First of all, notmuch is a wonderful idea, both the cmdline tool and
+ [ 2 more citation lines. Click/Enter to show. ]
+ >
+ > Have you considered sending an announcement to the org-mode mailing list?
+ > http://org-mode.org
+
+ Sorry, wrong URL, the correct one is: http://orgmode.org
+
+ > Various ways of searching/referencing emails from emacs were discussed
+ > there several times and none of them were as elegant as notmuch (not
+ > even close). Maybe notmuch would attract some of the developers
+ > there..
+
+   -- Jan
+ Carl Worth <cworth@cworth.org> (2009-11-18) (inbox unread)
+ Subject: [notmuch] What a great idea!
+ To: notmuch@notmuchmail.org
+ Date: Wed, 18 Nov 2009 02:49:52 -0800
+
+ On Tue, 17 Nov 2009 23:35:30 +0100, Jan Janak <jan at ryngle.com> wrote:
+ > First of all, notmuch is a wonderful idea, both the cmdline tool and
+ > the emacs interface! Thanks a lot for writing it, I was really excited
+ > when I read the announcement today.
+
+ Ah, here's where I planned a nice welcome. So welcome (again), Jan! :-)
+
+ I've been having a lot of fun with notmuch already, (though there have
+ been some days of pain before it was functional enough and my
+ email-reply latency went way up). But regardless---I got through that,
+ and I'm able to work more efficiently with notmuch now than I could with
+ sup before. So I'm happy.
+
+ And I'm delighted when other people find this interesting as well.
+
+ > Have you considered sending an announcement to the org-mode mailing list?
+ > http://orgmode.org
+
+ Thanks for the idea. I think I may have looked into org-mode years ago,
+ (when I was investigating planner-mode and various emacs "personal wiki"
+ systems for keeping random notes and what-not).
+
+ > Various ways of searching/referencing emails from emacs were discussed
+ > there several times and none of them were as elegant as notmuch (not
+ > even close). Maybe notmuch would attract some of the developers
+ > there..
+
+ Yeah. I'll drop them a mail. Having a real emacs wizard on board would
+ be nice. (I'm afraid the elisp I've written so far for this project is
+ fairly grim.)
+
+ -Carl
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'
  2012-01-25  5:25 [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility Pieter Praet
                   ` (4 preceding siblings ...)
  2012-02-22 18:43 ` [PATCH v2 2/7] test: emacs: new tests "notmuch-show: {, un}collapse all messages in thread" Pieter Praet
@ 2012-02-22 18:43 ` Pieter Praet
  2012-10-12 21:36   ` Ethan Glasser-Camp
  2012-02-22 18:43 ` [PATCH v2 4/7] emacs: rename `notmuch-show-toggle-message' to `notmuch-show-toggle-visibility-message' Pieter Praet
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Pieter Praet @ 2012-02-22 18:43 UTC (permalink / raw)
  To: Notmuch Mail

* emacs/notmuch-show.el

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

  (notmuch-show-mode-map):
    Update "h" binding wrt renamed `notmuch-show-toggle-headers'.

  (notmuch-message-headers):
    Update docstring wrt renamed `notmuch-show-toggle-headers'.

  (notmuch-message-headers-visible):
    Update docstring wrt renamed `notmuch-show-toggle-headers'.
    Also fixed a small typo.

* test/emacs:

  Update subtest wrt renamed `notmuch-show-toggle-headers':
  - "notmuch-show: hide message headers (w/ notmuch-show-toggle-headers)"
---
 emacs/notmuch-show.el |   12 ++++++------
 test/emacs            |    4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index aa9ccee..861018d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -48,8 +48,8 @@ (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
 
 For an open message, all of these headers will be made visible
 according to `notmuch-message-headers-visible' or can be toggled
-with `notmuch-show-toggle-headers'. For a closed message, only
-the first header in the list will be visible."
+with `notmuch-show-toggle-visibility-headers'. For a closed message,
+only the first header in the list will be visible."
   :type '(repeat string)
   :group 'notmuch-show)
 
@@ -59,8 +59,8 @@ (defcustom notmuch-message-headers-visible t
 If this value is non-nil, then all of the headers defined in
 `notmuch-message-headers' will be visible by default in the display
 of each message. Otherwise, these headers will be hidden and
-`notmuch-show-toggle-headers' can be used to make the visible for
-any given message."
+`notmuch-show-toggle-visibility-headers' can be used to make them
+visible for any given message."
   :type 'boolean
   :group 'notmuch-show)
 
@@ -1168,7 +1168,7 @@ (defvar notmuch-show-mode-map
 	(define-key map "v" 'notmuch-show-view-all-mime-parts)
 	(define-key map "c" 'notmuch-show-stash-map)
 	(define-key map "=" 'notmuch-show-refresh-view)
-	(define-key map "h" 'notmuch-show-toggle-headers)
+	(define-key map "h" 'notmuch-show-toggle-visibility-headers)
 	(define-key map "*" 'notmuch-show-tag-all)
 	(define-key map "-" 'notmuch-show-remove-tag)
 	(define-key map "+" 'notmuch-show-add-tag)
@@ -1650,7 +1650,7 @@ (defun notmuch-show-remove-tag ()
   (interactive)
   (notmuch-show-tag "-"))
 
-(defun notmuch-show-toggle-headers ()
+(defun notmuch-show-toggle-visibility-headers ()
   "Toggle the visibility of the current message headers."
   (interactive)
   (let ((props (notmuch-show-get-message-properties)))
diff --git a/test/emacs b/test/emacs
index 7d6e6ee..f17efbd 100755
--- a/test/emacs
+++ b/test/emacs
@@ -399,12 +399,12 @@ test_emacs \
 	   (test-visible-output))'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-message-with-headers-hidden
 
-test_begin_subtest "notmuch-show: hide message headers (w/ notmuch-show-toggle-headers)"
+test_begin_subtest "notmuch-show: hide message headers (w/ notmuch-show-toggle-visibility-headers)"
 test_emacs \
 	'(let ((notmuch-message-headers '\''("Subject" "To" "Cc" "Date"))
 	       (notmuch-message-headers-visible t))
 	   (notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
-	   (notmuch-show-toggle-headers)
+	   (notmuch-show-toggle-visibility-headers)
 	   (test-visible-output))'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-message-with-headers-hidden
 
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 4/7] emacs: rename `notmuch-show-toggle-message' to `notmuch-show-toggle-visibility-message'
  2012-01-25  5:25 [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility Pieter Praet
                   ` (5 preceding siblings ...)
  2012-02-22 18:43 ` [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers' Pieter Praet
@ 2012-02-22 18:43 ` Pieter Praet
  2012-02-22 18:43 ` [PATCH v2 5/7] emacs: rename `notmuch-show-open-or-close-all' to `notmuch-show-toggle-visibility-messages' Pieter Praet
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Pieter Praet @ 2012-02-22 18:43 UTC (permalink / raw)
  To: Notmuch Mail

* emacs/notmuch-show.el

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

  (notmuch-show-mode-map):
    Update "RET" binding wrt renamed `notmuch-show-toggle-message'.

* test/emacs:

  Update subtests wrt renamed `notmuch-show-toggle-message':
  - "Hiding message in notmuch-show view"
  - "Hiding message with visible citation in notmuch-show view"
  - "Refresh modified show buffer"
---
 emacs/notmuch-show.el |    4 ++--
 test/emacs            |    8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 861018d..9b879c7 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1183,7 +1183,7 @@ (defvar notmuch-show-mode-map
 	(define-key map (kbd "DEL") 'notmuch-show-rewind)
 	(define-key map " " 'notmuch-show-advance-and-archive)
 	(define-key map (kbd "M-RET") 'notmuch-show-open-or-close-all)
-	(define-key map (kbd "RET") 'notmuch-show-toggle-message)
+	(define-key map (kbd "RET") 'notmuch-show-toggle-visibility-message)
 	(define-key map "#" 'notmuch-show-print-message)
 	(define-key map "!" 'notmuch-show-toggle-elide-non-matching)
 	(define-key map "$" 'notmuch-show-toggle-process-crypto)
@@ -1659,7 +1659,7 @@ (defun notmuch-show-toggle-visibility-headers ()
      (not (plist-get props :headers-visible))))
   (force-window-update))
 
-(defun notmuch-show-toggle-message ()
+(defun notmuch-show-toggle-visibility-message ()
   "Toggle the visibility of the current message."
   (interactive)
   (let ((props (notmuch-show-get-message-properties)))
diff --git a/test/emacs b/test/emacs
index f17efbd..6f14895 100755
--- a/test/emacs
+++ b/test/emacs
@@ -371,7 +371,7 @@ test_expect_equal_file OUTPUT EXPECTED
 
 test_begin_subtest "Hiding message in notmuch-show view"
 test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
-	    (notmuch-show-toggle-message)
+	    (notmuch-show-toggle-visibility-message)
 	    (test-visible-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
 
@@ -379,7 +379,7 @@ test_begin_subtest "Hiding message with visible citation in notmuch-show view"
 test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
 	    (search-forward "Click/Enter to show.")
 	    (button-activate (button-at (point)))
-	    (notmuch-show-toggle-message)
+	    (notmuch-show-toggle-visibility-message)
 	    (test-visible-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
 
@@ -496,9 +496,9 @@ test_expect_equal_file OUTPUT EXPECTED
 test_begin_subtest "Refresh modified show buffer"
 test_subtest_known_broken
 test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
-	    (notmuch-show-toggle-message)
+	    (notmuch-show-toggle-visibility-message)
 	    (notmuch-show-next-message)
-	    (notmuch-show-toggle-message)
+	    (notmuch-show-toggle-visibility-message)
 	    (test-visible-output "EXPECTED")
 	    (notmuch-show-refresh-view)
 	    (test-visible-output)'
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 5/7] emacs: rename `notmuch-show-open-or-close-all' to `notmuch-show-toggle-visibility-messages'
  2012-01-25  5:25 [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility Pieter Praet
                   ` (6 preceding siblings ...)
  2012-02-22 18:43 ` [PATCH v2 4/7] emacs: rename `notmuch-show-toggle-message' to `notmuch-show-toggle-visibility-message' Pieter Praet
@ 2012-02-22 18:43 ` Pieter Praet
  2012-02-22 18:43 ` [PATCH v2 6/7] emacs: make `notmuch-show-toggle-visibility-messages' live up to its new name Pieter Praet
  2012-02-22 18:43 ` [PATCH v2 7/7] emacs: `notmuch-show-toggle-visibility-messages' with prefix arg filters by tag Pieter Praet
  9 siblings, 0 replies; 22+ messages in thread
From: Pieter Praet @ 2012-02-22 18:43 UTC (permalink / raw)
  To: Notmuch Mail

* emacs/notmuch-show.el

  (notmuch-show-open-or-close-all):
    Rename to `notmuch-show-toggle-visibility-messages'.

  (notmuch-show-mode-map):
    Update "M-RET" binding wrt renamed `notmuch-show-open-or-close-all'.

* test/emacs:

  Update subtests wrt renamed `notmuch-show-open-or-close-all':
  - "notmuch-show: collapse all messages in thread"
  - "notmuch-show: uncollapse all messages in thread"
---
 emacs/notmuch-show.el |    4 ++--
 test/emacs            |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 9b879c7..e4d1f9c 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1182,8 +1182,8 @@ (defvar notmuch-show-mode-map
 	(define-key map "p" 'notmuch-show-previous-open-message)
 	(define-key map (kbd "DEL") 'notmuch-show-rewind)
 	(define-key map " " 'notmuch-show-advance-and-archive)
-	(define-key map (kbd "M-RET") 'notmuch-show-open-or-close-all)
 	(define-key map (kbd "RET") 'notmuch-show-toggle-visibility-message)
+	(define-key map (kbd "M-RET") 'notmuch-show-toggle-visibility-messages)
 	(define-key map "#" 'notmuch-show-print-message)
 	(define-key map "!" 'notmuch-show-toggle-elide-non-matching)
 	(define-key map "$" 'notmuch-show-toggle-process-crypto)
@@ -1668,7 +1668,7 @@ (defun notmuch-show-toggle-visibility-message ()
      (not (plist-get props :message-visible))))
   (force-window-update))
 
-(defun notmuch-show-open-or-close-all ()
+(defun notmuch-show-toggle-visibility-messages ()
   "Set the visibility all of the messages in the current thread.
 By default make all of the messages visible. With a prefix
 argument, hide all of the messages."
diff --git a/test/emacs b/test/emacs
index 6f14895..29fdec0 100755
--- a/test/emacs
+++ b/test/emacs
@@ -411,13 +411,13 @@ test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-message-with-headers-hidden
 test_begin_subtest "notmuch-show: collapse all messages in thread"
 test_emacs '(notmuch-show "id:f35dbb950911171435ieecd458o853c873e35f4be95@mail.gmail.com")
 	(let ((current-prefix-arg t))
-	  (notmuch-show-open-or-close-all)
+	  (notmuch-show-toggle-visibility-messages)
 	  (test-visible-output))'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-all-messages-collapsed
 
 test_begin_subtest "notmuch-show: uncollapse all messages in thread"
 test_emacs '(notmuch-show "id:f35dbb950911171435ieecd458o853c873e35f4be95@mail.gmail.com")
-	(notmuch-show-open-or-close-all)
+	(notmuch-show-toggle-visibility-messages)
 	(test-visible-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-all-messages-uncollapsed
 
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 6/7] emacs: make `notmuch-show-toggle-visibility-messages' live up to its new name
  2012-01-25  5:25 [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility Pieter Praet
                   ` (7 preceding siblings ...)
  2012-02-22 18:43 ` [PATCH v2 5/7] emacs: rename `notmuch-show-open-or-close-all' to `notmuch-show-toggle-visibility-messages' Pieter Praet
@ 2012-02-22 18:43 ` Pieter Praet
  2012-02-22 18:43 ` [PATCH v2 7/7] emacs: `notmuch-show-toggle-visibility-messages' with prefix arg filters by tag Pieter Praet
  9 siblings, 0 replies; 22+ messages in thread
From: Pieter Praet @ 2012-02-22 18:43 UTC (permalink / raw)
  To: Notmuch Mail

* emacs/notmuch-show.el (notmuch-show-toggle-visibility-messages):
  Toggle visibility of all messages in current thread based on visibility
  of the current message, instead of setting visibility based on whether
  or not a prefix arg was supplied.

  Also move current buffer line to the 2nd window line so the current
  message is put properly into focus, whilst making the presence of
  previous messages in the thread obvious.

Same functionality, less effort (reaching for 'C-u' is a pain)...
---
 emacs/notmuch-show.el |   20 ++++++++++++--------
 test/emacs            |    6 +++---
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index e4d1f9c..82d4265 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1669,15 +1669,19 @@ (defun notmuch-show-toggle-visibility-message ()
   (force-window-update))
 
 (defun notmuch-show-toggle-visibility-messages ()
-  "Set the visibility all of the messages in the current thread.
-By default make all of the messages visible. With a prefix
-argument, hide all of the messages."
+  "Toggle the visibility of all messages in the current thread.
+If the current message is visible, hide all messages -- and vice versa."
   (interactive)
-  (save-excursion
-    (goto-char (point-min))
-    (loop do (notmuch-show-message-visible (notmuch-show-get-message-properties)
-					   (not current-prefix-arg))
-	  until (not (notmuch-show-goto-message-next))))
+  (let ((visible-p (notmuch-show-message-visible-p)))
+    (notmuch-show-mapc
+     (lambda () (notmuch-show-message-visible
+		 (notmuch-show-get-message-properties)
+		 (not visible-p)))))
+
+  ;; Put the current message properly into focus, but don't
+  ;; obscure the presence of previous messages in the thread.
+  (recenter-top-bottom 1)
+
   (force-window-update))
 
 (defun notmuch-show-next-button ()
diff --git a/test/emacs b/test/emacs
index 29fdec0..5c61743 100755
--- a/test/emacs
+++ b/test/emacs
@@ -410,14 +410,14 @@ test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-message-with-headers-hidden
 
 test_begin_subtest "notmuch-show: collapse all messages in thread"
 test_emacs '(notmuch-show "id:f35dbb950911171435ieecd458o853c873e35f4be95@mail.gmail.com")
-	(let ((current-prefix-arg t))
-	  (notmuch-show-toggle-visibility-messages)
-	  (test-visible-output))'
+	(notmuch-show-toggle-visibility-messages)
+	(test-visible-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-all-messages-collapsed
 
 test_begin_subtest "notmuch-show: uncollapse all messages in thread"
 test_emacs '(notmuch-show "id:f35dbb950911171435ieecd458o853c873e35f4be95@mail.gmail.com")
 	(notmuch-show-toggle-visibility-messages)
+	(notmuch-show-toggle-visibility-messages)
 	(test-visible-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-all-messages-uncollapsed
 
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 7/7] emacs: `notmuch-show-toggle-visibility-messages' with prefix arg filters by tag
  2012-01-25  5:25 [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility Pieter Praet
                   ` (8 preceding siblings ...)
  2012-02-22 18:43 ` [PATCH v2 6/7] emacs: make `notmuch-show-toggle-visibility-messages' live up to its new name Pieter Praet
@ 2012-02-22 18:43 ` Pieter Praet
  9 siblings, 0 replies; 22+ messages in thread
From: Pieter Praet @ 2012-02-22 18:43 UTC (permalink / raw)
  To: Notmuch Mail

* emacs/notmuch-show.el (notmuch-show-toggle-visibility-messages):
  When provided with a prefix arg, prompt the user for a tag.
  Show all messages that have it and hide those that don't.
---
 emacs/notmuch-show.el |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 82d4265..b5e482b 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -38,6 +38,7 @@
 
 (declare-function notmuch-call-notmuch-process "notmuch" (&rest args))
 (declare-function notmuch-fontify-headers "notmuch" nil)
+(declare-function notmuch-select-tag-with-completion "notmuch" (prompt &rest search-terms))
 (declare-function notmuch-read-tag-changes "notmuch" (&optional initial-input &rest search-terms))
 (declare-function notmuch-search-next-thread "notmuch" nil)
 (declare-function notmuch-search-show-thread "notmuch" nil)
@@ -1670,13 +1671,19 @@ (defun notmuch-show-toggle-visibility-message ()
 
 (defun notmuch-show-toggle-visibility-messages ()
   "Toggle the visibility of all messages in the current thread.
-If the current message is visible, hide all messages -- and vice versa."
+If the current message is visible, hide all messages -- and vice versa.
+
+With a prefix argument, prompt for a tag and only show messages which have it."
   (interactive)
-  (let ((visible-p (notmuch-show-message-visible-p)))
+  (let ((visible-p (notmuch-show-message-visible-p))
+	(filter (if current-prefix-arg
+		    (notmuch-select-tag-with-completion "Filter by tag: "))))
     (notmuch-show-mapc
      (lambda () (notmuch-show-message-visible
 		 (notmuch-show-get-message-properties)
-		 (not visible-p)))))
+		 (if current-prefix-arg
+		     (member filter (notmuch-show-get-tags))
+		   (not visible-p))))))
 
   ;; Put the current message properly into focus, but don't
   ;; obscure the presence of previous messages in the thread.
-- 
1.7.8.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'
  2012-02-22 18:43 ` [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers' Pieter Praet
@ 2012-10-12 21:36   ` Ethan Glasser-Camp
  2012-10-18  0:29     ` Ethan Glasser-Camp
  0 siblings, 1 reply; 22+ messages in thread
From: Ethan Glasser-Camp @ 2012-10-12 21:36 UTC (permalink / raw)
  To: Pieter Praet, Notmuch Mail

Pieter Praet <pieter@praet.org> writes:

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

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

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

Ethan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'
  2012-10-12 21:36   ` Ethan Glasser-Camp
@ 2012-10-18  0:29     ` Ethan Glasser-Camp
  2012-10-18  9:47       ` Tomi Ollila
  2012-10-18  9:50       ` Tomi Ollila
  0 siblings, 2 replies; 22+ messages in thread
From: Ethan Glasser-Camp @ 2012-10-18  0:29 UTC (permalink / raw)
  To: Ethan Glasser-Camp, Pieter Praet, Notmuch Mail

Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> writes:

> This patch, and its predecessors, all look great to me.

But a note: many of the first lines in your commit messages ("{show,
hide} message headers") contain tabs. I hate tabs. Is this intentional?
I have noticed it on other patches you've sent (such as
id:"1330122640-18895-6-git-send-email-pieter@praet.org" and
id:"1330122640-18895-7-git-send-email-pieter@praet.org").

Ethan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'
  2012-10-18  0:29     ` Ethan Glasser-Camp
@ 2012-10-18  9:47       ` Tomi Ollila
  2012-10-18  9:50       ` Tomi Ollila
  1 sibling, 0 replies; 22+ messages in thread
From: Tomi Ollila @ 2012-10-18  9:47 UTC (permalink / raw)
  To: Ethan Glasser-Camp, Ethan Glasser-Camp, Pieter Praet,
	Notmuch Mail

On Thu, Oct 18 2012, Ethan Glasser-Camp wrote:

> Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> writes:
>
>> This patch, and its predecessors, all look great to me.
>
> But a note: many of the first lines in your commit messages ("{show,
> hide} message headers") contain tabs. I hate tabs. Is this intentional?
> I have noticed it on other patches you've sent (such as
> id:"1330122640-18895-6-git-send-email-pieter@praet.org" and
> id:"1330122640-18895-7-git-send-email-pieter@praet.org").

I also see tabs, but when I press '<' (remove indent) then
those tabs go away... I guess emacs (or notmuch indentation code)
uses tabs when doing message indenting ?

> Ethan

Tomi

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'
  2012-10-18  0:29     ` Ethan Glasser-Camp
  2012-10-18  9:47       ` Tomi Ollila
@ 2012-10-18  9:50       ` Tomi Ollila
  2012-10-18 13:37         ` Ethan
  1 sibling, 1 reply; 22+ messages in thread
From: Tomi Ollila @ 2012-10-18  9:50 UTC (permalink / raw)
  To: Ethan Glasser-Camp, Ethan Glasser-Camp, Pieter Praet,
	Notmuch Mail

On Thu, Oct 18 2012, Ethan Glasser-Camp wrote:

> Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> writes:
>
>> This patch, and its predecessors, all look great to me.
>
> But a note: many of the first lines in your commit messages ("{show,

Hmm, first lines -- IIRC mailman adds those tabs to the subject line -- and
those are converted to spaces in git-am (??)
>
> Ethan

Tomi

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'
  2012-10-18  9:50       ` Tomi Ollila
@ 2012-10-18 13:37         ` Ethan
  2012-10-18 13:59           ` Tomi Ollila
  0 siblings, 1 reply; 22+ messages in thread
From: Ethan @ 2012-10-18 13:37 UTC (permalink / raw)
  To: Tomi Ollila; +Cc: Notmuch Mail, Pieter Praet

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]

On Thu, Oct 18, 2012 at 5:50 AM, Tomi Ollila <tomi.ollila@iki.fi> wrote:

> On Thu, Oct 18 2012, Ethan Glasser-Camp wrote:
>
> > Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> writes:
> >
> >> This patch, and its predecessors, all look great to me.
> >
> > But a note: many of the first lines in your commit messages ("{show,
>
> Hmm, first lines -- IIRC mailman adds those tabs to the subject line -- and
> those are converted to spaces in git-am (??)
>

I guess this is called "line folding" and notmuch should be sure to undo
it, as I guess git-am already does.

https://bugs.launchpad.net/mailman/+bug/265915

I guess I can stop complaining about it ;) Patches 1-3 are probably ready
then. Thanks, Tomi.

Ethan

[-- Attachment #2: Type: text/html, Size: 1218 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers'
  2012-10-18 13:37         ` Ethan
@ 2012-10-18 13:59           ` Tomi Ollila
  0 siblings, 0 replies; 22+ messages in thread
From: Tomi Ollila @ 2012-10-18 13:59 UTC (permalink / raw)
  To: Ethan; +Cc: Notmuch Mail, Pieter Praet

On Thu, Oct 18 2012, Ethan wrote:

> On Thu, Oct 18, 2012 at 5:50 AM, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>
>> On Thu, Oct 18 2012, Ethan Glasser-Camp wrote:
>>
>> > Ethan Glasser-Camp <ethan.glasser.camp@gmail.com> writes:
>> >
>> >> This patch, and its predecessors, all look great to me.
>> >
>> > But a note: many of the first lines in your commit messages ("{show,
>>
>> Hmm, first lines -- IIRC mailman adds those tabs to the subject line -- and
>> those are converted to spaces in git-am (??)
>>
>
> I guess this is called "line folding" and notmuch should be sure to undo
> it, as I guess git-am already does.
>
> https://bugs.launchpad.net/mailman/+bug/265915
>
> I guess I can stop complaining about it ;) Patches 1-3 are probably ready
> then. Thanks, Tomi.

Np. Tests pass and the function name change is trivial. 
Removed needs-review on these 3 patches.

> Ethan

Tomi

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/7] test: emacs: new tests "notmuch-show: {show, hide} message headers"
  2012-02-22 18:43 ` [PATCH v2 1/7] test: emacs: new tests "notmuch-show: {show, hide} message headers" Pieter Praet
@ 2012-10-20 20:40   ` David Bremner
  0 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2012-10-20 20:40 UTC (permalink / raw)
  To: Pieter Praet, Notmuch Mail

Pieter Praet <pieter@praet.org> writes:

> * test/emacs:
>
>   - New subtest "notmuch-show: show message headers":
>   - New subtest "notmuch-show: hide message headers":
>   - New subtest "notmuch-show: hide message headers (w/
>     notmuch-show-toggle-head
 
pushed (with Ethan's edit)

d

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2012-10-20 20:41 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-25  5:25 [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility Pieter Praet
2012-01-25  6:35 ` David Edmondson
2012-01-25 13:03   ` Tomi Ollila
2012-01-26 13:02   ` Pieter Praet
2012-01-26 15:04     ` David Edmondson
2012-01-26 15:58 ` Tomi Ollila
2012-02-13 10:51 ` Dmitry Kurochkin
2012-02-22 18:41   ` Pieter Praet
2012-02-22 18:43 ` [PATCH v2 1/7] test: emacs: new tests "notmuch-show: {show, hide} message headers" Pieter Praet
2012-10-20 20:40   ` David Bremner
2012-02-22 18:43 ` [PATCH v2 2/7] test: emacs: new tests "notmuch-show: {, un}collapse all messages in thread" Pieter Praet
2012-02-22 18:43 ` [PATCH v2 3/7] emacs: rename `notmuch-show-toggle-headers' to `notmuch-show-toggle-visibility-headers' Pieter Praet
2012-10-12 21:36   ` Ethan Glasser-Camp
2012-10-18  0:29     ` Ethan Glasser-Camp
2012-10-18  9:47       ` Tomi Ollila
2012-10-18  9:50       ` Tomi Ollila
2012-10-18 13:37         ` Ethan
2012-10-18 13:59           ` Tomi Ollila
2012-02-22 18:43 ` [PATCH v2 4/7] emacs: rename `notmuch-show-toggle-message' to `notmuch-show-toggle-visibility-message' Pieter Praet
2012-02-22 18:43 ` [PATCH v2 5/7] emacs: rename `notmuch-show-open-or-close-all' to `notmuch-show-toggle-visibility-messages' Pieter Praet
2012-02-22 18:43 ` [PATCH v2 6/7] emacs: make `notmuch-show-toggle-visibility-messages' live up to its new name Pieter Praet
2012-02-22 18:43 ` [PATCH v2 7/7] emacs: `notmuch-show-toggle-visibility-messages' with prefix arg filters by tag Pieter Praet

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).