From: Tomi Ollila Date: Wed, 23 Jan 2013 17:01:56 +0000 (+0200) Subject: Re: [PATCH v2] emacs: Disambiguate point placement after hiding message X-Git-Url: http://git.tremily.us/gitweb.cgi?a=commitdiff_plain;h=bf76e25c96437823a60df6cde42200dd1dafe5dc;p=notmuch-archives.git Re: [PATCH v2] emacs: Disambiguate point placement after hiding message --- diff --git a/74/7c66b29e12e322e18e27b52fdd92fdbfae6c5b b/74/7c66b29e12e322e18e27b52fdd92fdbfae6c5b new file mode 100644 index 000000000..c2a21f43d --- /dev/null +++ b/74/7c66b29e12e322e18e27b52fdd92fdbfae6c5b @@ -0,0 +1,242 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id B505B431FC0 + for ; Wed, 23 Jan 2013 09:02:09 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 0 +X-Spam-Level: +X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none] + 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 k2EagIttOpvF for ; + Wed, 23 Jan 2013 09:02:08 -0800 (PST) +Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) + by olra.theworths.org (Postfix) with ESMTP id 2A25B431FAE + for ; Wed, 23 Jan 2013 09:02:08 -0800 (PST) +Received: from guru.guru-group.fi (localhost [IPv6:::1]) + by guru.guru-group.fi (Postfix) with ESMTP id 66743100086; + Wed, 23 Jan 2013 19:01:56 +0200 (EET) +From: Tomi Ollila +To: Austin Clements , notmuch@notmuchmail.org +Subject: Re: [PATCH v2] emacs: Disambiguate point placement after hiding + message +In-Reply-To: <1357684759-25979-1-git-send-email-amdragon@mit.edu> +References: <1357591189-19487-1-git-send-email-amdragon@mit.edu> + <1357684759-25979-1-git-send-email-amdragon@mit.edu> +User-Agent: Notmuch/0.15+20~g7e29ae9 (http://notmuchmail.org) Emacs/24.2.1 + (x86_64-unknown-linux-gnu) +X-Face: HhBM'cA~ +MIME-Version: 1.0 +Content-Type: text/plain +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: Wed, 23 Jan 2013 17:02:09 -0000 + +On Wed, Jan 09 2013, Austin Clements wrote: + +> Currently, if point is in the middle of a message when the user +> collapses it, Emacs then displays the cursor on the header line of the +> next message, even though point is still on the collapsed message and +> even though, if you explicitly move point to the same visual location, +> it will be on the next message. As a result, following actions like +> re-expanding the message or modifying tags apply to the collapsed +> message, even though, visually, it looks like they will apply to the +> message following the collapsed message. +> +> This patch addresses this by explicitly moving point when a message is +> collapsed so it is visually unambiguous that the point is still on the +> collapsed message. +> --- +> +> v2 should fix the strange behavior observed in v1. The added code is +> essentially identical to v1, but v2 adds it to +> notmuch-show-toggle-message---which is only used +> interactively---rather than the core notmuch-show-message-visible +> function. +> +> emacs/notmuch-show.el | 28 +++++++++++++++++++++++----- +> 1 file changed, 23 insertions(+), 5 deletions(-) +> +> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el +> index 5751d98..6ab926c 100644 +> --- a/emacs/notmuch-show.el +> +++ b/emacs/notmuch-show.el +> @@ -1789,12 +1789,30 @@ See `notmuch-tag' for information on the format of TAG-CHANGES." +> (force-window-update)) +> +> (defun notmuch-show-toggle-message () +> - "Toggle the visibility of the current message." +> + "Toggle the visibility of the current message. +> + +> +If this hides the current message, it will also move point to +> +make it obvious it's still on the current message." +> (interactive) +> - (let ((props (notmuch-show-get-message-properties))) +> - (notmuch-show-message-visible +> - props +> - (not (plist-get props :message-visible)))) +> + (let* ((props (notmuch-show-get-message-properties)) +> + (visible-p (not (plist-get props :message-visible)))) +> + (notmuch-show-message-visible props visible-p) +> + (when (not visible-p) +> + (let ((ov (plist-get props :message-overlay))) +> + ;; If point was contained in the overlay, move it to a +> + ;; sensible spot that is visible and still on the same +> + ;; message. Strangely, the Emacs event loop doesn't move the +> + ;; point out of the invisible region for us like it normally +> + ;; does (perhaps because it doesn't know which way to go), so +> + ;; if we don't do this, it's visually ambiguous which message +> + ;; an action will apply to. +> + (let ((start (overlay-start ov)) +> + (end (overlay-end ov))) +> + (dolist (win (get-buffer-window-list nil nil t)) +> + (with-selected-window win +> + (when (and (<= start (point)) (< (point) end)) +> + (goto-char (1- start)) +> + (beginning-of-visual-line)))))))) + +Soo. the problem with this is still the behaviour of +(beginning-of-visual-line), which "leaks" to previous message +if (point) is in the first header line. Interestingly +(beginning-of-line) does not -- and this is supposed to move more +than beginning-of-visual-line... + +But this and the defadvice in +id:1357855176-31653-1-git-send-email-amdragon@mit.edu +could fix this problem -- if the "sketchy" approach were used... + +Austin: have you got further with the "alternate" approach +you mentioned in the other mail ? + +In the meanwhile I played a bit how those overlays are +positioned -- basically moved those to one character position +closer to the beginning of buffer -- so that overlays start +with "\n" and end just before "\n". My naive attempts just +to move brought some interesting side effects (line counts +in button change, header line coloring doesn't go to the +end of file and cursor is sometimes positioned interesting +-- but the cursor no longer leak to previous (or next) +message. + +For reference, The changes I made attached. I don't bother +to make proper patch until/unless we know this is definite +way to proceed (and this definitely have some implementation +issues, too)... + +>From bd3571c578aa45a23a120fc82b89f7e1649617fd Mon Sep 17 00:00:00 2001 +From: Tomi Ollila +Date: Wed, 23 Jan 2013 11:34:20 +0300 +Subject: [PATCH] these email headers copied just to make git am happy + +--- +diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el +index 1864dd1..7ee6d1d 100644 +--- a/emacs/notmuch-show.el ++++ b/emacs/notmuch-show.el +@@ -870,7 +870,7 @@ message at DEPTH in the current thread." + (defun notmuch-show-create-part-overlays (msg beg end hide) + "Add an overlay to the part between BEG and END" + (let* ((button (button-at beg)) +- (part-beg (and button (1+ (button-end button))))) ++ (part-beg (and button (button-end button)))) + + ;; If the part contains no text we do not make it toggleable. We + ;; also need to check that the button is a genuine part button not +@@ -898,7 +898,7 @@ If HIDE is non-nil then initially hide this part." + ;; Ensure that the part ends with a carriage return. + (unless (bolp) + (insert "\n")) +- (notmuch-show-create-part-overlays msg beg (point) hide))) ++ (notmuch-show-create-part-overlays msg beg (1- (point)) hide))) + + (defun notmuch-show-insert-body (msg body depth) + "Insert the body BODY at depth DEPTH in the current thread." +@@ -946,8 +946,8 @@ If HIDE is non-nil then initially hide this part." + (when (not (string= notmuch-show-previous-subject + bare-subject)) + (forward-line 1)) +- (setq headers-start (point-marker))) +- (setq headers-end (point-marker)) ++ (setq headers-start (copy-marker (1- (point))))) ++ (setq headers-end (copy-marker (1- (point)))) + + (setq notmuch-show-previous-subject bare-subject) + +@@ -958,7 +958,7 @@ If HIDE is non-nil then initially hide this part." + ;; Ensure that the body ends with a newline. + (unless (bolp) + (insert "\n")) +- (setq content-end (point-marker)) ++ (setq content-end (copy-marker (1- (point)))) + + ;; Indent according to the depth in the thread. + (if notmuch-show-indent-content +diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el +index d6db4fa..49e2dde 100644 +--- a/emacs/notmuch-wash.el ++++ b/emacs/notmuch-wash.el +@@ -167,11 +167,12 @@ that PREFIX should not include a newline." + (if prefix + (insert-before-markers prefix)) + (let ((button-beg (point))) +- (insert-before-markers (notmuch-wash-button-label overlay) "\n") +- (let ((button (make-button button-beg (1- (point)) ++ (insert-before-markers (notmuch-wash-button-label overlay)) ++ (let ((button (make-button button-beg (point) + 'overlay overlay + :type button-type))) +- (overlay-put overlay 'notmuch-wash-button button)))))) ++ (overlay-put overlay 'notmuch-wash-button button)) ++ (insert "\n"))))) ;; after marker(s) + + (defun notmuch-wash-excerpt-citations (msg depth) + "Excerpt citations and up to one signature." +@@ -183,7 +184,7 @@ that PREFIX should not include a newline." + (msg-end (point-max)) + (msg-lines (count-lines msg-start msg-end))) + (notmuch-wash-region-to-button +- msg msg-start msg-end "original"))) ++ msg msg-start (1- msg-end) "original"))) + (while (and (< (point) (point-max)) + (re-search-forward notmuch-wash-citation-regexp nil t)) + (let* ((cite-start (match-beginning 0)) +@@ -199,7 +200,7 @@ that PREFIX should not include a newline." + (goto-char cite-end) + (forward-line (- notmuch-wash-citation-lines-suffix)) + (notmuch-wash-region-to-button +- msg hidden-start (point-marker) ++ msg hidden-start (copy-marker (1- (point))) + "citation"))))) + (if (and (not (eobp)) + (re-search-forward notmuch-wash-signature-regexp nil t)) +@@ -207,10 +208,8 @@ that PREFIX should not include a newline." + (sig-end (match-end 0)) + (sig-lines (count-lines sig-start (point-max)))) + (if (<= sig-lines notmuch-wash-signature-lines-max) +- (let ((sig-start-marker (make-marker)) +- (sig-end-marker (make-marker))) +- (set-marker sig-start-marker sig-start) +- (set-marker sig-end-marker (point-max)) ++ (let ((sig-start-marker (copy-marker sig-start)) ++ (sig-end-marker (copy-marker (1- (point-max))))) + (overlay-put (make-overlay sig-start-marker sig-end-marker) 'face 'message-cited-text) + (notmuch-wash-region-to-button + msg sig-start-marker sig-end-marker