Re: [PATCH v2] emacs: Disambiguate point placement after hiding message
authorTomi Ollila <tomi.ollila@iki.fi>
Wed, 23 Jan 2013 17:01:56 +0000 (19:01 +0200)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:53:20 +0000 (09:53 -0800)
74/7c66b29e12e322e18e27b52fdd92fdbfae6c5b [new file with mode: 0644]

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