Re: [PATCH] emacs: wash: make word-wrap bound message width
[notmuch-archives.git] / 0e / e8909648371c8a76e357e4805c399638417f49
1 Return-Path: <amdragon@mit.edu>\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 2CCD0431FBF\r
6         for <notmuch@notmuchmail.org>; Sat, 29 Sep 2012 16:58:36 -0700 (PDT)\r
7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
8 X-Spam-Flag: NO\r
9 X-Spam-Score: -0.7\r
10 X-Spam-Level: \r
11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\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 NtBmrqh8Tr3F for <notmuch@notmuchmail.org>;\r
16         Sat, 29 Sep 2012 16:58:34 -0700 (PDT)\r
17 Received: from dmz-mailsec-scanner-8.mit.edu (DMZ-MAILSEC-SCANNER-8.MIT.EDU\r
18         [18.7.68.37])\r
19         by olra.theworths.org (Postfix) with ESMTP id 30629431FBC\r
20         for <notmuch@notmuchmail.org>; Sat, 29 Sep 2012 16:58:34 -0700 (PDT)\r
21 X-AuditID: 12074425-b7fcc6d00000091f-91-50678b29a175\r
22 Received: from mailhub-auth-4.mit.edu ( [18.7.62.39])\r
23         by dmz-mailsec-scanner-8.mit.edu (Symantec Messaging Gateway) with SMTP\r
24         id 6A.3D.02335.92B87605; Sat, 29 Sep 2012 19:58:33 -0400 (EDT)\r
25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
26         by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id q8TNwWiC016507; \r
27         Sat, 29 Sep 2012 19:58:33 -0400\r
28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
29         (authenticated bits=0)\r
30         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
31         by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q8TNwUhZ005141\r
32         (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
33         Sat, 29 Sep 2012 19:58:31 -0400 (EDT)\r
34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
35         (envelope-from <amdragon@mit.edu>)\r
36         id 1TI6vW-0006xx-GH; Sat, 29 Sep 2012 19:58:30 -0400\r
37 Date: Sat, 29 Sep 2012 19:58:30 -0400\r
38 From: Austin Clements <amdragon@MIT.EDU>\r
39 To: Jani Nikula <jani@nikula.org>\r
40 Subject: Re: [PATCH] emacs: simplify point placement in notmuch-hello refresh\r
41 Message-ID: <20120929235830.GZ26662@mit.edu>\r
42 References: <1348958664-1767-1-git-send-email-jani@nikula.org>\r
43 MIME-Version: 1.0\r
44 Content-Type: text/plain; charset=us-ascii\r
45 Content-Disposition: inline\r
46 In-Reply-To: <1348958664-1767-1-git-send-email-jani@nikula.org>\r
47 User-Agent: Mutt/1.5.21 (2010-09-15)\r
48 X-Brightmail-Tracker:\r
49  H4sIAAAAAAAAA+NgFmpileLIzCtJLcpLzFFi42IRYrdT19XsTg8wuLxa1aJpurPF9ZszmR2Y\r
50         PG7df83u8WzVLeYApigum5TUnMyy1CJ9uwSujNsNX9gLNvtVbOq7xtjAOMu2i5GTQ0LAROLI\r
51         jx1sELaYxIV764FsLg4hgX2MEtc/rGCCcDYwSny8tJQFpEpI4CSTxMdzuhCJJYwSF1vXgCVY\r
52         BFQl/q1bzwxiswloSGzbv5wRxBYRUJTYfHI/mM0sIC3x7XczE4gtLOArcXPVNrDVvAI6Eh/W\r
53         9TFCLLCTeLzpC1RcUOLkzCcsEL1aEjf+vQTq5QCbs/wfB0iYU8Be4vnkj2CtogIqElNObmOb\r
54         wCg0C0n3LCTdsxC6FzAyr2KUTcmt0s1NzMwpTk3WLU5OzMtLLdK10MvNLNFLTSndxAgOahfV\r
55         HYwTDikdYhTgYFTi4b15OjVAiDWxrLgy9xCjJAeTkiivZUd6gBBfUn5KZUZicUZ8UWlOavEh\r
56         RgkOZiUR3oxioHLelMTKqtSifJiUNAeLkjjvjZSb/kIC6YklqdmpqQWpRTBZGQ4OJQneyV1A\r
57         QwWLUtNTK9Iyc0oQ0kwcnCDDeYCGe4HU8BYXJOYWZ6ZD5E8xKkqJ82aBJARAEhmleXC9sKTz\r
58         ilEc6BVh3iCQKh5gwoLrfgU0mAlocNWqNJDBJYkIKakGxqiH9otL7ogGLvGbVHpoLUPavQTu\r
59         FRtZvjbsumXXuXiRx0y7K2knLhxQ8yywVf70OcTzEJPBuogXEmvuqP1Ue5u68RP/Tx/Jvini\r
60         O+7nuMyfIL7vELf3hc6zjdOedCerxUkyRn6U89raeGWduZLK3bboG0rZtZ8nCVUX55WbRFeJ\r
61         6cw7oNEVr8RSnJFoqMVcVJwIAPy8KngVAwAA\r
62 Cc: notmuch@notmuchmail.org\r
63 X-BeenThere: notmuch@notmuchmail.org\r
64 X-Mailman-Version: 2.1.13\r
65 Precedence: list\r
66 List-Id: "Use and development of the notmuch mail system."\r
67         <notmuch.notmuchmail.org>\r
68 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
69         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
70 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
71 List-Post: <mailto:notmuch@notmuchmail.org>\r
72 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
73 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
74         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
75 X-List-Received-Date: Sat, 29 Sep 2012 23:58:36 -0000\r
76 \r
77 LGTM.  I wasn't aware notmuch-hello even attempted to maintain point,\r
78 since I'd never seen it work.\r
79 \r
80 This patch could go a step further and remove found-target-pos from\r
81 notmuch-hello-insert-saved-searches, target-pos from\r
82 notmuch-hello-insert-searches, and found-target-pos from\r
83 notmuch-hello-insert-buttons.\r
84 \r
85 It might be more stable to record the line number and column count and\r
86 restore those, since point is sensitive to additions and removals of\r
87 text within earlier lines.\r
88 \r
89 \r
90 On a more abstract note, I feel like we have this same problem all\r
91 over the Emacs UI: we "update" some UI by completely rebuilding it,\r
92 and then we make some ad hoc, more-or-less (usually less) intelligent\r
93 guess about where point should go.  notmuch-hello has this problem,\r
94 updating search-mode lines on tag change has this problem, refreshing\r
95 the search buffer has this problem, and various buffer-rebuilding\r
96 toggles in show have this problem.\r
97 \r
98 Proper incremental UI updates are simply not an option for some of\r
99 these cases, because we have to call out to the CLI to get new data\r
100 and that data might be completely different from what we're currently\r
101 showing.  We could restructure things to support this---for example,\r
102 when toggling crypto, show could ask for the body of each message it's\r
103 already showing---but this would complicate the code and I'm not sure\r
104 we can restructure cases where the set of objects may change.\r
105 \r
106 But I wonder if there's an abstraction that would let us fully rebuild\r
107 UIs or parts of UIs, but keep enough context to make this fluid.  I'm\r
108 imagining something like representing a buffer as a tree of UI nodes,\r
109 where the buffer itself is simply the flattening of this tree, and\r
110 each UI node has a identifier that's unique among its siblings.  For\r
111 example, the search buffer would have a root child for each search\r
112 result, and then each piece of the result line would be a child of\r
113 that line's node.  This mechanism would give us a persistent context.\r
114 Before an update, it could record the identifiers of all of the nodes\r
115 in the path leading to the leaf node containing point, along with all\r
116 of the prior siblings of each of these nodes and the offset of point\r
117 within the leaf node.  After and update, it could replay this path as\r
118 best as possible: at each step in the path, if it can find a node with\r
119 the same identifier as the node in the path, it would descend into it;\r
120 otherwise, it would descend into the last recorded sibling it can find\r
121 in the new tree.  When this decent reaches a leaf, it can place point\r
122 there.  This same mechanism could be used to record and restore the\r
123 window start position, as well as other markers we may need.\r
124 \r
125 Quoth Jani Nikula on Sep 30 at  1:44 am:\r
126 > notmuch-hello (called also through notmuch-hello-update, bound to '='\r
127 > by default) tries to find the widget under or following point before\r
128 > refresh, and put the point back to the widget afterwards. The code has\r
129 > grown quite complicated, and has at least the following issues:\r
130\r
131 > 1) All the individual section functions have to include code to\r
132 >    support point placement. If there is no such support, point is\r
133 >    dropped to the search box. Only saved searches and all tags\r
134 >    sections support point placement.\r
135\r
136 > 2) Point placement is based on widget-value. If there are two widgets\r
137 >    with the same widget-value (for example a saved search with the\r
138 >    same name as a tag) the point is moved to the earlier one, even if\r
139 >    point was on the later one.\r
140\r
141 > 3) When first entering notmuch-hello notmuch-hello-target is nil, and\r
142 >    point is dropped to the search box.\r
143\r
144 > Moving the point to the search box is annoying because the user is\r
145 > required to move the point before being able to enter key bindings.\r
146\r
147 > Simplify the code by removing all point placement based on widgets, as\r
148 > it does not work properly, and trying to fix that would unnecessarily\r
149 > complicate the code.\r
150\r
151 > Point is simply saved before refresh, and put back to where it\r
152 > was. Sometimes, if notmuch-show-empty-saved-searches is nil, and the\r
153 > refresh adds or removes saved searches from the list, this has the\r
154 > appearance of moving the point relative to the nearest widgets. This\r
155 > is a much smaller and less frequent problem than the ones listed\r
156 > above.\r
157\r
158 > ---\r
159\r
160 > I've sent this before months ago, and received comments starting at\r
161 > id:"87aa2aohjs.fsf@gmail.com". I find the current behaviour annoying,\r
162 > and I prefer this simple fix over complicating the existing\r
163 > mess. Nobody has stood up to do anything better anyway. Perhaps we\r
164 > could take a step forward with this while waiting for Someone(tm) to\r
165 > come up with the perfect point placement?\r
166 > ---\r
167 >  emacs/notmuch-hello.el |   70 ++++++++++++------------------------------------\r
168 >  1 file changed, 17 insertions(+), 53 deletions(-)\r
169\r
170 > diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el\r
171 > index 684bedc..04d90cf 100644\r
172 > --- a/emacs/notmuch-hello.el\r
173 > +++ b/emacs/notmuch-hello.el\r
174 > @@ -154,11 +154,6 @@ International Bureau of Weights and Measures."\r
175 >  (defvar notmuch-hello-url "http://notmuchmail.org"\r
176 >    "The `notmuch' web site.")\r
177 >  \r
178 > -(defvar notmuch-hello-search-pos nil\r
179 > -  "Position of search widget, if any.\r
180 > -\r
181 > -This should only be set by `notmuch-hello-insert-search'.")\r
182 > -\r
183 >  (defvar notmuch-hello-custom-section-options\r
184 >    '((:filter (string :tag "Filter for each tag"))\r
185 >      (:filter-count (string :tag "Different filter to generate message counts"))\r
186 > @@ -209,11 +204,8 @@ function produces a section simply by adding content to the current\r
187 >  buffer.  A section should not end with an empty line, because a\r
188 >  newline will be inserted after each section by `notmuch-hello'.\r
189 >  \r
190 > -Each function should take no arguments.  If the produced section\r
191 > -includes `notmuch-hello-target' (i.e. cursor should be positioned\r
192 > -inside this section), the function should return this element's\r
193 > -position.\r
194 > -Otherwise, it should return nil.\r
195 > +Each function should take no arguments. The return value is\r
196 > +ignored.\r
197 >  \r
198 >  For convenience an element can also be a list of the form (FUNC ARG1\r
199 >  ARG2 .. ARGN) in which case FUNC will be applied to the rest of the\r
200 > @@ -240,15 +232,6 @@ supported for \"Customized queries section\" items."\r
201 >           notmuch-hello-query-section\r
202 >           (function :tag "Custom section"))))\r
203 >  \r
204 > -(defvar notmuch-hello-target nil\r
205 > -  "Button text at position of point before rebuilding the notmuch-buffer.\r
206 > -\r
207 > -This variable contains the text of the button, if any, the\r
208 > -point was positioned at before the notmuch-hello buffer was\r
209 > -rebuilt. This should never actually be global and is defined as a\r
210 > -defvar only for documentation purposes and to avoid a compiler\r
211 > -warning about it occurring as a free variable.")\r
212 > -\r
213 >  (defvar notmuch-hello-hidden-sections nil\r
214 >    "List of sections titles whose contents are hidden")\r
215 >  \r
216 > @@ -449,8 +432,6 @@ Such a list can be computed with `notmuch-hello-query-counts'."\r
217 >                    (msg-count (third elem)))\r
218 >               (widget-insert (format "%8s "\r
219 >                                      (notmuch-hello-nice-number msg-count)))\r
220 > -             (if (string= name notmuch-hello-target)\r
221 > -                 (setq found-target-pos (point-marker)))\r
222 >               (widget-create 'push-button\r
223 >                              :notify #'notmuch-hello-widget-search\r
224 >                              :notmuch-search-terms query\r
225 > @@ -589,7 +570,6 @@ Complete list of currently available key bindings:\r
226 >  (defun notmuch-hello-insert-search ()\r
227 >    "Insert a search widget."\r
228 >    (widget-insert "Search: ")\r
229 > -  (setq notmuch-hello-search-pos (point-marker))\r
230 >    (widget-create 'editable-field\r
231 >                ;; Leave some space at the start and end of the\r
232 >                ;; search boxes.\r
233 > @@ -763,13 +743,7 @@ following:\r
234 >        (set-buffer "*notmuch-hello*")\r
235 >      (switch-to-buffer "*notmuch-hello*"))\r
236 >  \r
237 > -  (let ((notmuch-hello-target (if (widget-at)\r
238 > -                               (widget-value (widget-at))\r
239 > -                             (condition-case nil\r
240 > -                                 (progn\r
241 > -                                   (widget-forward 1)\r
242 > -                                   (widget-value (widget-at)))\r
243 > -                               (error nil))))\r
244 > +  (let ((final-target-pos (point))\r
245 >       (inhibit-read-only t))\r
246 >  \r
247 >      ;; Delete all editable widget fields.  Editable widget fields are\r
248 > @@ -788,30 +762,20 @@ following:\r
249 >        (mapc 'delete-overlay (car all))\r
250 >        (mapc 'delete-overlay (cdr all)))\r
251 >  \r
252 > -    (let (final-target-pos)\r
253 > -      (mapc\r
254 > -       (lambda (section)\r
255 > -      (let ((point-before (point))\r
256 > -            (result (if (functionp section)\r
257 > -                        (funcall section)\r
258 > -                      (apply (car section) (cdr section)))))\r
259 > -        (if (and (not final-target-pos) (integer-or-marker-p result))\r
260 > -            (setq final-target-pos result))\r
261 > -        ;; don't insert a newline when the previous section didn't show\r
262 > -        ;; anything.\r
263 > -        (unless (eq (point) point-before)\r
264 > -          (widget-insert "\n"))))\r
265 > -       notmuch-hello-sections)\r
266 > -      (widget-setup)\r
267 > -\r
268 > -      (when final-target-pos\r
269 > -     (goto-char final-target-pos)\r
270 > -     (unless (widget-at)\r
271 > -       (widget-forward 1)))\r
272 > -\r
273 > -      (unless (widget-at)\r
274 > -     (when notmuch-hello-search-pos\r
275 > -       (goto-char notmuch-hello-search-pos)))))\r
276 > +    (mapc\r
277 > +     (lambda (section)\r
278 > +       (let ((point-before (point)))\r
279 > +      (if (functionp section)\r
280 > +          (funcall section)\r
281 > +        (apply (car section) (cdr section)))\r
282 > +      ;; don't insert a newline when the previous section didn't\r
283 > +      ;; show anything.\r
284 > +      (unless (eq (point) point-before)\r
285 > +        (widget-insert "\n"))))\r
286 > +     notmuch-hello-sections)\r
287 > +    (widget-setup)\r
288 > +\r
289 > +    (goto-char final-target-pos))\r
290 >    (run-hooks 'notmuch-hello-refresh-hook)\r
291 >    (setq notmuch-hello-first-run nil))\r
292 >  \r
293 \r
294 -- \r
295 Austin Clements                                      MIT/'06/PhD/CSAIL\r
296 amdragon@mit.edu                           http://web.mit.edu/amdragon\r
297        Somewhere in the dream we call reality you will find me,\r
298               searching for the reality we call dreams.\r