Date: Fri, 13 Jul 2012 13:53:52 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Mark Walters <markwalters1009@gmail.com>
Subject: Re: [PATCH 7/7] emacs: Fix navigation of multi-line search result
	formats
Message-ID: <20120713175352.GN7332@mit.edu>
References: <1342140319-19859-1-git-send-email-amdragon@mit.edu>
	<1342140319-19859-8-git-send-email-amdragon@mit.edu>
	<871ukfshjd.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <871ukfshjd.fsf@qmul.ac.uk>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Mark Walters on Jul 13 at  6:21 pm:
> On Fri, 13 Jul 2012, Austin Clements <amdragon@MIT.EDU> wrote:
> > At this point, the only remaining functions that don't support
> > multi-line search result formats are the thread navigation functions.
> > This patch fixes that by rewriting them in terms of
> > notmuch-search-result-{beginning,end}.
> 
> This seems to subtly change the behaviour in the normal case of a single
> line result. If point is not at the start of a search result then
> notmuch-search-previous-thread moves to the start of the current thread
> rather than the previous thread. Is that deliberate?
> 
> As far as I can see show uses p to move to the start of the previous
> message so this behaviour is different from there as well. 
> 
> I think your choice may be the nicest one (and actually even more so in
> show) but thought I would mention it anyway.

Oh, yes, that's true.  I suppose that choice was deliberate in the
sense that I wrote notmuch-search-previous-thread the way I thought it
should work without thinking very hard about how it originally worked.

I could rework it to keep the original behavior, or keep it this way
and document it in the commit message (and maybe NEWS).  Personally I
would prefer to keep it this way and update show's p binding to behave
similarly since I've always found show's p to be confusing.

> Best wishes 
> 
> Mark
> 
> > ---
> >  emacs/notmuch.el |   12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> > index f32cfb0..2ece97d 100644
> > --- a/emacs/notmuch.el
> > +++ b/emacs/notmuch.el
> > @@ -287,18 +287,24 @@ For a mouse binding, return nil."
> >  (defun notmuch-search-next-thread ()
> >    "Select the next thread in the search results."
> >    (interactive)
> > -  (forward-line 1))
> > +  (when (notmuch-search-get-result (notmuch-search-result-end))
> > +    (goto-char (notmuch-search-result-end))))
> >  
> >  (defun notmuch-search-previous-thread ()
> >    "Select the previous thread in the search results."
> >    (interactive)
> > -  (forward-line -1))
> > +  (if (notmuch-search-get-result)
> > +      (unless (bobp)
> > +	(goto-char (notmuch-search-result-beginning (- (point) 1))))
> > +    ;; We must be past the end; jump to the last result
> > +    (notmuch-search-last-thread)))
> >  
> >  (defun notmuch-search-last-thread ()
> >    "Select the last thread in the search results."
> >    (interactive)
> >    (goto-char (point-max))
> > -  (forward-line -2))
> > +  (forward-line -2)
> > +  (goto-char (notmuch-search-result-beginning)))
> >  
> >  (defun notmuch-search-first-thread ()
> >    "Select the first thread in the search results."