* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) @ 2014-01-13 12:13 Anders Lindgren 2014-01-16 4:39 ` Stefan Monnier 0 siblings, 1 reply; 12+ messages in thread From: Anders Lindgren @ 2014-01-13 12:13 UTC (permalink / raw) To: 16431 [-- Attachment #1: Type: text/plain, Size: 5390 bytes --] The function `follow-adjust-windows' (part of follow-mode) sometimes fails. Steps to repeat: emacs -Q C-h t C-x 3 C-x 3 M-x balance-windows RET C-x b *scratch* RET Eval the following three expressions: (require 'follow) (let ((owin (selected-window))) (select-window (next-window)) (let ((follow-mode t)) (follow-adjust-window (selected-window) (point-min))) (select-window owin)) OK here, beginning of the buffer is visible. (let ((owin (selected-window))) (select-window (next-window)) (let ((follow-mode t)) (follow-adjust-window (selected-window) (/ (point-max) 2)) (select-window owin))) Here, the intention is that the middle of the buffer should be visible. It's not. This is broken in 24.3 as well as on the trunk. The function was introduced in 24.3, even though similar code was present in `follow-post-command-hook' earlier. This is due to the fact that the function calls `(redisplay)'. However, this work on the current point, which has not been maintained. Fortunately, there seems to be an easy fix, see patch below: 1226,1232c1226,1232 < (goto-char dest) < (redisplay) < ;; If this `redisplay' moved point, we got clobbered by a < ;; previous call to `set-window-start'. Try again. < (when (/= (point) dest) < (goto-char dest) < (redisplay)) --- > (let ((opoint (point))) > (redisplay) > ;; If this `redisplay' moved point, we got clobbered by a > ;; previous call to `set-window-start'. Try again. > (when (/= (point) opoint) > (goto-char opoint) > (redisplay))) Sincerely, Anders Lindgren Ps. I'm the original author of Follow mode, even though I haven't worked on the package for many years. In GNU Emacs 24.3.50.2 (x86_64-apple-darwin13.0.0, NS apple-appkit-1265.00) of 2014-01-13 on macpro.lan Repository revision: 116005 juri@jurta.org-20140113080409-nncncbxckrayrogi Windowing system distributor `Apple', version 10.3.1265 Configured using: `configure --with-ns' Important settings: value of $LC_CTYPE: UTF-8 locale-coding-system: utf-8-unix Major mode: Lisp Interaction Minor modes in effect: tooltip-mode: t electric-indent-mode: t mouse-wheel-mode: t tool-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t line-number-mode: t transient-mark-mode: t Recent input: <left> <left> <left> <left> <left> <left> <left> <left> <left> <down> C-j <up> <up> <up> C-e <left> <left> <left> <left> <left> <left> <left> <left> <left> <left> <left> <left> <left> ( / SPC <C-s-268632070> <escape> d ( p o i n t <right> <right> <right> <right> <right> SPC 2 C-e <down> ) C-j <up> <up> <up> <up> <up> <up> <up> <up> C-e C-j <down> <down> <down> <down> <down> <down> C-e C-j <up> C-SPC <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <escape> w C-x o C-s a d j u s t C-w C-s C-s C-s C-a C-s a d j u s t - w C-w s-b s-b s-b s-b s-b <escape> b <escape> b <escape> b C-s C-w C-w C-w C-s C-s C-a C-SPC <C-s-268632070> <escape> C-f <escape> w C-x p C-x o C-x o <escape> > C-y <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <return> ( C-a C-k C-k <up> <return> <up> <tab> ( g t o <backspace> <backspace> o t o - c h a r SPC d e s t ) C-a C-x C-s C-g <down> <down> <escape> C-x <escape> < <up> <down> <down> <down> <down> <down> <right> <right> <right> <down> <down> <down> <down> <escape> C-x <down> <down> <down> <down> <down> <down> <down> <down> <escape> C-x <up> <up> <up> <up> <up> C-a C-SPC <down> C-w C-x C-s C-g <escape> x e m a c s - r e p <tab> <backspace> <tab> <backspace> <backspace> <s-backspace> <backspace> <backspace> <backspace> <backspace> <backspace> <backspace> r e p o r t - b <tab> <return> Recent messages: C-x p is undefined Mark set [2 times] Quit follow-adjust-window Mark set Beginning of buffer #<window 3 on *scratch*> [2 times] Mark set Quit <s-backspace> is undefined Load-path shadows: None found. Features: (shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils misearch multi-isearch vc-bzr jka-compr find-func pp help-fns follow debug tutorial help-mode easymenu time-date tooltip electric uniquify ediff-hook vc-hooks lisp-float-type mwheel ns-win tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment lisp-mode prog-mode register page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice loaddefs button faces cus-face macroexp files text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote make-network-process cocoa ns multi-tty emacs) [-- Attachment #2: Type: text/html, Size: 8550 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) 2014-01-13 12:13 bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) Anders Lindgren @ 2014-01-16 4:39 ` Stefan Monnier 2014-01-16 10:03 ` martin rudalics ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Stefan Monnier @ 2014-01-16 4:39 UTC (permalink / raw) To: Anders Lindgren; +Cc: 16431 > (let ((owin (selected-window))) > (select-window (next-window)) > (let ((follow-mode t)) > (follow-adjust-window (selected-window) (/ (point-max) 2)) > (select-window owin))) This recipe doesn't work any more now that follow-adjust-window only takes a single argument. Do you have some other way to reproduce the original problem (or has it been accidentally fixed by my recent commit)? > 1226,1232c1226,1232 > < (goto-char dest) > < (redisplay) Your patch (just like the one in your other bug-report) has spurious empty lines between every actual line. Also I think it was reversed (at least the "destination" code is the one that's been in Emacs for a while now). Finally, please use "diff -u" or "diff -c" format for patches. And since I'm here, thinking about how to better support follow-mode, here's an idea: IIUC the main problem currently (other than the "empty last buffers") is that we have to redisplay in order to find the window-end, after which we can adjust subsequent windows, and force another redisplay. So we'd like redisplay of the various windows to be done "in the right order" and be able to run some Elisp code in-between. One option for that would be to provide a new `redisplay-window' function which does a redisplay of only one window, and then use it inside pre-redisplay-function. This way, when we do a normal redisplay, our follow-pre-redisplay-function would check if some of the windows use follow-mode. If so, follow-pre-redisplay-function would redisplay its windows "manually" in the right order via `redisplay-window' (and setup window-starts between each window, as appropriate). After that the normal redisplay would continue and would find those windows "already up-to-date", which would avoid the double-redisplay. Stefan ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) 2014-01-16 4:39 ` Stefan Monnier @ 2014-01-16 10:03 ` martin rudalics 2014-01-16 10:20 ` Anders Lindgren 2014-01-16 17:53 ` Eli Zaretskii 2 siblings, 0 replies; 12+ messages in thread From: martin rudalics @ 2014-01-16 10:03 UTC (permalink / raw) To: Stefan Monnier; +Cc: 16431, Anders Lindgren > And since I'm here, thinking about how to better support follow-mode, > here's an idea: IIUC the main problem currently (other than the "empty > last buffers") is that we have to redisplay in order to find the > window-end, after which we can adjust subsequent windows, and force > another redisplay. I'd call `window-text-pixel-size' for each window in row ;-) martin ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) 2014-01-16 4:39 ` Stefan Monnier 2014-01-16 10:03 ` martin rudalics @ 2014-01-16 10:20 ` Anders Lindgren 2014-01-16 14:25 ` Stefan Monnier 2014-01-16 17:53 ` Eli Zaretskii 2 siblings, 1 reply; 12+ messages in thread From: Anders Lindgren @ 2014-01-16 10:20 UTC (permalink / raw) To: Stefan Monnier; +Cc: 16431 [-- Attachment #1: Type: text/plain, Size: 6851 bytes --] Hi! (Eli, I've included you in this as we have a parallel discussion on Follow mode going on. The last part of this letter is of interest to you.) On Thu, Jan 16, 2014 at 5:39 AM, Stefan Monnier <monnier@iro.umontreal.ca>wrote: > > (let ((owin (selected-window))) > > (select-window (next-window)) > > (let ((follow-mode t)) > > (follow-adjust-window (selected-window) (/ (point-max) 2)) > > (select-window owin))) > > This recipe doesn't work any more now that follow-adjust-window only > takes a single argument. > > Do you have some other way to reproduce the original problem (or has it > been accidentally fixed by my recent commit)? Right now, I don't see the problem, even when I have rewritten the code to match the new way to call the function. Maybe the fact that the window buffer is current made things much better... There are a number of `goto-char' in the code, which would have been applied to the wrong buffer earlier, so that could explain the situation. You can close it for now and if I see the problem again, I will let you know. I think that we would need to document the function a little bit better. What about: "Adjust the window WIN and its followers so that the point is visible. The window containing the point will be selected." Currently, I'm writing an application (incidentally, an interactive debugger for font-lock keywords) that should show a location in a source file, and I am trying to make it aware of Follow mode. (Like *grep*, but the source buffer should not be selected.) Unfortunately, Follow-mode was not written with this in mind (it only applies its magic to the selected window). Using `follow-adjust-window' is a step in the right direction, but it's still a bit cumbersome. This is the code I'm currently using (with the latest Emacs trunk): (let ((source-pos ....)) (let ((owin (selected-window))) (select-window win) (goto-char source-pos) (follow-adjust-window win) ;; Here, the windows are aligned, and the selected ;; window contains the location `source-pos'. ;; However, the window point is (unfortunately) ;; not set to it. (set-window-point (selected-window) source-pos) (select-window owin)) One thing still seems to be problematic -- `follow-adjust-window' only work properly when `win' is *selected*. If it's not, then the wrong buffer window will be scrolled. I think we need to fix that. (I *think* the culprit it `follow-windows-start-end' that tries to retain the original selected window, however, by doing so I think it indadvertedly change current buffer.) Also, by setting the window-point in the selected window to (point) at the end of `follow-adjust-windows', we could eliminate another line from the code above. In other words, that I would like to see is the following: (let ((source-pos ....)) (with-current-buffer (window-buffer win) (goto-char source-pos) (follow-adjust-window win))) Or, why not retain the `dest' argument (or make it optional) and let `follow-adjust-window' set the current buffer and go for: (let ((source-pos ....)) (follow-adjust-window win source-pos)) > 1226,1232c1226,1232 > > > < (goto-char dest) > > > < (redisplay) > > Your patch (just like the one in your other bug-report) has spurious > empty lines between every actual line. Also I think it was reversed > (at least the "destination" code is the one that's been in Emacs for > a while now). Finally, please use "diff -u" or "diff -c" format > for patches. > Ok, noted. > And since I'm here, thinking about how to better support follow-mode, > here's an idea: IIUC the main problem currently (other than the "empty > last buffers") is that we have to redisplay in order to find the > window-end, after which we can adjust subsequent windows, and force > another redisplay. So we'd like redisplay of the various windows to be > done "in the right order" and be able to run some Elisp code in-between. > One option for that would be to provide a new `redisplay-window' > function which does a redisplay of only one window, and then use it > inside pre-redisplay-function. This way, when we do a normal redisplay, > our follow-pre-redisplay-function would check if some of the windows use > follow-mode. If so, follow-pre-redisplay-function would redisplay its > windows "manually" in the right order via `redisplay-window' (and setup > window-starts between each window, as appropriate). After that the > normal redisplay would continue and would find those windows "already > up-to-date", which would avoid the double-redisplay. > There are two main scenarios, which I think should be addressed separately: 1) The windows are aligned. This typically happens when when you move the point around or when you simply insert some text. Today, cursor movement are handled OK since Follow-mode recognizes them and use a cached start and end values. For other commands, it needs to check if the windows are aligned, in this case `window-end' (with the "force" flag) is used. I know that Emacs internally has a "end is valid" flag, which would make `window-end' a fast operation when this is set, and probably a relatively slow one when it's not. I guess that we could speed up this by ensuring that more command would maintain the window-end value -- in particular this is important for `self-insert-command', as lagging when typing text is one of the problems I see. (Note that I'm just speculating here, maybe self-insert-command doesn't invalidate the window-end value.) When windows are aligned, Follow mode does not set the window start or do anything else to disturb the redisplay. 2) The windows are not aligned, and the point may be way off somewhere. Here, `redisplay' is issued so that we would get the the window repositioned to include the point, so that the others could be aligned around it. This happens, for example, when the user has scrolled way outside the normal viewing area, e.g. by pressing C-v. In this case, the selected window is first redisplayed so that the point would be located in a suitable place in a window and then the rest is placed nicely around it. Unfortunately, this looks like the selected window is first move and then, a couple of tenths afterwards, the others move. (Incidentally, in older Emacs versions, all windows were scrolled at the same time -- but I don't know when or why this stopped working.) In this scenario it would definitively help if there were a "window-only redisplay" or, even better, a "silent redisplay" that would calculate where the window would end up, but not actually draw anything. (Here, I must admit that my knowledge of the display engine is limited.) -- Anders [-- Attachment #2: Type: text/html, Size: 8821 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) 2014-01-16 10:20 ` Anders Lindgren @ 2014-01-16 14:25 ` Stefan Monnier 2014-01-16 15:51 ` Anders Lindgren 2014-01-16 17:56 ` Eli Zaretskii 0 siblings, 2 replies; 12+ messages in thread From: Stefan Monnier @ 2014-01-16 14:25 UTC (permalink / raw) To: Anders Lindgren; +Cc: 16431-done > You can close it for now and if I see the problem again, I will let you know. Done, thanks. > One thing still seems to be problematic -- `follow-adjust-window' only work > properly when `win' is *selected*. Indeed, I was tempted to add a (cl-assert (eq win (selected-window))) or to remove the `win' argument altogether. I didn't do it mostly because I remembered that we're in feature freeze. > In other words, that I would like to see is the following: > (let ((source-pos ....)) > (with-current-buffer (window-buffer win) > (goto-char source-pos) > (follow-adjust-window win))) I don't understand why you'd want to compute source-pos while outside of (window-buffer win). > When windows are aligned, Follow mode does not set the window start or do > anything else to disturb the redisplay. In my suggested approach, when windows are aligned, after each call to redisplay-window, window-end is cheap (since it was just computed and is hence marked as up-to-date) and since the windows are already properly aligned you'd find that window-start does not need to be updated, which in turn would ensure that the next redisplay-window is also cheap. > In this scenario it would definitively help if there were a "window-only > redisplay" or, even better, a "silent redisplay" that would calculate where > the window would end up, but not actually draw anything. (Here, I must > admit that my knowledge of the display engine is limited.) My intention would be for redisplay-window to only update the matrices (i.e. the internal representation of the rendered window) and let the subsequent full redisplay deal with updating the display. My knowledge of the display engine is also limited, but I think such a redisplay-window function should not be too hard to write. Stefan ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) 2014-01-16 14:25 ` Stefan Monnier @ 2014-01-16 15:51 ` Anders Lindgren 2014-01-16 17:57 ` Eli Zaretskii 2014-01-16 18:43 ` Stefan Monnier 2014-01-16 17:56 ` Eli Zaretskii 1 sibling, 2 replies; 12+ messages in thread From: Anders Lindgren @ 2014-01-16 15:51 UTC (permalink / raw) To: Stefan Monnier; +Cc: 16431-done [-- Attachment #1: Type: text/plain, Size: 3251 bytes --] Hi! > > One thing still seems to be problematic -- `follow-adjust-window' only > work > > properly when `win' is *selected*. > > Indeed, I was tempted to add a (cl-assert (eq win (selected-window))) or > to remove the `win' argument altogether. I didn't do it mostly because > I remembered that we're in feature freeze. > Hmm, but I would qualify that as a bug fix. Besides, you have already changed the signature of `follow-adjust-window', so in some way we are already thumbing on the rules somewhat. > In other words, that I would like to see is the following: > > > (let ((source-pos ....)) > > (with-current-buffer (window-buffer win) > > (goto-char source-pos) > > (follow-adjust-window win))) > > I don't understand why you'd want to compute source-pos while outside of > (window-buffer win). > Oh, I just wanted to show that it came from "the outside" -- it's not really important for the example. Think of this as the location of the "grep hit" or the location which "compile" should present as containing an error. (In my application, it's the location of the current search result of a font-lock rule.) Anyway, the effect that I'm after is that I want the user to walk through a number of locations in a file (again, think of grep hits). As long as the location is visible in one of the side-by-side windows I simply present the location using the overlay arrow and set the window point. Once we have passed beyond the end of the last window I want to reposition the buffer so that the new hit is visible in the middle of the first window -- all this in order to minimize buffer movement. What I want to achieve is that it should be as easy as possible (and future safe) for applications to present a buffer in Follow mode-like sense, even if the buffer is present in a window which isn't selected. > > When windows are aligned, Follow mode does not set the window start or do > > anything else to disturb the redisplay. > > In my suggested approach, when windows are aligned, after each call to > redisplay-window, window-end is cheap (since it was just computed and is > hence marked as up-to-date) and since the windows are already properly > aligned you'd find that window-start does not need to be updated, which > in turn would ensure that the next redisplay-window is also cheap. > > > In this scenario it would definitively help if there were a "window-only > > redisplay" or, even better, a "silent redisplay" that would calculate > where > > the window would end up, but not actually draw anything. (Here, I must > > admit that my knowledge of the display engine is limited.) > > My intention would be for redisplay-window to only update the matrices > (i.e. the internal representation of the rendered window) and let the > subsequent full redisplay deal with updating the display. My > knowledge of the display engine is also limited, but I think such > a redisplay-window function should not be too hard to write. Sounds like a very good idea! Another thing -- you mentioned earlier that the region highlight is nowadays done in elisp, and that it might be possible for Follow mode to make it span multiple windows. Where can I find out more about this? -- Anders [-- Attachment #2: Type: text/html, Size: 4262 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) 2014-01-16 15:51 ` Anders Lindgren @ 2014-01-16 17:57 ` Eli Zaretskii 2014-01-16 18:43 ` Stefan Monnier 1 sibling, 0 replies; 12+ messages in thread From: Eli Zaretskii @ 2014-01-16 17:57 UTC (permalink / raw) To: Anders Lindgren; +Cc: 16431-done > Date: Thu, 16 Jan 2014 16:51:50 +0100 > From: Anders Lindgren <andlind@gmail.com> > Cc: 16431-done@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org> > > Another thing -- you mentioned earlier that the region highlight is > nowadays done in elisp, and that it might be possible for Follow mode to > make it span multiple windows. Where can I find out more about this? If you turn on highlight-nonselected-windows, you can have it right now. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) 2014-01-16 15:51 ` Anders Lindgren 2014-01-16 17:57 ` Eli Zaretskii @ 2014-01-16 18:43 ` Stefan Monnier 1 sibling, 0 replies; 12+ messages in thread From: Stefan Monnier @ 2014-01-16 18:43 UTC (permalink / raw) To: Anders Lindgren; +Cc: 16431-done > Another thing -- you mentioned earlier that the region highlight is > nowadays done in elisp, and that it might be possible for Follow mode to > make it span multiple windows. Where can I find out more about this? Look for redisplay-highlight-region-function and redisplay-unhighlight-region-function. You can also grep for them so as to see some example uses (it's used currently in simple.el for the regular region stuff, and in rect.el and cua-rect.el). Stefan ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) 2014-01-16 14:25 ` Stefan Monnier 2014-01-16 15:51 ` Anders Lindgren @ 2014-01-16 17:56 ` Eli Zaretskii 1 sibling, 0 replies; 12+ messages in thread From: Eli Zaretskii @ 2014-01-16 17:56 UTC (permalink / raw) To: Stefan Monnier; +Cc: 16431-done, andlind > From: Stefan Monnier <monnier@iro.umontreal.ca> > Cc: 16431-done@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org> > Date: Thu, 16 Jan 2014 09:25:46 -0500 > > My intention would be for redisplay-window to only update the matrices > (i.e. the internal representation of the rendered window) and let the > subsequent full redisplay deal with updating the display. But if we mark the windows as up-to-date, then subsequent redisplay will see that and decide that it has nothing to do. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) 2014-01-16 4:39 ` Stefan Monnier 2014-01-16 10:03 ` martin rudalics 2014-01-16 10:20 ` Anders Lindgren @ 2014-01-16 17:53 ` Eli Zaretskii 2014-01-16 18:58 ` Stefan Monnier 2 siblings, 1 reply; 12+ messages in thread From: Eli Zaretskii @ 2014-01-16 17:53 UTC (permalink / raw) To: Stefan Monnier; +Cc: 16431, andlind > From: Stefan Monnier <monnier@iro.umontreal.ca> > Date: Wed, 15 Jan 2014 23:39:50 -0500 > Cc: 16431@debbugs.gnu.org > > And since I'm here, thinking about how to better support follow-mode, > here's an idea: IIUC the main problem currently (other than the "empty > last buffers") is that we have to redisplay in order to find the > window-end, after which we can adjust subsequent windows, and force > another redisplay. So we'd like redisplay of the various windows to be > done "in the right order" and be able to run some Elisp code in-between. > One option for that would be to provide a new `redisplay-window' > function which does a redisplay of only one window, and then use it > inside pre-redisplay-function. This way, when we do a normal redisplay, > our follow-pre-redisplay-function would check if some of the windows use > follow-mode. If so, follow-pre-redisplay-function would redisplay its > windows "manually" in the right order via `redisplay-window' (and setup > window-starts between each window, as appropriate). After that the > normal redisplay would continue and would find those windows "already > up-to-date", which would avoid the double-redisplay. Not sure if this discussion belongs here, but... I don't have a clear understanding of how your suggestion would work. As you well know, a redisplay cycle has 2 parts: first, the "desired" glyph matrices are created for each window that needs to be redisplayed, and then the differences between the "desired" and the "current" glyph matrices are computed and delivered to the glass. The second part is inside the call to update_frame. Now, the above doesn't say, but your later message indicates that by "redisplay a window" you mean only the first part. However, the current redisplay cycle marks a window's display "accurate" only after update_frame was called and was able to complete its job. I'm not sure we can mark a window "up to date" without actually delivering the stiff to the glass. Next, there's a complication on TTYs: there, update_frame updates the entire frame at once, not one window at a time (as it does on GUI displays). So the TTY redisplay cannot easily update several windows one by one without doing a lot of redundant work. The function that redisplays a window in your sense already exists: it's redisplay_window; you just need to expose it to Lisp. But calling that function from pre-redisplay-function, i.e. from inside redisplay, means you are potentially re-entering the display engine, which is non-reentrant. OTOH, the job at hand is very simple: we need to ask the display engine to redisplay several windows in a particular order, and to determine the window-start of each of these windows from the window-end of the previous one. This is a much easier job, one that doesn't require re-entering the display engine, nor usurping its job by redisplaying some windows behind its back "by hand". IOW, I'm not sure it is a good idea to use pre-redisplay-function for doing part of redisplay's job. Instead, we should let the display engine do its job, and provide some hints to it that would produce the necessary effect. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) 2014-01-16 17:53 ` Eli Zaretskii @ 2014-01-16 18:58 ` Stefan Monnier 2014-01-16 21:38 ` Eli Zaretskii 0 siblings, 1 reply; 12+ messages in thread From: Stefan Monnier @ 2014-01-16 18:58 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 16431, andlind > I don't have a clear understanding of how your suggestion would work. Don't worry: you're not alone. > Now, the above doesn't say, but your later message indicates that by > "redisplay a window" you mean only the first part. That's right. > However, the current redisplay cycle marks a window's display > "accurate" only after update_frame was called and was able to complete > its job. I'm not sure we can mark a window "up to date" without > actually delivering the stiff to the glass. Indeed, we may need to introduce a new boolean flag to distinguish the "matrix up to date" from "display up to date". I haven't had to deal with this part of the code yet, so I'm not sure it will turn out. But seen from a distance, it looks like it should be doable without any serious restructuring. > Next, there's a complication on TTYs: there, update_frame updates the > entire frame at once, not one window at a time (as it does on GUI > displays). So the TTY redisplay cannot easily update several windows > one by one without doing a lot of redundant work. That's actually good. It means there's an existing road that goes in the direction of separating the "update the matrix" vs "update the display". > The function that redisplays a window in your sense already exists: > it's redisplay_window; you just need to expose it to Lisp. More or less, yes. > But calling that function from pre-redisplay-function, i.e. from > inside redisplay, means you are potentially re-entering the display > engine, which is non-reentrant. Note that pre-redisplay-function is called from redisplay, not from redisplay_window, so having pre-redisplay-function call redisplay_window does not in itself create a circularity. Of course, the devil is in the detail, but again, from a distance it looks doable. > OTOH, the job at hand is very simple: we need to ask the display > engine to redisplay several windows in a particular order, and to > determine the window-start of each of these windows from the > window-end of the previous one. It's not quite that simple because point may want to move from one window to the other instead of scrolling. Of course, my plan currently does not account for this either (my best guess so far is that we should somehow tell redisplay-window not to change scroll and if that causes point to move it means scrolling was needed, at which point we can decide whether to move point to another window, or to call redisplay-window again (but letting it scroll this time)). > This is a much easier job, one that doesn't require re-entering the > display engine, nor usurping its job by redisplaying some windows > behind its back "by hand". Could be. As you know, I'm a big fan of moving code to Elisp to provide ever more rope for users to shoot themselves in the foot. E.g. maybe the same functionality can later be used for scroll-all-mode and two-columns-mode. Presumably, ediff could also use that kind of functionality. Stefan ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) 2014-01-16 18:58 ` Stefan Monnier @ 2014-01-16 21:38 ` Eli Zaretskii 0 siblings, 0 replies; 12+ messages in thread From: Eli Zaretskii @ 2014-01-16 21:38 UTC (permalink / raw) To: Stefan Monnier; +Cc: 16431, andlind > From: Stefan Monnier <monnier@iro.umontreal.ca> > Cc: andlind@gmail.com, 16431@debbugs.gnu.org > Date: Thu, 16 Jan 2014 13:58:41 -0500 > > > However, the current redisplay cycle marks a window's display > > "accurate" only after update_frame was called and was able to complete > > its job. I'm not sure we can mark a window "up to date" without > > actually delivering the stiff to the glass. > > Indeed, we may need to introduce a new boolean flag to distinguish the > "matrix up to date" from "display up to date". I haven't had to deal > with this part of the code yet, so I'm not sure it will turn out. > But seen from a distance, it looks like it should be doable without > any serious restructuring. > > > Next, there's a complication on TTYs: there, update_frame updates the > > entire frame at once, not one window at a time (as it does on GUI > > displays). So the TTY redisplay cannot easily update several windows > > one by one without doing a lot of redundant work. > > That's actually good. It means there's an existing road that goes in > the direction of separating the "update the matrix" vs "update the > display". Sure, that's what I said above: update_frame is where the display is being updated. In a GUI session, it calls update_window_tree, which updates the windows one by one, whereas in the TTY case it updates the entire foreground frame. In both cases, preparation of the desired matrices is separate from updating the display. > It's not quite that simple because point may want to move from one > window to the other instead of scrolling. Not sure this part should be in C, as it just selects a window. But if so, it's not hard. > > This is a much easier job, one that doesn't require re-entering the > > display engine, nor usurping its job by redisplaying some windows > > behind its back "by hand". > > Could be. As you know, I'm a big fan of moving code to Elisp to provide > ever more rope for users to shoot themselves in the foot. > > E.g. maybe the same functionality can later be used for scroll-all-mode > and two-columns-mode. Presumably, ediff could also use that kind > of functionality. Right. The hard part, as always, is to come up with a design that is flexible enough to serve these use cases, and yet simple enough to avoid placing more burden on redisplay. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-01-16 21:38 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-13 12:13 bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included) Anders Lindgren 2014-01-16 4:39 ` Stefan Monnier 2014-01-16 10:03 ` martin rudalics 2014-01-16 10:20 ` Anders Lindgren 2014-01-16 14:25 ` Stefan Monnier 2014-01-16 15:51 ` Anders Lindgren 2014-01-16 17:57 ` Eli Zaretskii 2014-01-16 18:43 ` Stefan Monnier 2014-01-16 17:56 ` Eli Zaretskii 2014-01-16 17:53 ` Eli Zaretskii 2014-01-16 18:58 ` Stefan Monnier 2014-01-16 21:38 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).