Re: [RFC PATCH 1/4] emacs: simplify point placement in notmuch-hello
authorDmitry Kurochkin <dmitry.kurochkin@gmail.com>
Tue, 17 Apr 2012 09:04:39 +0000 (13:04 +0400)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:46:30 +0000 (09:46 -0800)
22/2b7e2871686c124a04c9cb8c58bc5d43a59e18 [new file with mode: 0644]

diff --git a/22/2b7e2871686c124a04c9cb8c58bc5d43a59e18 b/22/2b7e2871686c124a04c9cb8c58bc5d43a59e18
new file mode 100644 (file)
index 0000000..ed12b88
--- /dev/null
@@ -0,0 +1,268 @@
+Return-Path: <dmitry.kurochkin@gmail.com>\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 88FF1431FB6\r
+       for <notmuch@notmuchmail.org>; Tue, 17 Apr 2012 02:06:36 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.799\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.799 tagged_above=-999 required=5\r
+       tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,\r
+       FREEMAIL_FROM=0.001, 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 BVIk8pujlIwU for <notmuch@notmuchmail.org>;\r
+       Tue, 17 Apr 2012 02:06:32 -0700 (PDT)\r
+Received: from mail-lb0-f181.google.com (mail-lb0-f181.google.com\r
+       [209.85.217.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+       (No client certificate requested)\r
+       by olra.theworths.org (Postfix) with ESMTPS id D7EB7431FAE\r
+       for <notmuch@notmuchmail.org>; Tue, 17 Apr 2012 02:06:31 -0700 (PDT)\r
+Received: by lbdb5 with SMTP id b5so1760128lbd.26\r
+       for <notmuch@notmuchmail.org>; Tue, 17 Apr 2012 02:06:30 -0700 (PDT)\r
+DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;\r
+       h=from:to:subject:in-reply-to:references:user-agent:date:message-id\r
+       :mime-version:content-type;\r
+       bh=wRBPB2EEMEQaXTLKjKIq4GRxfKv+b6jRCRn/thl/mIg=;\r
+       b=eQNxLBpCsRwvv/eJMAyu1esaqKs2rRAnnvOZWTun6K6f7YCXCBZhx5hWf2xN/Ir0eF\r
+       eQ1ImeGCynmWkNeWFPP/k1KWgERecarVGUzpSyNFmGoPpr1mMT12bKotyPqW4ypWZfRc\r
+       yOOgLI1SMFQ+cQUuYnJOGsBtarLSZze8K61dKaeC93hi3c4JdkDJfOLnYCx3G0u+85il\r
+       X1cnlNJ3fYyaosuQwi3G1RExBKEHbYcCKRg65tsGo6Ep2W6mVFBKf7Ycvj4pju5nobBs\r
+       jn7H3/HRkf8eBZSxsI2w5cUpwLjdYzOlcRII3Bc8/Bca0f9i3//Xiv8ewEIE9+CyLQCW\r
+       37YA==\r
+Received: by 10.112.85.228 with SMTP id k4mr6500563lbz.76.1334653590068;\r
+       Tue, 17 Apr 2012 02:06:30 -0700 (PDT)\r
+Received: from localhost ([91.144.186.21])\r
+       by mx.google.com with ESMTPS id nv7sm10584298lab.9.2012.04.17.02.06.28\r
+       (version=TLSv1/SSLv3 cipher=OTHER);\r
+       Tue, 17 Apr 2012 02:06:29 -0700 (PDT)\r
+From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>\r
+To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org\r
+Subject: Re: [RFC PATCH 1/4] emacs: simplify point placement in notmuch-hello\r
+In-Reply-To:\r
+ <bcfdc0d1969997e89e5abe0b320d77ee2109796a.1334651669.git.jani@nikula.org>\r
+References:\r
+ <bcfdc0d1969997e89e5abe0b320d77ee2109796a.1334651669.git.jani@nikula.org>\r
+User-Agent: Notmuch/0.12+104~gf9ad237 (http://notmuchmail.org) Emacs/23.4.1\r
+       (x86_64-pc-linux-gnu)\r
+Date: Tue, 17 Apr 2012 13:04:39 +0400\r
+Message-ID: <87aa2aohjs.fsf@gmail.com>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\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: Tue, 17 Apr 2012 09:06:36 -0000\r
+\r
+Hi Jani.\r
+\r
+Jani Nikula <jani@nikula.org> writes:\r
+\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
+> gotten a bit 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.\r
+>\r
+> 3) When first entering notmuch-hello notmuch-hello-target is nil, and\r
+>    point is dropped to the search box.\r
+>\r
+> This patch simplifies the code by removing all point placement based\r
+> on widgets. Point is simply saved before refresh, and put back to\r
+> where it was. Sometimes, but not very often, this would have the\r
+> appearance of moving the point relative to the nearest widgets. IMHO\r
+> this is a minor problem compared to the issues listed above.\r
+>\r
+> A downside is that there's no visual cue (point movement) to indicate\r
+> that refresh has finished. Then again, neither was there before, if\r
+> point was at the beginning of a widget.\r
+\r
+Thanks for looking into this.  This is an annoying issue indeed.  And I\r
+was thinking about fixing it myself.\r
+\r
+I am not sure I like the approach of moving the cursor to the same\r
+position.  It is common that buffer content would change significantly\r
+after a refresh (e.g. after I archived all new mail).  That would mean\r
+the cursor would just randomly jump somewhere.  IMO we should allow\r
+smart cursor positioning which means that logic should go to individual\r
+sections.  I would propose the following plan:\r
+\r
+1. Remove special case for search box.  No section should be special.\r
+   Moreover it is possible to remove it (I did it) and in that case the\r
+   cursor would be left at the end of the buffer.  By default, the\r
+   cursor should be moved to the beginning of the buffer.\r
+\r
+2. Replace current cursor positioning logic with section specific code.\r
+   I.e. `notmuch-hello' would not do any cursor positioning (except for\r
+   item 1) but queries and tags section would save required state when a\r
+   button is clicked and the same section would use this state to\r
+   restore cursor position on refresh.  What state should be saved would\r
+   depend on the section but we should at least save the section\r
+   name/ID.  If during refresh no section set the cursor position, then\r
+   the cursor is moved to the beginning of the buffer.\r
+\r
+3. Provide a custom variable to set the default section to move the\r
+   cursor to.  I.e. set the section name/ID part of the state from item\r
+   2.  Again, details on what the default position inside the section is\r
+   would depend on the section.  For search box, it would be the input\r
+   field.  For queries/tags it would be the first tag.\r
+\r
+Item 1 is pretty simple.  The rest may be more tricky.  What do you\r
+think?\r
+\r
+Regards,\r
+  Dmitry\r
+\r
+> ---\r
+>  emacs/notmuch-hello.el |   70 +++++++++++------------------------------------\r
+>  1 files changed, 17 insertions(+), 53 deletions(-)\r
+>\r
+> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el\r
+> index 71d37b8..9cd907a 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
+> 1.7.1\r
+>\r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r