1 Return-Path: <tomi.ollila@iki.fi>
\r
2 X-Original-To: notmuch@notmuchmail.org
\r
3 Delivered-To: notmuch@notmuchmail.org
\r
4 Received: from localhost (localhost [127.0.0.1])
\r
5 by olra.theworths.org (Postfix) with ESMTP id BF663431FC2
\r
6 for <notmuch@notmuchmail.org>; Sun, 30 Sep 2012 01:53:21 -0700 (PDT)
\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
\r
11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]
\r
13 Received: from olra.theworths.org ([127.0.0.1])
\r
14 by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)
\r
15 with ESMTP id H+5tNzIE0XlU for <notmuch@notmuchmail.org>;
\r
16 Sun, 30 Sep 2012 01:53:19 -0700 (PDT)
\r
17 Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])
\r
18 by olra.theworths.org (Postfix) with ESMTP id 6B203431FBC
\r
19 for <notmuch@notmuchmail.org>; Sun, 30 Sep 2012 01:53:19 -0700 (PDT)
\r
20 Received: from guru.guru-group.fi (localhost [IPv6:::1])
\r
21 by guru.guru-group.fi (Postfix) with ESMTP id 2A6F01000D0;
\r
22 Sun, 30 Sep 2012 11:53:23 +0300 (EEST)
\r
23 From: Tomi Ollila <tomi.ollila@iki.fi>
\r
24 To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org
\r
25 Subject: Re: [PATCH v2] emacs: simplify point placement in notmuch-hello
\r
27 In-Reply-To: <1348990572-7860-1-git-send-email-jani@nikula.org>
\r
28 References: <1348990572-7860-1-git-send-email-jani@nikula.org>
\r
29 User-Agent: Notmuch/0.14+48~g640455c (http://notmuchmail.org) Emacs/24.2.1
\r
30 (x86_64-unknown-linux-gnu)
\r
31 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL
\r
32 $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F
\r
33 !)g;OY^,BjTbr)Np:%c_o'jj,Z
\r
34 Date: Sun, 30 Sep 2012 11:53:22 +0300
\r
35 Message-ID: <m28vbr7uyl.fsf@guru.guru-group.fi>
\r
37 Content-Type: text/plain
\r
38 X-BeenThere: notmuch@notmuchmail.org
\r
39 X-Mailman-Version: 2.1.13
\r
41 List-Id: "Use and development of the notmuch mail system."
\r
42 <notmuch.notmuchmail.org>
\r
43 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
\r
44 <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
\r
45 List-Archive: <http://notmuchmail.org/pipermail/notmuch>
\r
46 List-Post: <mailto:notmuch@notmuchmail.org>
\r
47 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
\r
48 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
\r
49 <mailto:notmuch-request@notmuchmail.org?subject=subscribe>
\r
50 X-List-Received-Date: Sun, 30 Sep 2012 08:53:21 -0000
\r
52 On Sun, Sep 30 2012, Jani Nikula <jani@nikula.org> wrote:
\r
54 > notmuch-hello (called also through notmuch-hello-update, bound to '='
\r
55 > by default) tries to find the widget under or following point before
\r
56 > refresh, and put the point back to the widget afterwards. The code has
\r
57 > grown quite complicated, and has at least the following issues:
\r
59 > 1) All the individual section functions have to include code to
\r
60 > support point placement. If there is no such support, point is
\r
61 > dropped to the search box. Only saved searches and all tags
\r
62 > sections support point placement.
\r
64 > 2) Point placement is based on widget-value. If there are two widgets
\r
65 > with the same widget-value (for example a saved search with the
\r
66 > same name as a tag) the point is moved to the earlier one, even if
\r
67 > point was on the later one.
\r
69 > 3) When first entering notmuch-hello notmuch-hello-target is nil, and
\r
70 > point is dropped to the search box.
\r
72 > Moving the point to the search box is annoying because the user is
\r
73 > required to move the point before being able to enter key bindings.
\r
75 > Simplify the code by removing all point placement based on widgets, as
\r
76 > it does not work properly, and trying to fix that would unnecessarily
\r
77 > complicate the code.
\r
79 > Save current line and column before refresh, and restore them
\r
80 > afterwards. Sometimes, if notmuch-show-empty-saved-searches is nil,
\r
81 > and the refresh adds or removes saved searches from the list, this has
\r
82 > the appearance of moving the point relative to the nearest
\r
83 > widgets. This is a much smaller and less frequent problem than the
\r
84 > ones listed above.
\r
94 > v2 of id:"1348958664-1767-1-git-send-email-jani@nikula.org": More
\r
95 > (found-)target-pos cleanup, and use line and column instead of point
\r
96 > directly, both suggested by Austin.
\r
98 > emacs/notmuch-hello.el | 108 ++++++++++++++++--------------------------------
\r
99 > 1 file changed, 35 insertions(+), 73 deletions(-)
\r
101 > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
\r
102 > index 684bedc..052aaeb 100644
\r
103 > --- a/emacs/notmuch-hello.el
\r
104 > +++ b/emacs/notmuch-hello.el
\r
105 > @@ -154,11 +154,6 @@ International Bureau of Weights and Measures."
\r
106 > (defvar notmuch-hello-url "http://notmuchmail.org"
\r
107 > "The `notmuch' web site.")
\r
109 > -(defvar notmuch-hello-search-pos nil
\r
110 > - "Position of search widget, if any.
\r
112 > -This should only be set by `notmuch-hello-insert-search'.")
\r
114 > (defvar notmuch-hello-custom-section-options
\r
115 > '((:filter (string :tag "Filter for each tag"))
\r
116 > (:filter-count (string :tag "Different filter to generate message counts"))
\r
117 > @@ -209,11 +204,8 @@ function produces a section simply by adding content to the current
\r
118 > buffer. A section should not end with an empty line, because a
\r
119 > newline will be inserted after each section by `notmuch-hello'.
\r
121 > -Each function should take no arguments. If the produced section
\r
122 > -includes `notmuch-hello-target' (i.e. cursor should be positioned
\r
123 > -inside this section), the function should return this element's
\r
125 > -Otherwise, it should return nil.
\r
126 > +Each function should take no arguments. The return value is
\r
129 > For convenience an element can also be a list of the form (FUNC ARG1
\r
130 > ARG2 .. ARGN) in which case FUNC will be applied to the rest of the
\r
131 > @@ -240,15 +232,6 @@ supported for \"Customized queries section\" items."
\r
132 > notmuch-hello-query-section
\r
133 > (function :tag "Custom section"))))
\r
135 > -(defvar notmuch-hello-target nil
\r
136 > - "Button text at position of point before rebuilding the notmuch-buffer.
\r
138 > -This variable contains the text of the button, if any, the
\r
139 > -point was positioned at before the notmuch-hello buffer was
\r
140 > -rebuilt. This should never actually be global and is defined as a
\r
141 > -defvar only for documentation purposes and to avoid a compiler
\r
142 > -warning about it occurring as a free variable.")
\r
144 > (defvar notmuch-hello-hidden-sections nil
\r
145 > "List of sections titles whose contents are hidden")
\r
147 > @@ -435,8 +418,7 @@ Such a list can be computed with `notmuch-hello-query-counts'."
\r
148 > (reordered-list (notmuch-hello-reflect searches tags-per-line))
\r
149 > ;; Hack the display of the buttons used.
\r
150 > (widget-push-button-prefix "")
\r
151 > - (widget-push-button-suffix "")
\r
152 > - (found-target-pos nil))
\r
153 > + (widget-push-button-suffix ""))
\r
154 > ;; dme: It feels as though there should be a better way to
\r
155 > ;; implement this loop than using an incrementing counter.
\r
156 > (mapc (lambda (elem)
\r
157 > @@ -449,8 +431,6 @@ Such a list can be computed with `notmuch-hello-query-counts'."
\r
158 > (msg-count (third elem)))
\r
159 > (widget-insert (format "%8s "
\r
160 > (notmuch-hello-nice-number msg-count)))
\r
161 > - (if (string= name notmuch-hello-target)
\r
162 > - (setq found-target-pos (point-marker)))
\r
163 > (widget-create 'push-button
\r
164 > :notify #'notmuch-hello-widget-search
\r
165 > :notmuch-search-terms query
\r
166 > @@ -466,8 +446,7 @@ Such a list can be computed with `notmuch-hello-query-counts'."
\r
167 > ;; If the last line was not full (and hence did not include a
\r
168 > ;; carriage return), insert one now.
\r
169 > (unless (eq (% count tags-per-line) 0)
\r
170 > - (widget-insert "\n"))
\r
171 > - found-target-pos))
\r
172 > + (widget-insert "\n"))))
\r
174 > (defimage notmuch-hello-logo ((:type png :file "notmuch-logo.png")))
\r
176 > @@ -571,8 +550,7 @@ Complete list of currently available key bindings:
\r
177 > (funcall notmuch-saved-search-sort-function
\r
178 > notmuch-saved-searches)
\r
179 > notmuch-saved-searches)
\r
180 > - :show-empty-searches notmuch-show-empty-saved-searches))
\r
181 > - found-target-pos)
\r
182 > + :show-empty-searches notmuch-show-empty-saved-searches)))
\r
184 > (widget-insert "Saved searches: ")
\r
185 > (widget-create 'push-button
\r
186 > @@ -581,15 +559,12 @@ Complete list of currently available key bindings:
\r
188 > (widget-insert "\n\n")
\r
189 > (let ((start (point)))
\r
190 > - (setq found-target-pos
\r
191 > - (notmuch-hello-insert-buttons searches))
\r
192 > - (indent-rigidly start (point) notmuch-hello-indent)
\r
193 > - found-target-pos))))
\r
194 > + (notmuch-hello-insert-buttons searches)
\r
195 > + (indent-rigidly start (point) notmuch-hello-indent)))))
\r
197 > (defun notmuch-hello-insert-search ()
\r
198 > "Insert a search widget."
\r
199 > (widget-insert "Search: ")
\r
200 > - (setq notmuch-hello-search-pos (point-marker))
\r
201 > (widget-create 'editable-field
\r
202 > ;; Leave some space at the start and end of the
\r
204 > @@ -689,16 +664,13 @@ Supports the following entries in OPTIONS as a plist:
\r
205 > (notmuch-hello-update))
\r
207 > (widget-insert "\n")
\r
208 > - (let (target-pos)
\r
209 > - (when (not is-hidden)
\r
210 > - (let ((searches (apply 'notmuch-hello-query-counts query-alist options)))
\r
211 > - (when (or (not (plist-get options :hide-if-empty))
\r
213 > - (widget-insert "\n")
\r
214 > - (setq target-pos
\r
215 > - (notmuch-hello-insert-buttons searches))
\r
216 > - (indent-rigidly start (point) notmuch-hello-indent))))
\r
218 > + (when (not is-hidden)
\r
219 > + (let ((searches (apply 'notmuch-hello-query-counts query-alist options)))
\r
220 > + (when (or (not (plist-get options :hide-if-empty))
\r
222 > + (widget-insert "\n")
\r
223 > + (notmuch-hello-insert-buttons searches)
\r
224 > + (indent-rigidly start (point) notmuch-hello-indent))))))
\r
226 > (defun notmuch-hello-insert-tags-section (&optional title &rest options)
\r
227 > "Insert a section displaying all tags with message counts.
\r
228 > @@ -763,13 +735,8 @@ following:
\r
229 > (set-buffer "*notmuch-hello*")
\r
230 > (switch-to-buffer "*notmuch-hello*"))
\r
232 > - (let ((notmuch-hello-target (if (widget-at)
\r
233 > - (widget-value (widget-at))
\r
234 > - (condition-case nil
\r
236 > - (widget-forward 1)
\r
237 > - (widget-value (widget-at)))
\r
239 > + (let ((target-line (line-number-at-pos))
\r
240 > + (target-column (current-column))
\r
241 > (inhibit-read-only t))
\r
243 > ;; Delete all editable widget fields. Editable widget fields are
\r
244 > @@ -788,30 +755,25 @@ following:
\r
245 > (mapc 'delete-overlay (car all))
\r
246 > (mapc 'delete-overlay (cdr all)))
\r
248 > - (let (final-target-pos)
\r
250 > - (lambda (section)
\r
251 > - (let ((point-before (point))
\r
252 > - (result (if (functionp section)
\r
253 > - (funcall section)
\r
254 > - (apply (car section) (cdr section)))))
\r
255 > - (if (and (not final-target-pos) (integer-or-marker-p result))
\r
256 > - (setq final-target-pos result))
\r
257 > - ;; don't insert a newline when the previous section didn't show
\r
259 > - (unless (eq (point) point-before)
\r
260 > - (widget-insert "\n"))))
\r
261 > - notmuch-hello-sections)
\r
264 > - (when final-target-pos
\r
265 > - (goto-char final-target-pos)
\r
266 > - (unless (widget-at)
\r
267 > - (widget-forward 1)))
\r
269 > - (unless (widget-at)
\r
270 > - (when notmuch-hello-search-pos
\r
271 > - (goto-char notmuch-hello-search-pos)))))
\r
273 > + (lambda (section)
\r
274 > + (let ((point-before (point)))
\r
275 > + (if (functionp section)
\r
276 > + (funcall section)
\r
277 > + (apply (car section) (cdr section)))
\r
278 > + ;; don't insert a newline when the previous section didn't
\r
279 > + ;; show anything.
\r
280 > + (unless (eq (point) point-before)
\r
281 > + (widget-insert "\n"))))
\r
282 > + notmuch-hello-sections)
\r
285 > + ;; Move point back to where it was before refresh. Use line and
\r
286 > + ;; column instead of point directly to be insensitive to additions
\r
287 > + ;; and removals of text within earlier lines.
\r
288 > + (goto-char (point-min))
\r
289 > + (forward-line (1- target-line))
\r
290 > + (move-to-column target-column))
\r
291 > (run-hooks 'notmuch-hello-refresh-hook)
\r
292 > (setq notmuch-hello-first-run nil))
\r
297 > _______________________________________________
\r
298 > notmuch mailing list
\r
299 > notmuch@notmuchmail.org
\r
300 > http://notmuchmail.org/mailman/listinfo/notmuch
\r