--- /dev/null
+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 BF663431FC2\r
+ for <notmuch@notmuchmail.org>; Sun, 30 Sep 2012 01:53:21 -0700 (PDT)\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 H+5tNzIE0XlU for <notmuch@notmuchmail.org>;\r
+ Sun, 30 Sep 2012 01:53:19 -0700 (PDT)\r
+Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
+ by olra.theworths.org (Postfix) with ESMTP id 6B203431FBC\r
+ for <notmuch@notmuchmail.org>; Sun, 30 Sep 2012 01:53:19 -0700 (PDT)\r
+Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
+ by guru.guru-group.fi (Postfix) with ESMTP id 2A6F01000D0;\r
+ Sun, 30 Sep 2012 11:53:23 +0300 (EEST)\r
+From: Tomi Ollila <tomi.ollila@iki.fi>\r
+To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v2] emacs: simplify point placement in notmuch-hello\r
+ refresh\r
+In-Reply-To: <1348990572-7860-1-git-send-email-jani@nikula.org>\r
+References: <1348990572-7860-1-git-send-email-jani@nikula.org>\r
+User-Agent: Notmuch/0.14+48~g640455c (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: Sun, 30 Sep 2012 11:53:22 +0300\r
+Message-ID: <m28vbr7uyl.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: Sun, 30 Sep 2012 08:53:21 -0000\r
+\r
+On Sun, Sep 30 2012, Jani Nikula <jani@nikula.org> wrote:\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
+> 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
+> Save current line and column before refresh, and restore them\r
+> afterwards. Sometimes, if notmuch-show-empty-saved-searches is nil,\r
+> and the refresh adds or removes saved searches from the list, this has\r
+> the appearance of moving the point relative to the nearest\r
+> widgets. This is a much smaller and less frequent problem than the\r
+> ones listed above.\r
+\r
+LGTM.\r
+\r
+Tomi\r
+\r
+\r
+>\r
+> ---\r
+>\r
+> v2 of id:"1348958664-1767-1-git-send-email-jani@nikula.org": More\r
+> (found-)target-pos cleanup, and use line and column instead of point\r
+> directly, both suggested by Austin.\r
+> ---\r
+> emacs/notmuch-hello.el | 108 ++++++++++++++++--------------------------------\r
+> 1 file changed, 35 insertions(+), 73 deletions(-)\r
+>\r
+> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el\r
+> index 684bedc..052aaeb 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
+> @@ -435,8 +418,7 @@ Such a list can be computed with `notmuch-hello-query-counts'."\r
+> (reordered-list (notmuch-hello-reflect searches tags-per-line))\r
+> ;; Hack the display of the buttons used.\r
+> (widget-push-button-prefix "")\r
+> - (widget-push-button-suffix "")\r
+> - (found-target-pos nil))\r
+> + (widget-push-button-suffix ""))\r
+> ;; dme: It feels as though there should be a better way to\r
+> ;; implement this loop than using an incrementing counter.\r
+> (mapc (lambda (elem)\r
+> @@ -449,8 +431,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
+> @@ -466,8 +446,7 @@ Such a list can be computed with `notmuch-hello-query-counts'."\r
+> ;; If the last line was not full (and hence did not include a\r
+> ;; carriage return), insert one now.\r
+> (unless (eq (% count tags-per-line) 0)\r
+> - (widget-insert "\n"))\r
+> - found-target-pos))\r
+> + (widget-insert "\n"))))\r
+> \r
+> (defimage notmuch-hello-logo ((:type png :file "notmuch-logo.png")))\r
+> \r
+> @@ -571,8 +550,7 @@ Complete list of currently available key bindings:\r
+> (funcall notmuch-saved-search-sort-function\r
+> notmuch-saved-searches)\r
+> notmuch-saved-searches)\r
+> - :show-empty-searches notmuch-show-empty-saved-searches))\r
+> - found-target-pos)\r
+> + :show-empty-searches notmuch-show-empty-saved-searches)))\r
+> (when searches\r
+> (widget-insert "Saved searches: ")\r
+> (widget-create 'push-button\r
+> @@ -581,15 +559,12 @@ Complete list of currently available key bindings:\r
+> "edit")\r
+> (widget-insert "\n\n")\r
+> (let ((start (point)))\r
+> - (setq found-target-pos\r
+> - (notmuch-hello-insert-buttons searches))\r
+> - (indent-rigidly start (point) notmuch-hello-indent)\r
+> - found-target-pos))))\r
+> + (notmuch-hello-insert-buttons searches)\r
+> + (indent-rigidly start (point) notmuch-hello-indent)))))\r
+> \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
+> @@ -689,16 +664,13 @@ Supports the following entries in OPTIONS as a plist:\r
+> (notmuch-hello-update))\r
+> "hide"))\r
+> (widget-insert "\n")\r
+> - (let (target-pos)\r
+> - (when (not is-hidden)\r
+> - (let ((searches (apply 'notmuch-hello-query-counts query-alist options)))\r
+> - (when (or (not (plist-get options :hide-if-empty))\r
+> - searches)\r
+> - (widget-insert "\n")\r
+> - (setq target-pos\r
+> - (notmuch-hello-insert-buttons searches))\r
+> - (indent-rigidly start (point) notmuch-hello-indent))))\r
+> - target-pos)))\r
+> + (when (not is-hidden)\r
+> + (let ((searches (apply 'notmuch-hello-query-counts query-alist options)))\r
+> + (when (or (not (plist-get options :hide-if-empty))\r
+> + searches)\r
+> + (widget-insert "\n")\r
+> + (notmuch-hello-insert-buttons searches)\r
+> + (indent-rigidly start (point) notmuch-hello-indent))))))\r
+> \r
+> (defun notmuch-hello-insert-tags-section (&optional title &rest options)\r
+> "Insert a section displaying all tags with message counts.\r
+> @@ -763,13 +735,8 @@ 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 ((target-line (line-number-at-pos))\r
+> + (target-column (current-column))\r
+> (inhibit-read-only t))\r
+> \r
+> ;; Delete all editable widget fields. Editable widget fields are\r
+> @@ -788,30 +755,25 @@ 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
+> + ;; Move point back to where it was before refresh. Use line and\r
+> + ;; column instead of point directly to be insensitive to additions\r
+> + ;; and removals of text within earlier lines.\r
+> + (goto-char (point-min))\r
+> + (forward-line (1- target-line))\r
+> + (move-to-column target-column))\r
+> (run-hooks 'notmuch-hello-refresh-hook)\r
+> (setq notmuch-hello-first-run nil))\r
+> \r
+> -- \r
+> 1.7.9.5\r
+>\r
+> _______________________________________________\r
+> notmuch mailing list\r
+> notmuch@notmuchmail.org\r
+> http://notmuchmail.org/mailman/listinfo/notmuch\r
projects / notmuch-archives.git / commitdiff