Re: [PATCH] emacs: simplify point placement in notmuch-hello refresh
authorAustin Clements <amdragon@MIT.EDU>
Sat, 29 Sep 2012 23:58:30 +0000 (19:58 +2000)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:49:39 +0000 (09:49 -0800)
0e/e8909648371c8a76e357e4805c399638417f49 [new file with mode: 0644]

diff --git a/0e/e8909648371c8a76e357e4805c399638417f49 b/0e/e8909648371c8a76e357e4805c399638417f49
new file mode 100644 (file)
index 0000000..5744fc3
--- /dev/null
@@ -0,0 +1,298 @@
+Return-Path: <amdragon@mit.edu>\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 2CCD0431FBF\r
+       for <notmuch@notmuchmail.org>; Sat, 29 Sep 2012 16:58:36 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+       tests=[RCVD_IN_DNSWL_LOW=-0.7] 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 NtBmrqh8Tr3F for <notmuch@notmuchmail.org>;\r
+       Sat, 29 Sep 2012 16:58:34 -0700 (PDT)\r
+Received: from dmz-mailsec-scanner-8.mit.edu (DMZ-MAILSEC-SCANNER-8.MIT.EDU\r
+       [18.7.68.37])\r
+       by olra.theworths.org (Postfix) with ESMTP id 30629431FBC\r
+       for <notmuch@notmuchmail.org>; Sat, 29 Sep 2012 16:58:34 -0700 (PDT)\r
+X-AuditID: 12074425-b7fcc6d00000091f-91-50678b29a175\r
+Received: from mailhub-auth-4.mit.edu ( [18.7.62.39])\r
+       by dmz-mailsec-scanner-8.mit.edu (Symantec Messaging Gateway) with SMTP\r
+       id 6A.3D.02335.92B87605; Sat, 29 Sep 2012 19:58:33 -0400 (EDT)\r
+Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
+       by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id q8TNwWiC016507; \r
+       Sat, 29 Sep 2012 19:58:33 -0400\r
+Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
+       (authenticated bits=0)\r
+       (User authenticated as amdragon@ATHENA.MIT.EDU)\r
+       by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q8TNwUhZ005141\r
+       (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
+       Sat, 29 Sep 2012 19:58:31 -0400 (EDT)\r
+Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
+       (envelope-from <amdragon@mit.edu>)\r
+       id 1TI6vW-0006xx-GH; Sat, 29 Sep 2012 19:58:30 -0400\r
+Date: Sat, 29 Sep 2012 19:58:30 -0400\r
+From: Austin Clements <amdragon@MIT.EDU>\r
+To: Jani Nikula <jani@nikula.org>\r
+Subject: Re: [PATCH] emacs: simplify point placement in notmuch-hello refresh\r
+Message-ID: <20120929235830.GZ26662@mit.edu>\r
+References: <1348958664-1767-1-git-send-email-jani@nikula.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+Content-Disposition: inline\r
+In-Reply-To: <1348958664-1767-1-git-send-email-jani@nikula.org>\r
+User-Agent: Mutt/1.5.21 (2010-09-15)\r
+X-Brightmail-Tracker:\r
+ H4sIAAAAAAAAA+NgFmpileLIzCtJLcpLzFFi42IRYrdT19XsTg8wuLxa1aJpurPF9ZszmR2Y\r
+       PG7df83u8WzVLeYApigum5TUnMyy1CJ9uwSujNsNX9gLNvtVbOq7xtjAOMu2i5GTQ0LAROLI\r
+       jx1sELaYxIV764FsLg4hgX2MEtc/rGCCcDYwSny8tJQFpEpI4CSTxMdzuhCJJYwSF1vXgCVY\r
+       BFQl/q1bzwxiswloSGzbv5wRxBYRUJTYfHI/mM0sIC3x7XczE4gtLOArcXPVNrDVvAI6Eh/W\r
+       9TFCLLCTeLzpC1RcUOLkzCcsEL1aEjf+vQTq5QCbs/wfB0iYU8Be4vnkj2CtogIqElNObmOb\r
+       wCg0C0n3LCTdsxC6FzAyr2KUTcmt0s1NzMwpTk3WLU5OzMtLLdK10MvNLNFLTSndxAgOahfV\r
+       HYwTDikdYhTgYFTi4b15OjVAiDWxrLgy9xCjJAeTkiivZUd6gBBfUn5KZUZicUZ8UWlOavEh\r
+       RgkOZiUR3oxioHLelMTKqtSifJiUNAeLkjjvjZSb/kIC6YklqdmpqQWpRTBZGQ4OJQneyV1A\r
+       QwWLUtNTK9Iyc0oQ0kwcnCDDeYCGe4HU8BYXJOYWZ6ZD5E8xKkqJ82aBJARAEhmleXC9sKTz\r
+       ilEc6BVh3iCQKh5gwoLrfgU0mAlocNWqNJDBJYkIKakGxqiH9otL7ogGLvGbVHpoLUPavQTu\r
+       FRtZvjbsumXXuXiRx0y7K2knLhxQ8yywVf70OcTzEJPBuogXEmvuqP1Ue5u68RP/Tx/Jvini\r
+       O+7nuMyfIL7vELf3hc6zjdOedCerxUkyRn6U89raeGWduZLK3bboG0rZtZ8nCVUX55WbRFeJ\r
+       6cw7oNEVr8RSnJFoqMVcVJwIAPy8KngVAwAA\r
+Cc: notmuch@notmuchmail.org\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: Sat, 29 Sep 2012 23:58:36 -0000\r
+\r
+LGTM.  I wasn't aware notmuch-hello even attempted to maintain point,\r
+since I'd never seen it work.\r
+\r
+This patch could go a step further and remove found-target-pos from\r
+notmuch-hello-insert-saved-searches, target-pos from\r
+notmuch-hello-insert-searches, and found-target-pos from\r
+notmuch-hello-insert-buttons.\r
+\r
+It might be more stable to record the line number and column count and\r
+restore those, since point is sensitive to additions and removals of\r
+text within earlier lines.\r
+\r
+\r
+On a more abstract note, I feel like we have this same problem all\r
+over the Emacs UI: we "update" some UI by completely rebuilding it,\r
+and then we make some ad hoc, more-or-less (usually less) intelligent\r
+guess about where point should go.  notmuch-hello has this problem,\r
+updating search-mode lines on tag change has this problem, refreshing\r
+the search buffer has this problem, and various buffer-rebuilding\r
+toggles in show have this problem.\r
+\r
+Proper incremental UI updates are simply not an option for some of\r
+these cases, because we have to call out to the CLI to get new data\r
+and that data might be completely different from what we're currently\r
+showing.  We could restructure things to support this---for example,\r
+when toggling crypto, show could ask for the body of each message it's\r
+already showing---but this would complicate the code and I'm not sure\r
+we can restructure cases where the set of objects may change.\r
+\r
+But I wonder if there's an abstraction that would let us fully rebuild\r
+UIs or parts of UIs, but keep enough context to make this fluid.  I'm\r
+imagining something like representing a buffer as a tree of UI nodes,\r
+where the buffer itself is simply the flattening of this tree, and\r
+each UI node has a identifier that's unique among its siblings.  For\r
+example, the search buffer would have a root child for each search\r
+result, and then each piece of the result line would be a child of\r
+that line's node.  This mechanism would give us a persistent context.\r
+Before an update, it could record the identifiers of all of the nodes\r
+in the path leading to the leaf node containing point, along with all\r
+of the prior siblings of each of these nodes and the offset of point\r
+within the leaf node.  After and update, it could replay this path as\r
+best as possible: at each step in the path, if it can find a node with\r
+the same identifier as the node in the path, it would descend into it;\r
+otherwise, it would descend into the last recorded sibling it can find\r
+in the new tree.  When this decent reaches a leaf, it can place point\r
+there.  This same mechanism could be used to record and restore the\r
+window start position, as well as other markers we may need.\r
+\r
+Quoth Jani Nikula on Sep 30 at  1:44 am:\r
+> notmuch-hello (called also through notmuch-hello-update, bound to '='\r
+> by default) tries to find the widget under or following point before\r
+> refresh, and put the point back to the widget afterwards. The code has\r
+> grown quite complicated, and has at least the following issues:\r
+> \r
+> 1) All the individual section functions have to include code to\r
+>    support point placement. If there is no such support, point is\r
+>    dropped to the search box. Only saved searches and all tags\r
+>    sections support point placement.\r
+> \r
+> 2) Point placement is based on widget-value. If there are two widgets\r
+>    with the same widget-value (for example a saved search with the\r
+>    same name as a tag) the point is moved to the earlier one, even if\r
+>    point was on the later one.\r
+> \r
+> 3) When first entering notmuch-hello notmuch-hello-target is nil, and\r
+>    point is dropped to the search box.\r
+> \r
+> Moving the point to the search box is annoying because the user is\r
+> required to move the point before being able to enter key bindings.\r
+> \r
+> Simplify the code by removing all point placement based on widgets, as\r
+> it does not work properly, and trying to fix that would unnecessarily\r
+> complicate the code.\r
+> \r
+> Point is simply saved before refresh, and put back to where it\r
+> was. Sometimes, if notmuch-show-empty-saved-searches is nil, and the\r
+> refresh adds or removes saved searches from the list, this has the\r
+> appearance of moving the point relative to the nearest widgets. This\r
+> is a much smaller and less frequent problem than the ones listed\r
+> above.\r
+> \r
+> ---\r
+> \r
+> I've sent this before months ago, and received comments starting at\r
+> id:"87aa2aohjs.fsf@gmail.com". I find the current behaviour annoying,\r
+> and I prefer this simple fix over complicating the existing\r
+> mess. Nobody has stood up to do anything better anyway. Perhaps we\r
+> could take a step forward with this while waiting for Someone(tm) to\r
+> come up with the perfect point placement?\r
+> ---\r
+>  emacs/notmuch-hello.el |   70 ++++++++++++------------------------------------\r
+>  1 file changed, 17 insertions(+), 53 deletions(-)\r
+> \r
+> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el\r
+> index 684bedc..04d90cf 100644\r
+> --- a/emacs/notmuch-hello.el\r
+> +++ b/emacs/notmuch-hello.el\r
+> @@ -154,11 +154,6 @@ International Bureau of Weights and Measures."\r
+>  (defvar notmuch-hello-url "http://notmuchmail.org"\r
+>    "The `notmuch' web site.")\r
+>  \r
+> -(defvar notmuch-hello-search-pos nil\r
+> -  "Position of search widget, if any.\r
+> -\r
+> -This should only be set by `notmuch-hello-insert-search'.")\r
+> -\r
+>  (defvar notmuch-hello-custom-section-options\r
+>    '((:filter (string :tag "Filter for each tag"))\r
+>      (:filter-count (string :tag "Different filter to generate message counts"))\r
+> @@ -209,11 +204,8 @@ function produces a section simply by adding content to the current\r
+>  buffer.  A section should not end with an empty line, because a\r
+>  newline will be inserted after each section by `notmuch-hello'.\r
+>  \r
+> -Each function should take no arguments.  If the produced section\r
+> -includes `notmuch-hello-target' (i.e. cursor should be positioned\r
+> -inside this section), the function should return this element's\r
+> -position.\r
+> -Otherwise, it should return nil.\r
+> +Each function should take no arguments. The return value is\r
+> +ignored.\r
+>  \r
+>  For convenience an element can also be a list of the form (FUNC ARG1\r
+>  ARG2 .. ARGN) in which case FUNC will be applied to the rest of the\r
+> @@ -240,15 +232,6 @@ supported for \"Customized queries section\" items."\r
+>          notmuch-hello-query-section\r
+>          (function :tag "Custom section"))))\r
+>  \r
+> -(defvar notmuch-hello-target nil\r
+> -  "Button text at position of point before rebuilding the notmuch-buffer.\r
+> -\r
+> -This variable contains the text of the button, if any, the\r
+> -point was positioned at before the notmuch-hello buffer was\r
+> -rebuilt. This should never actually be global and is defined as a\r
+> -defvar only for documentation purposes and to avoid a compiler\r
+> -warning about it occurring as a free variable.")\r
+> -\r
+>  (defvar notmuch-hello-hidden-sections nil\r
+>    "List of sections titles whose contents are hidden")\r
+>  \r
+> @@ -449,8 +432,6 @@ Such a list can be computed with `notmuch-hello-query-counts'."\r
+>                   (msg-count (third elem)))\r
+>              (widget-insert (format "%8s "\r
+>                                     (notmuch-hello-nice-number msg-count)))\r
+> -            (if (string= name notmuch-hello-target)\r
+> -                (setq found-target-pos (point-marker)))\r
+>              (widget-create 'push-button\r
+>                             :notify #'notmuch-hello-widget-search\r
+>                             :notmuch-search-terms query\r
+> @@ -589,7 +570,6 @@ Complete list of currently available key bindings:\r
+>  (defun notmuch-hello-insert-search ()\r
+>    "Insert a search widget."\r
+>    (widget-insert "Search: ")\r
+> -  (setq notmuch-hello-search-pos (point-marker))\r
+>    (widget-create 'editable-field\r
+>               ;; Leave some space at the start and end of the\r
+>               ;; search boxes.\r
+> @@ -763,13 +743,7 @@ following:\r
+>        (set-buffer "*notmuch-hello*")\r
+>      (switch-to-buffer "*notmuch-hello*"))\r
+>  \r
+> -  (let ((notmuch-hello-target (if (widget-at)\r
+> -                              (widget-value (widget-at))\r
+> -                            (condition-case nil\r
+> -                                (progn\r
+> -                                  (widget-forward 1)\r
+> -                                  (widget-value (widget-at)))\r
+> -                              (error nil))))\r
+> +  (let ((final-target-pos (point))\r
+>      (inhibit-read-only t))\r
+>  \r
+>      ;; Delete all editable widget fields.  Editable widget fields are\r
+> @@ -788,30 +762,20 @@ following:\r
+>        (mapc 'delete-overlay (car all))\r
+>        (mapc 'delete-overlay (cdr all)))\r
+>  \r
+> -    (let (final-target-pos)\r
+> -      (mapc\r
+> -       (lambda (section)\r
+> -     (let ((point-before (point))\r
+> -           (result (if (functionp section)\r
+> -                       (funcall section)\r
+> -                     (apply (car section) (cdr section)))))\r
+> -       (if (and (not final-target-pos) (integer-or-marker-p result))\r
+> -           (setq final-target-pos result))\r
+> -       ;; don't insert a newline when the previous section didn't show\r
+> -       ;; anything.\r
+> -       (unless (eq (point) point-before)\r
+> -         (widget-insert "\n"))))\r
+> -       notmuch-hello-sections)\r
+> -      (widget-setup)\r
+> -\r
+> -      (when final-target-pos\r
+> -    (goto-char final-target-pos)\r
+> -    (unless (widget-at)\r
+> -      (widget-forward 1)))\r
+> -\r
+> -      (unless (widget-at)\r
+> -    (when notmuch-hello-search-pos\r
+> -      (goto-char notmuch-hello-search-pos)))))\r
+> +    (mapc\r
+> +     (lambda (section)\r
+> +       (let ((point-before (point)))\r
+> +     (if (functionp section)\r
+> +         (funcall section)\r
+> +       (apply (car section) (cdr section)))\r
+> +     ;; don't insert a newline when the previous section didn't\r
+> +     ;; show anything.\r
+> +     (unless (eq (point) point-before)\r
+> +       (widget-insert "\n"))))\r
+> +     notmuch-hello-sections)\r
+> +    (widget-setup)\r
+> +\r
+> +    (goto-char final-target-pos))\r
+>    (run-hooks 'notmuch-hello-refresh-hook)\r
+>    (setq notmuch-hello-first-run nil))\r
+>  \r
+\r
+-- \r
+Austin Clements                                      MIT/'06/PhD/CSAIL\r
+amdragon@mit.edu                           http://web.mit.edu/amdragon\r
+       Somewhere in the dream we call reality you will find me,\r
+              searching for the reality we call dreams.\r