[PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Pieter Praet]

On Mon, 13 Feb 2012 14:51:17 +0400, Dmitry Kurochkin  wrote:
> Hi Pieter.
> 
> On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet  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 

Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Pieter Praet]

On Mon, 13 Feb 2012 14:51:17 +0400, Dmitry Kurochkin 
dmitry.kuroch...@gmail.com wrote:
 Hi Pieter.
 
 On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet pie...@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 

[PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Dmitry Kurochkin]

Hi Pieter.

On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet  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 at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Dmitry Kurochkin]

Hi Pieter.

On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet pie...@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
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Tomi Ollila]

+1

Tomi

[PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[David Edmondson]

On Thu, 26 Jan 2012 14:02:15 +0100, Pieter Praet  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).
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 

[PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Pieter Praet]

On Wed, 25 Jan 2012 06:35:33 +, David Edmondson  wrote:
> On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet  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 at notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch


Peace

-- 
Pieter

Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Pieter Praet]

On Wed, 25 Jan 2012 06:35:33 +, David Edmondson d...@dme.org wrote:
 On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet pie...@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
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[David Edmondson]

On Thu, 26 Jan 2012 14:02:15 +0100, Pieter Praet pie...@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).


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

Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Tomi Ollila]

+1

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

[PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Tomi Ollila]

On Wed, 25 Jan 2012 06:35:33 +, David Edmondson  wrote:
> On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet  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

[PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[David Edmondson]

On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet  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 at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 

[PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Pieter Praet]

* 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

Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Tomi Ollila]

On Wed, 25 Jan 2012 06:35:33 +, David Edmondson d...@dme.org wrote:
 On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet pie...@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
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[Pieter Praet]

* 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

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

Re: [PATCH] emacs: make `notmuch-show-open-or-close-all' toggle visibility

[David Edmondson]

On Wed, 25 Jan 2012 06:25:39 +0100, Pieter Praet pie...@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


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