unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* 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  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 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 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 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).