all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Anders Lindgren <andlind@gmail.com>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 16431@debbugs.gnu.org
Subject: bug#16431: 24.3.50; `follow-adjust-windows' sometimes fails (patch included)
Date: Thu, 16 Jan 2014 11:20:50 +0100	[thread overview]
Message-ID: <CABr8ebZ+UPhp077a1Ay5jeVNSn6nxgwokojsysHrvOSYEaTu6Q@mail.gmail.com> (raw)
In-Reply-To: <jwvmwiwsgc1.fsf-monnier+emacsbugs@gnu.org>

[-- 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 --]

  parent reply	other threads:[~2014-01-16 10:20 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABr8ebZ+UPhp077a1Ay5jeVNSn6nxgwokojsysHrvOSYEaTu6Q@mail.gmail.com \
    --to=andlind@gmail.com \
    --cc=16431@debbugs.gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.