From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id DFE2A431FD0 for ; Thu, 22 Dec 2011 04:38:46 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id AGuxIkDiYb82 for ; Thu, 22 Dec 2011 04:38:46 -0800 (PST) Received: from mail-we0-f181.google.com (mail-we0-f181.google.com [74.125.82.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id DDF1B431FB6 for ; Thu, 22 Dec 2011 04:38:45 -0800 (PST) Received: by werm12 with SMTP id m12so4109236wer.26 for ; Thu, 22 Dec 2011 04:38:43 -0800 (PST) Received: by 10.216.138.148 with SMTP id a20mr11347087wej.15.1324557523413; Thu, 22 Dec 2011 04:38:43 -0800 (PST) Received: from hotblack-desiato.hh.sledj.net (host81-149-164-25.in-addr.btopenworld.com. [81.149.164.25]) by mx.google.com with ESMTPS id z5sm22147967wix.5.2011.12.22.04.38.41 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 22 Dec 2011 04:38:42 -0800 (PST) Received: by hotblack-desiato.hh.sledj.net (Postfix, from userid 30000) id 634C49FC4F; Thu, 22 Dec 2011 12:38:40 +0000 (GMT) From: David Edmondson To: notmuch@notmuchmail.org Subject: [RFC][PATCH v3] emacs: Re-implement advance/rewind functions of notmuch-show-mode. Date: Thu, 22 Dec 2011 12:38:38 +0000 Message-Id: <1324557518-19269-1-git-send-email-dme@dme.org> X-Mailer: git-send-email 1.7.7.3 In-Reply-To: <1324556204-15700-1-git-send-email-dme@dme.org> References: <1324556204-15700-1-git-send-email-dme@dme.org> X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Dec 2011 12:38:47 -0000 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