[RFC][PATCH v3] emacs: Re-implement advance/rewind functions of notmuch-show-mode.
David Edmondson
dme at dme.org
Thu Dec 22 04:38:38 PST 2011
The advance/rewind functions had become complex, which made it hard to
determine who they are expected to behave. Re-implement them simply in
order to poll user-experience and expectation.
---
This one passes the test suite and, consequently, works better when
the last open message in a thread has trailing invisible text.
emacs/notmuch-show.el | 124 +++++++++++++++++++++++++-----------------------
1 files changed, 65 insertions(+), 59 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 46525aa..9fec499 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1156,38 +1156,49 @@ Some useful entries are:
;; Commands typically bound to keys.
(defun notmuch-show-advance ()
- "Advance through thread.
+ "Advance through the current thread.
-If the current message in the thread is not yet fully visible,
-scroll by a near screenful to read more of the message.
+Scroll the current message if the end of it is not visible,
+otherwise move to the next message.
-Otherwise, (the end of the current message is already within the
-current window), advance to the next open message."
+Return `t' if we are at the end of the last message, otherwise
+`nil'."
(interactive)
- (let* ((end-of-this-message (notmuch-show-message-bottom))
- (visible-end-of-this-message (1- end-of-this-message))
- (ret nil))
- (while (invisible-p visible-end-of-this-message)
- (setq visible-end-of-this-message
- (previous-single-char-property-change visible-end-of-this-message
- 'invisible)))
- (cond
- ;; Ideally we would test `end-of-this-message' against the result
- ;; of `window-end', but that doesn't account for the fact that
- ;; the end of the message might be hidden.
- ((and visible-end-of-this-message
- (> visible-end-of-this-message (window-end)))
- ;; The bottom of this message is not visible - scroll.
- (scroll-up nil))
-
- ((not (= end-of-this-message (point-max)))
- ;; This is not the last message - move to the next visible one.
- (notmuch-show-next-open-message))
-
- (t
- ;; This is the last message - change the return value
- (setq ret t)))
- ret))
+ (cond
+ ((eobp)
+ ;; If we are at the end of the buffer then move to the next
+ ;; thread.
+ t)
+
+ ;; Ideally we would simply do:
+ ;;
+ ;; ((> (notmuch-show-message-bottom) (window-end))
+ ;;
+ ;; here, but that fails if the trailing text in the buffer is
+ ;; invisible (`window-end' returns the last visible character,
+ ;; which can then be smaller than `notmuch-show-message-bottom').
+ ;;
+ ;; So we need to find the last _visible_ character of the
+ ;; message. We do this by checking the invisibility of the
+ ;; characters from `notmuch-show-message-bottom'-1 towards the start
+ ;; of the message. When we find a non-invisible character, we test
+ ;; to see whether it is visible in the window.
+
+ ((let ((visible-bottom
+ (save-excursion
+ (goto-char (1- (notmuch-show-message-bottom)))
+ (while (invisible-p (point))
+ (backward-char))
+ (point))))
+ (> visible-bottom (window-end)))
+ ;; The end of this message is not visible - scroll.
+ (scroll-up)
+ nil)
+
+ (t
+ ;; Show the start of the next message.
+ (notmuch-show-next-open-message)
+ nil)))
(defun notmuch-show-advance-and-archive ()
"Advance through thread and archive.
@@ -1201,44 +1212,39 @@ from each message), kills the buffer, and displays the next
thread from the search from which this thread was originally
shown."
(interactive)
- (if (notmuch-show-advance)
- (notmuch-show-archive-thread)))
+ (when (notmuch-show-advance)
+ (notmuch-show-archive-thread)))
(defun notmuch-show-rewind ()
- "Backup through the thread, (reverse scrolling compared to \\[notmuch-show-advance-and-archive]).
+ "Move backwards through a thread, the counterpart to \\[notmuch-show-advance-and-archive]."
-Specifically, if the beginning of the previous email is fewer
-than `window-height' lines from the current point, move to it
-just like `notmuch-show-previous-message'.
-
-Otherwise, just scroll down a screenful of the current message.
-
-This command does not modify any message tags, (it does not undo
-any effects from previous calls to
-`notmuch-show-advance-and-archive'."
(interactive)
- (let ((start-of-message (notmuch-show-message-top))
- (start-of-window (window-start)))
+ (let ((start-of-message (notmuch-show-message-top)))
(cond
- ;; Either this message is properly aligned with the start of the
- ;; window or the start of this message is not visible on the
- ;; screen - scroll.
- ((or (= start-of-message start-of-window)
- (< start-of-message start-of-window))
+ ((= start-of-message (point))
+ ;; If the cursor is at the start of the current message, move to
+ ;; the previous open message.
+ (notmuch-show-previous-open-message))
+
+ ((< start-of-message (window-start))
+ ;; If the start of the current message is not visible, scroll
+ ;; down.
(scroll-down)
- ;; If a small number of lines from the previous message are
- ;; visible, realign so that the top of the current message is at
- ;; the top of the screen.
- (if (<= (count-screen-lines (window-start) start-of-message)
- next-screen-context-lines)
- (progn
- (goto-char (notmuch-show-message-top))
- (notmuch-show-message-adjust)))
- ;; Move to the top left of the window.
- (goto-char (window-start)))
+ ;; If the start of the current message became visible, align it
+ ;; with the top of the window.
+ (when (> start-of-message (window-start))
+ (goto-char start-of-message)
+ (notmuch-show-message-adjust)))
+
+ ((> start-of-message (window-start))
+ ;; If the cursor is not at the start of the current (visible)
+ ;; message, move it there, but do not adjust or scroll the
+ ;; display.
+ (goto-char start-of-message))
+
(t
;; Move to the previous message.
- (notmuch-show-previous-message)))))
+ (notmuch-show-previous-open-message)))))
(defun notmuch-show-reply (&optional prompt-for-sender)
"Reply to the current message."
--
1.7.7.3
More information about the notmuch
mailing list