[PATCH v2] emacs: simplify point placement in notmuch-hello refresh
Tomi Ollila
tomi.ollila at iki.fi
Sun Sep 30 01:53:22 PDT 2012
On Sun, Sep 30 2012, Jani Nikula <jani at nikula.org> wrote:
> notmuch-hello (called also through notmuch-hello-update, bound to '='
> by default) tries to find the widget under or following point before
> refresh, and put the point back to the widget afterwards. The code has
> grown quite complicated, and has at least the following issues:
>
> 1) All the individual section functions have to include code to
> support point placement. If there is no such support, point is
> dropped to the search box. Only saved searches and all tags
> sections support point placement.
>
> 2) Point placement is based on widget-value. If there are two widgets
> with the same widget-value (for example a saved search with the
> same name as a tag) the point is moved to the earlier one, even if
> point was on the later one.
>
> 3) When first entering notmuch-hello notmuch-hello-target is nil, and
> point is dropped to the search box.
>
> Moving the point to the search box is annoying because the user is
> required to move the point before being able to enter key bindings.
>
> Simplify the code by removing all point placement based on widgets, as
> it does not work properly, and trying to fix that would unnecessarily
> complicate the code.
>
> Save current line and column before refresh, and restore them
> afterwards. Sometimes, if notmuch-show-empty-saved-searches is nil,
> and the refresh adds or removes saved searches from the list, this has
> the appearance of moving the point relative to the nearest
> widgets. This is a much smaller and less frequent problem than the
> ones listed above.
LGTM.
Tomi
>
> ---
>
> v2 of id:"1348958664-1767-1-git-send-email-jani at nikula.org": More
> (found-)target-pos cleanup, and use line and column instead of point
> directly, both suggested by Austin.
> ---
> emacs/notmuch-hello.el | 108 ++++++++++++++++--------------------------------
> 1 file changed, 35 insertions(+), 73 deletions(-)
>
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index 684bedc..052aaeb 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -154,11 +154,6 @@ International Bureau of Weights and Measures."
> (defvar notmuch-hello-url "http://notmuchmail.org"
> "The `notmuch' web site.")
>
> -(defvar notmuch-hello-search-pos nil
> - "Position of search widget, if any.
> -
> -This should only be set by `notmuch-hello-insert-search'.")
> -
> (defvar notmuch-hello-custom-section-options
> '((:filter (string :tag "Filter for each tag"))
> (:filter-count (string :tag "Different filter to generate message counts"))
> @@ -209,11 +204,8 @@ function produces a section simply by adding content to the current
> buffer. A section should not end with an empty line, because a
> newline will be inserted after each section by `notmuch-hello'.
>
> -Each function should take no arguments. If the produced section
> -includes `notmuch-hello-target' (i.e. cursor should be positioned
> -inside this section), the function should return this element's
> -position.
> -Otherwise, it should return nil.
> +Each function should take no arguments. The return value is
> +ignored.
>
> For convenience an element can also be a list of the form (FUNC ARG1
> ARG2 .. ARGN) in which case FUNC will be applied to the rest of the
> @@ -240,15 +232,6 @@ supported for \"Customized queries section\" items."
> notmuch-hello-query-section
> (function :tag "Custom section"))))
>
> -(defvar notmuch-hello-target nil
> - "Button text at position of point before rebuilding the notmuch-buffer.
> -
> -This variable contains the text of the button, if any, the
> -point was positioned at before the notmuch-hello buffer was
> -rebuilt. This should never actually be global and is defined as a
> -defvar only for documentation purposes and to avoid a compiler
> -warning about it occurring as a free variable.")
> -
> (defvar notmuch-hello-hidden-sections nil
> "List of sections titles whose contents are hidden")
>
> @@ -435,8 +418,7 @@ Such a list can be computed with `notmuch-hello-query-counts'."
> (reordered-list (notmuch-hello-reflect searches tags-per-line))
> ;; Hack the display of the buttons used.
> (widget-push-button-prefix "")
> - (widget-push-button-suffix "")
> - (found-target-pos nil))
> + (widget-push-button-suffix ""))
> ;; dme: It feels as though there should be a better way to
> ;; implement this loop than using an incrementing counter.
> (mapc (lambda (elem)
> @@ -449,8 +431,6 @@ Such a list can be computed with `notmuch-hello-query-counts'."
> (msg-count (third elem)))
> (widget-insert (format "%8s "
> (notmuch-hello-nice-number msg-count)))
> - (if (string= name notmuch-hello-target)
> - (setq found-target-pos (point-marker)))
> (widget-create 'push-button
> :notify #'notmuch-hello-widget-search
> :notmuch-search-terms query
> @@ -466,8 +446,7 @@ Such a list can be computed with `notmuch-hello-query-counts'."
> ;; If the last line was not full (and hence did not include a
> ;; carriage return), insert one now.
> (unless (eq (% count tags-per-line) 0)
> - (widget-insert "\n"))
> - found-target-pos))
> + (widget-insert "\n"))))
>
> (defimage notmuch-hello-logo ((:type png :file "notmuch-logo.png")))
>
> @@ -571,8 +550,7 @@ Complete list of currently available key bindings:
> (funcall notmuch-saved-search-sort-function
> notmuch-saved-searches)
> notmuch-saved-searches)
> - :show-empty-searches notmuch-show-empty-saved-searches))
> - found-target-pos)
> + :show-empty-searches notmuch-show-empty-saved-searches)))
> (when searches
> (widget-insert "Saved searches: ")
> (widget-create 'push-button
> @@ -581,15 +559,12 @@ Complete list of currently available key bindings:
> "edit")
> (widget-insert "\n\n")
> (let ((start (point)))
> - (setq found-target-pos
> - (notmuch-hello-insert-buttons searches))
> - (indent-rigidly start (point) notmuch-hello-indent)
> - found-target-pos))))
> + (notmuch-hello-insert-buttons searches)
> + (indent-rigidly start (point) notmuch-hello-indent)))))
>
> (defun notmuch-hello-insert-search ()
> "Insert a search widget."
> (widget-insert "Search: ")
> - (setq notmuch-hello-search-pos (point-marker))
> (widget-create 'editable-field
> ;; Leave some space at the start and end of the
> ;; search boxes.
> @@ -689,16 +664,13 @@ Supports the following entries in OPTIONS as a plist:
> (notmuch-hello-update))
> "hide"))
> (widget-insert "\n")
> - (let (target-pos)
> - (when (not is-hidden)
> - (let ((searches (apply 'notmuch-hello-query-counts query-alist options)))
> - (when (or (not (plist-get options :hide-if-empty))
> - searches)
> - (widget-insert "\n")
> - (setq target-pos
> - (notmuch-hello-insert-buttons searches))
> - (indent-rigidly start (point) notmuch-hello-indent))))
> - target-pos)))
> + (when (not is-hidden)
> + (let ((searches (apply 'notmuch-hello-query-counts query-alist options)))
> + (when (or (not (plist-get options :hide-if-empty))
> + searches)
> + (widget-insert "\n")
> + (notmuch-hello-insert-buttons searches)
> + (indent-rigidly start (point) notmuch-hello-indent))))))
>
> (defun notmuch-hello-insert-tags-section (&optional title &rest options)
> "Insert a section displaying all tags with message counts.
> @@ -763,13 +735,8 @@ following:
> (set-buffer "*notmuch-hello*")
> (switch-to-buffer "*notmuch-hello*"))
>
> - (let ((notmuch-hello-target (if (widget-at)
> - (widget-value (widget-at))
> - (condition-case nil
> - (progn
> - (widget-forward 1)
> - (widget-value (widget-at)))
> - (error nil))))
> + (let ((target-line (line-number-at-pos))
> + (target-column (current-column))
> (inhibit-read-only t))
>
> ;; Delete all editable widget fields. Editable widget fields are
> @@ -788,30 +755,25 @@ following:
> (mapc 'delete-overlay (car all))
> (mapc 'delete-overlay (cdr all)))
>
> - (let (final-target-pos)
> - (mapc
> - (lambda (section)
> - (let ((point-before (point))
> - (result (if (functionp section)
> - (funcall section)
> - (apply (car section) (cdr section)))))
> - (if (and (not final-target-pos) (integer-or-marker-p result))
> - (setq final-target-pos result))
> - ;; don't insert a newline when the previous section didn't show
> - ;; anything.
> - (unless (eq (point) point-before)
> - (widget-insert "\n"))))
> - notmuch-hello-sections)
> - (widget-setup)
> -
> - (when final-target-pos
> - (goto-char final-target-pos)
> - (unless (widget-at)
> - (widget-forward 1)))
> -
> - (unless (widget-at)
> - (when notmuch-hello-search-pos
> - (goto-char notmuch-hello-search-pos)))))
> + (mapc
> + (lambda (section)
> + (let ((point-before (point)))
> + (if (functionp section)
> + (funcall section)
> + (apply (car section) (cdr section)))
> + ;; don't insert a newline when the previous section didn't
> + ;; show anything.
> + (unless (eq (point) point-before)
> + (widget-insert "\n"))))
> + notmuch-hello-sections)
> + (widget-setup)
> +
> + ;; Move point back to where it was before refresh. Use line and
> + ;; column instead of point directly to be insensitive to additions
> + ;; and removals of text within earlier lines.
> + (goto-char (point-min))
> + (forward-line (1- target-line))
> + (move-to-column target-column))
> (run-hooks 'notmuch-hello-refresh-hook)
> (setq notmuch-hello-first-run nil))
>
> --
> 1.7.9.5
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list