From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH] emacs: Make moving to the previous message move to the
	previous boundary
In-Reply-To: <1342237668-2489-1-git-send-email-amdragon@mit.edu>
References: <1342237668-2489-1-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.13.2+90~g84fa1ef (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Wed, 18 Jul 2012 22:54:48 +0100
Message-ID: <87sjcod9av.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Sat, 14 Jul 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> Previously, notmuch-show-previous-message would move to the beginning
> of the message before the message containing point.  This patch makes
> it instead move to the previous message *boundary*.  That is, if point
> isn't already at the beginning of the message, it moves to the
> beginning of the current message.  This is consistent with
> notmuch-show-next-message, which can be thought of as moving to the
> next message boundary.  Several people have expressed a preference for
> this.

I like this change: the current behaviour has mildly confused/annoyed me
in the past.

>
> This patch accompanies the series in [0] (though they're independent
> and can be applied in either order).  This makes the behavior of 'p'
> and 'P' in show-mode conceptually similar to the new behavior of 'p'
> in search-mode.
>
> [0] 1342140319-19859-1-git-send-email-amdragon@mit.edu
>
>  emacs/notmuch-show.el |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 6335d45..02e319f 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1525,9 +1525,11 @@ thread, navigate to the next thread in the parent search buffer."
>        (goto-char (point-max)))))
>  
>  (defun notmuch-show-previous-message ()
> -  "Show the previous message."
> +  "Show the previous message or the start of the current message."
>    (interactive)
> -  (notmuch-show-goto-message-previous)
> +  (if (= (point) (notmuch-show-message-top))
> +      (notmuch-show-goto-message-previous)
> +    (notmuch-show-move-to-message-top))
>    (notmuch-show-mark-read)
>    (notmuch-show-message-adjust))
>  
> @@ -1587,7 +1589,9 @@ to show, nil otherwise."
>  (defun notmuch-show-previous-open-message ()
>    "Show the previous open message."
>    (interactive)
> -  (while (and (notmuch-show-goto-message-previous)
> +  (while (and (if (= (point) (notmuch-show-message-top))
> +		  (notmuch-show-goto-message-previous)
> +		(notmuch-show-move-to-message-top))
>  	      (not (notmuch-show-message-visible-p))))
>    (notmuch-show-mark-read)
>    (notmuch-show-message-adjust))

I would mildly prefer taking the testing for top of message out of the
loop (I know it's one line longer) as

  (if (= (point) (notmuch-show-message-top))
      (notmuch-show-goto-message-previous)
    (notmuch-show-move-to-message-top))
  (while (and (not (notmuch-show-message-visible-p))
	      (notmuch-show-goto-message-previous)))

but its obviously fine as is.

Best wishes

Mark