Re: [RFC][PATCH v4] emacs: Re-implement advance/rewind functions of notmuch-show-mode.

Subject: Re: [RFC][PATCH v4] emacs: Re-implement advance/rewind functions of notmuch-show-mode.

Date: Mon, 26 Dec 2011 10:54:14 +0000

To: Aaron Ecay, Austin Clements

Cc: notmuch@notmuchmail.org

From: David Edmondson


On Sun, 25 Dec 2011 23:11:27 -0500, Aaron Ecay <aaronecay@gmail.com> wrote:
> > > +   ((> (let ((visible-bottom (notmuch-show-message-bottom)))
> > > +	 (while (invisible-p visible-bottom)
> > > +	   (setq visible-bottom (max (point-min)
> > > +				     (1- (previous-single-char-property-change
> > > +					  visible-bottom 'invisible)))))
> > > +	 visible-bottom) (window-end))
> 
> Can this (let...) be lifted out of the (cond...)?  IMO it is very
> confusing to be doing non-trivial computation in the test portion of a
> cond form.

It ends up a long way from where it's used, diluting the value of the
comment. I do like the current layout, but what if it was (something
like):

   ((let ((visible-bottom (1- (notmuch-show-message-bottom))))
      (while (invisible-p visible-bottom)
	(setq visible-bottom (max (point-min)
				  (1- (previous-single-char-property-change
				       visible-bottom 'invisible)))))
      (> visible-bottom (window-end)))
    ;; The end of this message is not visible - scroll to show more of
    ;; it.
    (scroll-up)
    nil)

That would seem more palatable, perhaps.

> Agreed.  I would like to see this case move back one screenful of text or
> to the previous beginning-of-message, whichever is shorter.

See previous comment - I agreed that it's not symmetric - just wonder
which is more useful behaviour.
part-000.sig (application/pgp-signature)

Thread: