27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Phil Sainty]

(The following might all apply to the global hook as well?
I've only been testing with the buffer-local value.)


The NEWS entry "Window change functions have been redesigned."
explains that a bunch of things have changed in Emacs 27.

The particular issue I'm having seems to be due to:

"Hooks reacting to window changes run now only when
redisplay detects that a change has actually occurred."

The new behaviour still works in practice for my library[1],
but in my tests it fails (in Emacs 27 only), and I don't know
what I should be doing instead.

In a test for a buffer-local window-configuration-change-hook
I can assert that a buffer is initially not displayed, and then
call `display-buffer' and assert that it *is* now displayed in
a window -- yet the buffer's window-configuration-change-hook
function hasn't been called.  (Which, IIUC, is intentional.)


;; -*- lexical-binding:t -*-
(require 'ert)
(ert-deftest buffer-local-window-configuration-change-hook ()
  (with-temp-buffer
    (add-hook 'window-configuration-change-hook 'emacs-lisp-mode nil t)
    (should (eq major-mode 'fundamental-mode))
    (should (eq nil (get-buffer-window)))
    (display-buffer (current-buffer))
    (should (window-live-p (get-buffer-window)))
    (should (eq major-mode 'emacs-lisp-mode))))

This fails (only at the final assertion) for both interactive
(M-x ert-run-tests-interactively) and batch mode testing
(emacs -batch -l ./my-test.el -f ert-run-tests-batch-and-exit).
e.g.:

Selector: buffer-local-window-configuration-change-hook
Passed:  0
Failed:  1 (1 unexpected)
Skipped: 0
Total:   1/1

Started at:   2019-10-24 19:30:08+1300
Finished.
Finished at:  2019-10-24 19:30:08+1300

F

F buffer-local-window-configuration-change-hook
    (ert-test-failed
     ((should
       (eq major-mode 'emacs-lisp-mode))
      :form
      (eq fundamental-mode emacs-lisp-mode)
      :value nil))


Similarly, evaling the following in *scratch*:

(with-temp-buffer
  (add-hook 'window-configuration-change-hook 'emacs-lisp-mode nil t)
  (display-buffer (current-buffer))
  (message "%S %S" major-mode (current-buffer)))

Produces:

26.3: "emacs-lisp-mode #<buffer  *temp*>"
27.0.50: "fundamental-mode #<buffer  *temp*>"


If I then add an explicit call to `redisplay':

(with-temp-buffer
  (add-hook 'window-configuration-change-hook 'emacs-lisp-mode nil t)
  (display-buffer (current-buffer))
  (redisplay)
  (message "%S %S" major-mode (current-buffer)))

I observe a flicker of the temporary buffer, and the output in
27.0.50 is what I want:

"emacs-lisp-mode #<buffer  *temp*>"


Adding an equivalent call to `redisplay' in the ERT test also allows
the test to succeed when called interactively:

Selector: buffer-local-window-configuration-change-hook
Passed:  1
Failed:  0
Skipped: 0
Total:   1/1


But in batch mode, the revised test still fails:

$ emacs -batch -l ./my-test.el -f ert-run-tests-batch-and-exit
Running 1 tests (2019-10-24 20:19:07+1300, selector ‘t’)
Test buffer-local-window-configuration-change-hook backtrace:
  signal(ert-test-failed (((should (eq major-mode 'emacs-lisp-mode)) :
  ert-fail(((should (eq major-mode 'emacs-lisp-mode)) :form (eq fundam
  (if (unwind-protect (setq value-17 (apply fn-15 args-16)) (setq form
  (let (form-description-19) (if (unwind-protect (setq value-17 (apply
  (let ((value-17 'ert-form-evaluation-aborted-18)) (let (form-descrip
  (let* ((fn-15 #'eq) (args-16 (condition-case err (let ((signal-hook-
  (progn (add-hook 'window-configuration-change-hook 'emacs-lisp-mode
  (unwind-protect (progn (add-hook 'window-configuration-change-hook '
  (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn
  (let ((temp-buffer (generate-new-buffer " *temp*"))) (save-current-b
  (closure (t) nil (let ((temp-buffer (generate-new-buffer " *temp*"))
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name buffer-local-window-configuration-cha
  ert-run-or-rerun-test(#s(ert--stats :selector t :tests [#s(ert-test
  ert-run-tests(t #f(compiled-function (event-type &rest event-args) #
  ert-run-tests-batch(nil)
  ert-run-tests-batch-and-exit()
  command-line-1(("-l" "./my-test.el" "-f" "ert-run-tests-batch-and-ex
  command-line()
  normal-top-level()
Test buffer-local-window-configuration-change-hook condition:
    (ert-test-failed
     ((should
       (eq major-mode 'emacs-lisp-mode))
      :form
      (eq fundamental-mode emacs-lisp-mode)
      :value nil))
   FAILED  1/1  buffer-local-window-configuration-change-hook (0.000329 sec)

Ran 1 tests, 0 results as expected, 1 unexpected (2019-10-24
20:19:07+1300, 0.174241 sec)

1 unexpected results:
   FAILED  buffer-local-window-configuration-change-hook



How does one write tests for this new behaviour?

(And very tangentially, should I be adding :tags '(:causes-redisplay)
to this test?  I encountered that while trying to figure this out, but
a basic grep suggests nothing other than ERT's own tests use it.)


-Phil

[1] Context is:
https://git.savannah.nongnu.org/cgit/so-long.git/commit/so-long.el?h=wip&id=9a4fe9f3e108fc9a041bc937efa8953e39863f7d

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Phil Sainty <psainty@orcon.net.nz>
> Date: Thu, 24 Oct 2019 23:50:47 +1300
> 
> How does one write tests for this new behaviour?

I don't think you can.  Redisplay does nothing in batch mode, by
design:

  static void
  redisplay_internal (void)
  {
    ...
    /* No redisplay if running in batch mode or frame is not yet fully
       initialized, or redisplay is explicitly turned off by setting
       Vinhibit_redisplay.  */
    if (FRAME_INITIAL_P (SELECTED_FRAME ())  <<<<<<<<<<<<<<<<<<<<<<<
	|| !NILP (Vinhibit_redisplay))
      return;

In batch mode FRAME_INITIAL_P returns non-zero.

This is not just an optimization: in batch mode important data
structures related to the display engine are not set up, because we
have no device to display on, so trying to enter redisplay will crash
very soon.

It would be a very important and useful feature to add a
pseudo-terminal which would allow invoking display in batch mode and
accessing the results of such a "display" in memory in order to verify
its correctness.  Such a feature would allow us to finally start
adding display tests, something that we sorely need.  So if you (or
anyone else) are interested, working on that would be very welcome,
and as a side effect will make the person who does that an expert on
our display code.  But until this job is done, display-related tests
can only be run interactively.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Phil Sainty]

Hi Eli,

On 25/10/19 3:23 AM, Eli Zaretskii wrote:
>> How does one write tests for this new behaviour?
> 
> I don't think you can.  Redisplay does nothing in batch mode,
> by design:

That certainly explains things; thanks.

I think the ERT manual would benefit from a mention of that
general constraint for batch mode; and perhaps (elisp) Batch Mode
and (elisp) Forcing Redisplay could comment on it as well?


For the specific changes to the window change functions, I think
it would be good to call out these testing side-effects in the
NEWS entry (I've spent a fair chunk of time looking at this, so
it might save someone else the same trouble).

AFAICS of the six window change hooks in Emacs 27, only two of
them were present in earlier versions:

* window-configuration-change-hook
* window-size-change-functions

The former has `run-window-configuration-change-hook' (which I've
now used in my test as a workaround).

AFAICS the latter cannot be invoked from lisp?

`run_window_size_change_functions' is a C function only.  Emacs 27
adds `run_window_change_functions' which is an umbrella for the
various new things, but that's also a C function only.

However the only *calls* to 'run_window_size_change_functions' in
Emacs 26.3 are in xdisp.c -- so I'm presuming this hook was already
untestable in batch mode?

So it looks to me as if `window-configuration-change-hook' is the
only one which would break existing tests.


How does this look?

--- etc/NEWS
+++ #<buffer NEWS>
@@ -2709,6 +2709,16 @@
 See the section "(elisp) Window Hooks" in the Elisp manual for a
 detailed explanation of the new behavior.

+As 'window-configuration-change-hook' is now invoked by redisplay,
+existing tests relying on it may fail.  Adding a call to 'redisplay'
+is sufficient for interactive test runs; but for batch mode you will
+need to run the hook explicitly, as 'redisplay' has no effect:
+
+  (unless (version< emacs-version "27")
+    (redisplay)
+    (when noninteractive
+      (run-window-configuration-change-hook)))
+
 +++
 ** Making scroll bar and fringe settings persistent for windows.
 The functions 'set-window-scroll-bars' and 'set-window-fringes' now



> This is not just an optimization: in batch mode important data
> structures related to the display engine are not set up, because we
> have no device to display on, so trying to enter redisplay will crash
> very soon.
>
> It would be a very important and useful feature to add a
> pseudo-terminal which would allow invoking display in batch mode and
> accessing the results of such a "display" in memory in order to verify
> its correctness.  Such a feature would allow us to finally start
> adding display tests, something that we sorely need.  So if you (or
> anyone else) are interested, working on that would be very welcome,
> and as a side effect will make the person who does that an expert on
> our display code.  But until this job is done, display-related tests
> can only be run interactively.

I don't believe I'm the person for this, sorry.  I do have a (perhaps
stupid) question about it, though.

When you say "pseudo-terminal" are we talking about text terminals,
or graphical 'terminals', or potentially both independently?  Or am
I misinterpreting entirely?

I am *imagining* that adding a text-based pseudo-terminal would be
a substantially simpler task than a graphical one, and would be
sufficient for testing redisplay as it pertains to text frames?

I'm also imagining that there is existing GPL'd tty/pty code which
might conceivably form a basis, and that the harder part would be
making it possible to interrogate?

So would step 1 be to provide a tty/pty for batch mode, and remove
the code inhibiting 'redisplay', and have Emacs "not crash"?


-Phil

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Phil Sainty <psainty@orcon.net.nz>
> Date: Fri, 25 Oct 2019 12:53:46 +1300
> 
> I think the ERT manual would benefit from a mention of that
> general constraint for batch mode; and perhaps (elisp) Batch Mode
> and (elisp) Forcing Redisplay could comment on it as well?

Patches welcome.

> For the specific changes to the window change functions, I think
> it would be good to call out these testing side-effects in the
> NEWS entry (I've spent a fair chunk of time looking at this, so
> it might save someone else the same trouble).

See below.

> AFAICS of the six window change hooks in Emacs 27, only two of
> them were present in earlier versions:
> 
> * window-configuration-change-hook
> * window-size-change-functions
> 
> The former has `run-window-configuration-change-hook' (which I've
> now used in my test as a workaround).
> 
> AFAICS the latter cannot be invoked from lisp?

Why not? they are just Lisp functions, so you can invoke them from
Lisp as usual, no?

I don't know why Martin needed run-window-configuration-change-hook,
it doesn't seem to be called anywhere in Emacs.  But in general, I see
no reason why we would expect a hook to be callable from Lisp via some
wrapper, as opposed to directoy, what would be the use case for that?

> `run_window_size_change_functions' is a C function only.  Emacs 27
> adds `run_window_change_functions' which is an umbrella for the
> various new things, but that's also a C function only.
> 
> However the only *calls* to 'run_window_size_change_functions' in
> Emacs 26.3 are in xdisp.c -- so I'm presuming this hook was already
> untestable in batch mode?

"Untestable" in what sense?  If you mean a test that would make sure
the hook is called under some specific situations related to display,
then we are back to the problem that the display code does nothing in
batch mode.  If you mean calling the hook functions directly, then you
certainly _can_ do that -- just walk the list and call each function
in the list directly.  Or what am I missing?

> How does this look?
> 
> --- etc/NEWS
> +++ #<buffer NEWS>
> @@ -2709,6 +2709,16 @@
>  See the section "(elisp) Window Hooks" in the Elisp manual for a
>  detailed explanation of the new behavior.
> 
> +As 'window-configuration-change-hook' is now invoked by redisplay,
> +existing tests relying on it may fail.  Adding a call to 'redisplay'
> +is sufficient for interactive test runs; but for batch mode you will
> +need to run the hook explicitly, as 'redisplay' has no effect:
> +
> +  (unless (version< emacs-version "27")
> +    (redisplay)
> +    (when noninteractive
> +      (run-window-configuration-change-hook)))

I think the example is too much for NEWS, and is also a bit
misleading, per the above discussion.  I'd also suggest to make the
text more general, because window-configuration-change-hook is not the
only hook/feature affected by this issue.

> > It would be a very important and useful feature to add a
> > pseudo-terminal which would allow invoking display in batch mode and
> > accessing the results of such a "display" in memory in order to verify
> > its correctness.  Such a feature would allow us to finally start
> > adding display tests, something that we sorely need.  So if you (or
> > anyone else) are interested, working on that would be very welcome,
> > and as a side effect will make the person who does that an expert on
> > our display code.  But until this job is done, display-related tests
> > can only be run interactively.
> 
> I don't believe I'm the person for this, sorry.  I do have a (perhaps
> stupid) question about it, though.
> 
> When you say "pseudo-terminal" are we talking about text terminals,
> or graphical 'terminals', or potentially both independently?  Or am
> I misinterpreting entirely?
> 
> I am *imagining* that adding a text-based pseudo-terminal would be
> a substantially simpler task than a graphical one, and would be
> sufficient for testing redisplay as it pertains to text frames?

We actually need both an emulation of a text-mode and of a GUI
display, because redisplay treats them differently, and because some
display features are not supported on text-mode terminals.

It is true that emulating a text-mode display is somewhat easier,
because we need to implement fewer APIs, but I'm not sure the
difference is too significant, at least not with some designs.

> I'm also imagining that there is existing GPL'd tty/pty code which
> might conceivably form a basis, and that the harder part would be
> making it possible to interrogate?

I'm not sure how the pty code is relevant.  If you are thinking of
hooking the pty device and somehow redirecting the stuff back to us,
then I would recommend against such a design because it is unportable,
and cannot be extended to support GUI displays.  (Texinfo does
something like that in its test suite for the stand-alone Info reader,
if someone wants to take a look.)

My idea is to intercept the displayed stuff inside Emacs, and analyze
the results by accessing some in-memory data instead.  But I admit
that I never thought about this long enough to see if this idea will
work well enough, so maybe trying it will hit some brick wall.

> So would step 1 be to provide a tty/pty for batch mode, and remove
> the code inhibiting 'redisplay', and have Emacs "not crash"?

We don't need to remove any code for that, we just need to implement a
new type of "window-system" whose frames will not pass the
FRAME_INITIAL_P test.  Running Emacs in batch mode with such a
window-system will need to provide the necessary parameters the
display engine needs, like the dimensions of the display, in some
initialization code.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[martin rudalics]

 > I don't know why Martin needed run-window-configuration-change-hook,
 > it doesn't seem to be called anywhere in Emacs.

It was previously used for running the buffer-local functions on that
hook from Elisp with the respective buffer current and its windows
selected.  It's no more called now.

martin

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Fri, 25 Oct 2019 11:20:06 +0200
> Cc: emacs-devel@gnu.org
> 
>  > I don't know why Martin needed run-window-configuration-change-hook,
>  > it doesn't seem to be called anywhere in Emacs.
> 
> It was previously used for running the buffer-local functions on that
> hook from Elisp with the respective buffer current and its windows
> selected.  It's no more called now.

So, a historic accident.  Maybe we should deprecate it.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[martin rudalics]

 >> It was previously used for running the buffer-local functions on that
 >> hook from Elisp with the respective buffer current and its windows
 >> selected.  It's no more called now.
 >
 > So, a historic accident.  Maybe we should deprecate it.

Its comment already says

   This function should not be needed any more and will be therefore
   considered obsolete.

but maybe we should defer that deprecation until Emacs 28 and then do
it only if Phil finds a better solution than he has now.

martin

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> Cc: psainty@orcon.net.nz, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Sat, 26 Oct 2019 09:49:22 +0200
> 
>  > So, a historic accident.  Maybe we should deprecate it.
> 
> Its comment already says
> 
>    This function should not be needed any more and will be therefore
>    considered obsolete.
> 
> but maybe we should defer that deprecation until Emacs 28 and then do
> it only if Phil finds a better solution than he has now.

I don't think Phil needs any solution here, he is just testing his
hook functions in a context that is very different from the actual
redisplay.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Phil Sainty]

On 25/10/19 9:37 PM, Eli Zaretskii wrote:
>> * window-configuration-change-hook
>> * window-size-change-functions
>>
>> AFAICS the latter cannot be invoked from lisp?
>
> Why not?

Hmmm.  It's no longer clear to me.  I'd assumed from the existence
of the C wrapper function that it *required* a wrapper function
(and, as no lisp wrapper existed, that it could therefore only be
used in C).  run_window_size_change_functions() does more than just
`run-hook-with-args' at any rate, so maybe it's inadvisable to run
it without the wrapper, but I don't really know.



> they are just Lisp functions, so you can invoke them from
> Lisp as usual, no?

I guess I'm equating "can" with "should" here.  If a hook is normally
called via a wrapper, and that wrapper does more than just the usual
`run-hooks' behaviours, then I'm assuming that the only correct way
to call the hook is using the wrapper -- which, in this instance, was
not exposed to lisp.



> I don't know why Martin needed run-window-configuration-change-hook,
> it doesn't seem to be called anywhere in Emacs.

Well the underlying run_window_configuration_change_hook (as opposed
to simply using `run-hooks') is needed because this doesn't behave
like a typical hook at all.

The global value is run once with the modified frame selected;
and then, for EACH window of that frame, if the window's buffer
has a buffer-local value for the hook then that local value is
run with that window selected.  So multiple buffer-local values
could be triggered by a single run of this hook (not necessarily
including the current buffer).

Exposing it to lisp as `run-window-configuration-change-hook' might
be a hold-over from versions prior to 27 -- in 26.3 it is called
many times from window.el.  In 27 the lisp function is no longer
used (except in my new test code, so I'm glad that it still remains).



> But in general, I see no reason why we would expect a hook to be
> callable from Lisp via some wrapper, as opposed to directly, what
> would be the use case for that?

As above, it behaves unusually for a hook, so the use-case is simply
any lisp code which needs to run that hook, because `run-hooks'
isn't going to do the trick for this one.



>> However the only *calls* to 'run_window_size_change_functions' in
>> Emacs 26.3 are in xdisp.c -- so I'm presuming this hook was already
>> untestable in batch mode?
>
> "Untestable" in what sense?  If you mean a test that would make sure
> the hook is called under some specific situations related to display,
> then we are back to the problem that the display code does nothing in
> batch mode.

Yes, that's all I meant -- that it's not possible to write a batch mode
test which relies upon `window-size-change-functions' being triggered
by Emacs in the usual way, when "the usual way" is via redisplay, and
redisplay isn't doing anything.



>> +As 'window-configuration-change-hook' is now invoked by redisplay,
>> +existing tests relying on it may fail.  Adding a call to 'redisplay'
>> +is sufficient for interactive test runs; but for batch mode you will
>> +need to run the hook explicitly, as 'redisplay' has no effect:
>> +
>> +  (unless (version< emacs-version "27")
>> +    (redisplay)
>> +    (when noninteractive
>> +      (run-window-configuration-change-hook)))
>
> I think the example is too much for NEWS, and is also a bit
> misleading, per the above discussion.

See above, I guess, if you're referring here to whether or not
`run-window-configuration-change-hook' should exist?



> I'd also suggest to make the text more general, because
> window-configuration-change-hook is not the only hook/feature
> affected by this issue.

I think it is?  My suggested addition to this specific NEWS item is
purely about flagging a potential backwards-incompatibility, and this
is the only hook out of the original two which is affected in this way
(because `window-size-change-functions' was already run only by
redisplay).

The other four hooks are new to 27, so no backwards-incompatibility
issues can exist for them.


-Phil

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Phil Sainty <psainty@orcon.net.nz>
> Cc: "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> Date: Fri, 25 Oct 2019 23:31:33 +1300
> 
> On 25/10/19 9:37 PM, Eli Zaretskii wrote:
> >> * window-configuration-change-hook
> >> * window-size-change-functions
> >>
> >> AFAICS the latter cannot be invoked from lisp?
> >
> > Why not?
> 
> Hmmm.  It's no longer clear to me.  I'd assumed from the existence
> of the C wrapper function that it *required* a wrapper function
> (and, as no lisp wrapper existed, that it could therefore only be
> used in C).  run_window_size_change_functions() does more than just
> `run-hook-with-args' at any rate, so maybe it's inadvisable to run
> it without the wrapper, but I don't really know.
> 
> > they are just Lisp functions, so you can invoke them from
> > Lisp as usual, no?
> 
> I guess I'm equating "can" with "should" here.  If a hook is normally
> called via a wrapper, and that wrapper does more than just the usual
> `run-hooks' behaviours, then I'm assuming that the only correct way
> to call the hook is using the wrapper -- which, in this instance, was
> not exposed to lisp.

It depends on what you want to do; "correct" is context dependent.  In
general, no Lisp code should ever call a hook that is meant to be
called from C, because only the C code "knows" when and how to call it
"correctly" so that the hook does its documented job.  But if you need
to call a hook function for testing purposes, all bets are off.

> > But in general, I see no reason why we would expect a hook to be
> > callable from Lisp via some wrapper, as opposed to directly, what
> > would be the use case for that?
> 
> As above, it behaves unusually for a hook, so the use-case is simply
> any lisp code which needs to run that hook, because `run-hooks'
> isn't going to do the trick for this one.

Once again, I don't expect any Lisp to call a hook "correct;y" when
that hook can only do its job when called from C.

> >> However the only *calls* to 'run_window_size_change_functions' in
> >> Emacs 26.3 are in xdisp.c -- so I'm presuming this hook was already
> >> untestable in batch mode?
> >
> > "Untestable" in what sense?  If you mean a test that would make sure
> > the hook is called under some specific situations related to display,
> > then we are back to the problem that the display code does nothing in
> > batch mode.
> 
> Yes, that's all I meant -- that it's not possible to write a batch mode
> test which relies upon `window-size-change-functions' being triggered
> by Emacs in the usual way, when "the usual way" is via redisplay, and
> redisplay isn't doing anything.

Yes, but it it also is impossible to write a batch-mode test which
runs _any_ of the display-related hooks, because the only "correct"
way to do that is to trigger redisplay, and you cannot do that in
batch mode.  So you can only write partial tests for these hooks, by
invoking them directly from Lisp.

> >> +  (unless (version< emacs-version "27")
> >> +    (redisplay)
> >> +    (when noninteractive
> >> +      (run-window-configuration-change-hook)))
> >
> > I think the example is too much for NEWS, and is also a bit
> > misleading, per the above discussion.
> 
> See above, I guess, if you're referring here to whether or not
> `run-window-configuration-change-hook' should exist?

No, I'm referring to the fact that this test will not in general check
that the hook is working, because it is unable to recreate its context
and its triggering.  It's a very partial test.

> > I'd also suggest to make the text more general, because
> > window-configuration-change-hook is not the only hook/feature
> > affected by this issue.
> 
> I think it is?

No, it isn't.  People who write tests for these hooks need to
understand that they can never invoke the hooks from Lisp in the right
context and at the right moment.

> My suggested addition to this specific NEWS item is purely about
> flagging a potential backwards-incompatibility, and this is the only
> hook out of the original two which is affected in this way (because
> `window-size-change-functions' was already run only by redisplay).

You are looking at this from the narrow POV of backwards
incompatibility, for someone who had a test that used
run-window-configuration-change-hook.  I suggest instead to write text
that will be useful for people who might have tests for other similar
hooks, even though they are new in Emacs 27.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Stefan Monnier]

> Yes, but it it also is impossible to write a batch-mode test which
> runs _any_ of the display-related hooks, because the only "correct"
> way to do that is to trigger redisplay, and you cannot do that in
> batch mode.  So you can only write partial tests for these hooks, by
> invoking them directly from Lisp.

IIRC there are ways to run Emacs in "batch" yet make it "display" at the
same time.  E.g. you can run it in a virtual X server, and I'm sure
there are ways to do the same in "virtual terminal emulators".


        Stefan

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Phil Sainty <psainty@orcon.net.nz>,  emacs-devel@gnu.org
> Date: Fri, 25 Oct 2019 08:53:11 -0400
> 
> > Yes, but it it also is impossible to write a batch-mode test which
> > runs _any_ of the display-related hooks, because the only "correct"
> > way to do that is to trigger redisplay, and you cannot do that in
> > batch mode.  So you can only write partial tests for these hooks, by
> > invoking them directly from Lisp.
> 
> IIRC there are ways to run Emacs in "batch" yet make it "display" at the
> same time.  E.g. you can run it in a virtual X server, and I'm sure
> there are ways to do the same in "virtual terminal emulators".

Thanks.  However, unless these emulators are components of a
garden-variety system out there, using them for Emacs testing would
mean that only a few of us could ever run these tests.  E.g., if I
cannot run redisplay tests on any of my systems, my motivation for
writing such tests will be exactly zero.  It is much better/easier to
run the tests interactively.

Also, using a display emulator for such testing would most probably
mean we cannot verify the correctness of the resulting display in some
automated way, as we do with ert facilities.  If so, the utility of
such emulators for testing Emacs display code will be rather low: it
will only be useful for making sure Emacs doesn't crash in some cases,
and allow testing of some display-related hooks (the issue that
triggered this thread), and maybe also features for which we can
produce indirect evidence using existing Lisp APIs, like make sure a
window or a stretch of text has the expected dimensions under some
conditions.

Bottom line: I think using these emulators is only a very partial
solution to being able to test our display code.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Stefan Monnier]

> Thanks.  However, unless these emulators are components of a
> garden-variety system out there, using them for Emacs testing would
> mean that only a few of us could ever run these tests.

Yes, it's not a real solution, especially since we don't even have
a "sample" to point to for other people to write&run such tests.

Maybe a better approach would be to allow Emacs to run the redisplay
in batch mode (i.e. build glyph matrices and just do nothing for the
actual redraw).

> Also, using a display emulator for such testing would most probably
> mean we cannot verify the correctness of the resulting display in some
> automated way, as we do with ert facilities.

Indeed, it doesn't solve all display-testing problems.  It would be good
enough for the window-configuration-change-hook case and a few others
(e.g. testing window-start and window-end positions, or pixel position
of particular chunks of text), but to do anything more we'd probably
have to provide ways to examine the glyph matrix.

> Bottom line: I think using these emulators is only a very partial
> solution to being able to test our display code.

I only meant it for people who want to start writing such tests, in the
absence of actual support for that.


        Stefan

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: psainty@orcon.net.nz,  emacs-devel@gnu.org
> Date: Fri, 25 Oct 2019 11:27:00 -0400
> 
> Maybe a better approach would be to allow Emacs to run the redisplay
> in batch mode (i.e. build glyph matrices and just do nothing for the
> actual redraw).

I don't think we can build the glyph matrices without some information
about the terminal.  The current display code assumes some terminal,
so we need to fake it to make this work.  That's the "pseudo-terminal"
idea I described up-thread.

> I only meant it for people who want to start writing such tests, in the
> absence of actual support for that.

Display tests can be run interactively, but the problem is that the
results need to be examined by eyeballing them.  So even for
interactive tests we currently can only test very little, like those
hooks we discussed.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks.  However, unless these emulators are components of a
> garden-variety system out there, using them for Emacs testing would
> mean that only a few of us could ever run these tests.  E.g., if I
> cannot run redisplay tests on any of my systems, my motivation for
> writing such tests will be exactly zero.  It is much better/easier to
> run the tests interactively.

I think it's conceivable to create a "standard" VM image that has a
known look -- i.e., a specific scaling factor, known set of fonts
installed, specific desktop size -- that we could then distribute and
run.  Both at the test rig and as developers.

We could then write ert tests that tests for basically anything
display-related ("does this line wrap after this character?  Does
scrolling down twice in this buffer display this image in full?") in a
pretty confident manner.

Perhaps using Xen as the hypervisor?

I don't know whether this has been considered before or not.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  psainty@orcon.net.nz,
>   emacs-devel@gnu.org
> Date: Sat, 26 Oct 2019 14:33:32 +0200
> 
> I think it's conceivable to create a "standard" VM image that has a
> known look -- i.e., a specific scaling factor, known set of fonts
> installed, specific desktop size -- that we could then distribute and
> run.  Both at the test rig and as developers.

I don't really understand what that means from Emacs's POV.  What will
go into that image, and how will it fix the issue at hand?

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I don't really understand what that means from Emacs's POV.  What will
> go into that image, and how will it fix the issue at hand?

The image will have a GNU/Linux distribution of some kind, and saying
"make xen-tests" (or whatever) will spin up that image, pull down the
code, run the display tests, exit the image and report the results.

While developing the tests, it probably makes sense to work in the image
itself, but that'd be up to the people writing the tests.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@iro.umontreal.ca,  psainty@orcon.net.nz,  emacs-devel@gnu.org
> Date: Sat, 26 Oct 2019 15:13:32 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I don't really understand what that means from Emacs's POV.  What will
> > go into that image, and how will it fix the issue at hand?
> 
> The image will have a GNU/Linux distribution of some kind, and saying
> "make xen-tests" (or whatever) will spin up that image, pull down the
> code, run the display tests, exit the image and report the results.

What would it use as the display terminal?

And how will the test suite be able to check the results against some
expected results?

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> The image will have a GNU/Linux distribution of some kind, and saying
>> "make xen-tests" (or whatever) will spin up that image, pull down the
>> code, run the display tests, exit the image and report the results.
>
> What would it use as the display terminal?

Xen can open a window on your display, but it can also run without
displaying anything, I think?  Emacs (inside Xen) just sees a standard X
display of a given size.

> And how will the test suite be able to check the results against some
> expected results?

We may have to add some new primitives to give more information about
the glyph matrix back to Lisp land, but I think we already have most of
what we need.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@iro.umontreal.ca,  psainty@orcon.net.nz,  emacs-devel@gnu.org
> Date: Sat, 26 Oct 2019 15:50:37 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> The image will have a GNU/Linux distribution of some kind, and saying
> >> "make xen-tests" (or whatever) will spin up that image, pull down the
> >> code, run the display tests, exit the image and report the results.
> >
> > What would it use as the display terminal?
> 
> Xen can open a window on your display, but it can also run without
> displaying anything, I think?  Emacs (inside Xen) just sees a standard X
> display of a given size.

So maybe this will do for one, maybe 2, of the 4 possible display
back-ends.

> > And how will the test suite be able to check the results against some
> > expected results?
> 
> We may have to add some new primitives to give more information about
> the glyph matrix back to Lisp land, but I think we already have most of
> what we need.

Information about the glyph matrix is not enough to test the end
results.  Emacs doesn't directly display the glyph matrices.

In any case, if information about the glyph matrices is what we want,
then Xen sounds like a very significant overkill.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> Xen can open a window on your display, but it can also run without
>> displaying anything, I think?  Emacs (inside Xen) just sees a standard X
>> display of a given size.
>
> So maybe this will do for one, maybe 2, of the 4 possible display
> back-ends.

You can have any OS inside Xen, but of course, we can't redistribute the
non-free ones.  It's still possible to create a framework that'd make it
easy to build a Xen image with Apple or Microsoft OS-es inside.

>> We may have to add some new primitives to give more information about
>> the glyph matrix back to Lisp land, but I think we already have most of
>> what we need.
>
> Information about the glyph matrix is not enough to test the end
> results.  Emacs doesn't directly display the glyph matrices.
>
> In any case, if information about the glyph matrices is what we want,
> then Xen sounds like a very significant overkill.

What Xen gives us is repeatability, and without that, automatic testing
becomes dicey.  There are so many variables (screen size, fonts
installed, etc) that doing testing on these things automatically without
a controlled environment is somewhat futile.

With a commonly distributed image, we'll all be doing the testing with
identical fonts (etc.), which will allow a new class of automatic tests
to be performed.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@iro.umontreal.ca,  psainty@orcon.net.nz,  emacs-devel@gnu.org
> Date: Sat, 26 Oct 2019 16:45:20 +0200
> 
> What Xen gives us is repeatability, and without that, automatic testing
> becomes dicey.  There are so many variables (screen size, fonts
> installed, etc) that doing testing on these things automatically without
> a controlled environment is somewhat futile.

Not IME.  For most display tests I can think of, small variations in
the dimensions don't matter, and where they do, we can always arrange
for exact matches.  Look at the large fraction of display problems
that we usually succeed in reproducing on a completely different
system.

I guess we have very different experiences regarding this, or maybe we
think about very different aspects to test.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Phil Sainty]

On 26/10/19 1:42 AM, Eli Zaretskii wrote:
>> My suggested addition to this specific NEWS item is purely about
>> flagging a potential backwards-incompatibility, and this is the only
>> hook out of the original two which is affected in this way (because
>> `window-size-change-functions' was already run only by redisplay).
> 
> You are looking at this from the narrow POV of backwards
> incompatibility, for someone who had a test that used
> run-window-configuration-change-hook.  I suggest instead to write text
> that will be useful for people who might have tests for other similar
> hooks, even though they are new in Emacs 27.

I agree with you generally, but I don't think that those other details
are needed in the NEWS item?  Your reasoning here was similar to my own
reasoning for suggesting that additions be made to the various relevant
nodes in the ert and elisp manuals:

>>>> I think the ERT manual would benefit from a mention of that
>>>> general constraint for batch mode; and perhaps (elisp) Batch Mode
>>>> and (elisp) Forcing Redisplay could comment on it as well?

We could add (elisp) Window Hooks to that list, in order to more
directly cover the window change functions.


My own feeling is still that the more general information belongs in
those manuals, and only the backwards-incompatibility notice is needed
in the NEWS.

I don't *object* to the other info being added to the NEWS, though --
this is a very minor point which doesn't warrant lots of discussion;
so if you feel it belongs in NEWS then let's do that.


-Phil

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Phil Sainty <psainty@orcon.net.nz>
> Date: Sat, 26 Oct 2019 02:16:22 +1300
> 
> I don't *object* to the other info being added to the NEWS, though --
> this is a very minor point which doesn't warrant lots of discussion;
> so if you feel it belongs in NEWS then let's do that.

I do.  TIA.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[martin rudalics]

 > +As 'window-configuration-change-hook' is now invoked by redisplay,
 > +existing tests relying on it may fail.  Adding a call to 'redisplay'
 > +is sufficient for interactive test runs; but for batch mode you will
 > +need to run the hook explicitly, as 'redisplay' has no effect:
 > +
 > +  (unless (version< emacs-version "27")
 > +    (redisplay)
 > +    (when noninteractive
 > +      (run-window-configuration-change-hook)))
 > +

Could you please provide a more realistic example for the function you
want to put on that hook than the

     (add-hook 'window-configuration-change-hook 'emacs-lisp-mode nil t)

from your initial example?  Then we might be able to come up with
something more practical than invoking a hook explicitly.

Thanks, martin

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Phil Sainty]

Hi Martin,

On 25/10/19 9:51 PM, martin rudalics wrote:
> Could you please provide a more realistic example for the function you
> want to put on that hook than the
> 
>     (add-hook 'window-configuration-change-hook 'emacs-lisp-mode nil t)
> 
> from your initial example?  Then we might be able to come up with
> something more practical than invoking a hook explicitly.

The example is actually pretty close to the real use-case.  If so-long
sees a file with very long lines being visited, it (now) only triggers
its mitigations if and when that buffer is first displayed.  So if the
buffer is never displayed then nothing is done.

Here's the key part of the (non-test) library code:

https://git.savannah.nongnu.org/cgit/so-long.git/tree/so-long.el?h=1572000262&id=80ed099#n1591

(defun so-long-deferred ()
  "Arrange to call `so-long' if the current buffer is displayed
in a window."
  ;; The first time that a window-configuration change results in the
  ;; buffer being displayed in a window, `so-long' will be called
  ;; (with the window selected and the buffer set as current).
  ;; Because `so-long' removes this buffer-local hook value, it
  ;; triggers once at most.
  (add-hook 'window-configuration-change-hook #'so-long nil :local))


Where `so-long' itself (by default) is invoking a major mode.

(It *does* do more besides, but as I say -- not miles away from the
example I'd used.)


And the relevant part of the related test is:

  ;; Invisible buffer.
  (with-temp-buffer
    (insert "#!emacs\n")
    (normal-mode)
    (so-long-tests-remember)
    (insert (make-string (1+ so-long-threshold) ?x))
    (normal-mode)
    (should (eq major-mode 'emacs-lisp-mode))
    (should (eq nil (get-buffer-window)))
    ;; Displaying the buffer should invoke `so-long'.
    (display-buffer (current-buffer))
    (should (window-live-p (get-buffer-window)))
    (unless (version< emacs-version "27")
      ;; From Emacs 27 the `display-buffer' call is insufficient.
      ;; The various 'window change functions' are now invoked by the
      ;; redisplay, and redisplay does nothing at all in batch mode,
      ;; so we cannot test under this revised behaviour.  Refer to:
      ;;
https://lists.gnu.org/archive/html/emacs-devel/2019-10/msg00971.html
      ;; For interactive (non-batch) test runs, calling `redisplay'
      ;; does do the trick; so do that first.
      (redisplay)
      ;; In batch mode we need to cheat, and just pretend that
      ;; `redisplay' triggered `window-configuration-change-hook'.
      ;; Test `so-long--active' though, in case a future version of
      ;; Emacs adds the framework necessary to make this work.
      (when noninteractive
        (unless (eq so-long--active t)
          (run-window-configuration-change-hook))))
    (so-long-tests-assert-and-revert 'so-long-mode))


Without `run-window-configuration-change-hook' I'd have to replicate
some of that functionality myself, and the test would be getting even
further away from the real processes, and therefore becoming less and
less useful as a test.  I'd like to have to fake as little as possible.


-Phil

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[martin rudalics]

 > The example is actually pretty close to the real use-case.  If so-long
 > sees a file with very long lines being visited, it (now) only triggers
 > its mitigations if and when that buffer is first displayed.  So if the
 > buffer is never displayed then nothing is done.

Eli, do you think that Phil's use case would warrant a special hook
say 'buffer-first-display-functions' triggered when a buffer's
display-count changes from 0?  It would be stunningly simple to
implement that.  OTOH, scanning all window changes to find out whether
a buffer has been displayed for the first time just produces lots of
wasted cyles.

 > Without `run-window-configuration-change-hook' I'd have to replicate
 > some of that functionality myself, and the test would be getting even
 > further away from the real processes, and therefore becoming less and
 > less useful as a test.  I'd like to have to fake as little as possible.

Understandable.  Rewriting this in Elisp is no fun.

martin

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Sat, 26 Oct 2019 09:49:02 +0200
> 
>  > The example is actually pretty close to the real use-case.  If so-long
>  > sees a file with very long lines being visited, it (now) only triggers
>  > its mitigations if and when that buffer is first displayed.  So if the
>  > buffer is never displayed then nothing is done.
> 
> Eli, do you think that Phil's use case would warrant a special hook
> say 'buffer-first-display-functions' triggered when a buffer's
> display-count changes from 0?  It would be stunningly simple to
> implement that.

Could be.  But why does it have to be related to the first time a
buffer is displayed?  Don't we want to do that when the file is
visited instead?

>  > Without `run-window-configuration-change-hook' I'd have to replicate
>  > some of that functionality myself, and the test would be getting even
>  > further away from the real processes, and therefore becoming less and
>  > less useful as a test.  I'd like to have to fake as little as possible.
> 
> Understandable.  Rewriting this in Elisp is no fun.

Per previous messages, rewriting in Lisp is either (a) not necessary,
or (b) will not help even if done.  So I don't think this discussion
concluded that run-window-configuration-change-hook is of any
importance that would justify leaving it in our sources in the long
run.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Phil Sainty]

On 26/10/19 2:44 AM, Eli Zaretskii wrote:
>> if you feel it belongs in NEWS then let's do that.
>
> I do.  TIA.

All good.  I'll write that along with some updates for the manuals
sometime soon.


On 26/10/19 10:07 PM, Eli Zaretskii wrote:
> Could be.  But why does it have to be related to the first time a
> buffer is displayed?  Don't we want to do that when the file is
> visited instead?

Your comment was to Martin, but were you wondering why so-long would
wait for the buffer to be displayed?

It did originally act as soon as the file was visited (and the code
currently in master still does) regardless of buffer visibility; but
I ended up with some bug reports related to libraries which were
visiting files in order to do some processing behind-the-scenes,
but which were getting tripped up by so-long doing things to those
buffers unexpectedly.

Refer to:
https://lists.gnu.org/archive/html/bug-gnu-emacs/2019-10/msg01726.html

I don't think long lines pose any performance problems in buffers
which are never displayed, so the revised approach means those buffers
are simply left alone (they just get a buffer-local hook value which
will most likely never be examined or acted on), and it's only buffers
which actually get displayed that are subject to so-long's actions.

I've not been able to spot any down-sides to the change, so I'm pretty
sure it's the right thing to do.


>>  > Without `run-window-configuration-change-hook' I'd have to replicate
>>  > some of that functionality myself, and the test would be getting even
>>  > further away from the real processes, and therefore becoming less and
>>  > less useful as a test.  I'd like to have to fake as little as possible.
>>
>> Understandable.  Rewriting this in Elisp is no fun.
> 
> Per previous messages, rewriting in Lisp is either (a) not necessary,
> or (b) will not help even if done.  So I don't think this discussion
> concluded that run-window-configuration-change-hook is of any
> importance that would justify leaving it in our sources in the long
> run.

I'll argue for not removing the function while it has a use.  The actual
implementation in C is going to remain, so removing the lisp binding
seems like an extremely minimal saving, whereas keeping it provides a
concrete benefit in at least this case.  Running the hook explicitly in
my test may not be the same thing as redisplay running it; but then the
thing I'm really testing is that when the hook runs, so-long does its
thing, so ultimately I don't think it's too important how we get to that
point, so long as I get to compare 'before hooks runs' and 'after hook
runs' -- which, at present, I can do.


-Phil

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> Cc: martin rudalics <rudalics@gmx.at>, emacs-devel@gnu.org
> From: Phil Sainty <psainty@orcon.net.nz>
> Date: Sat, 26 Oct 2019 23:57:49 +1300
> 
> On 26/10/19 2:44 AM, Eli Zaretskii wrote:
> >> if you feel it belongs in NEWS then let's do that.
> >
> > I do.  TIA.
> 
> All good.  I'll write that along with some updates for the manuals
> sometime soon.

Thanks.

> It did originally act as soon as the file was visited (and the code
> currently in master still does) regardless of buffer visibility; but
> I ended up with some bug reports related to libraries which were
> visiting files in order to do some processing behind-the-scenes,
> but which were getting tripped up by so-long doing things to those
> buffers unexpectedly.
> 
> Refer to:
> https://lists.gnu.org/archive/html/bug-gnu-emacs/2019-10/msg01726.html

Looks like the problem was specifically with setting the buffer
read-only?  Why does so-long do that?

> I don't think long lines pose any performance problems in buffers
> which are never displayed

You are wrong: it will cause problems in any function that uses the
display code, even if nothing is displayed.  For example,
vertical-motion, next-line, posn-at-point, etc. all use the display
code internally.

> I've not been able to spot any down-sides to the change, so I'm pretty
> sure it's the right thing to do.

Well, one downside is that you now need to use a hook that was not
really meant for that, or we'd need to introduce yet another hook.
Which is perfectly OK, as long as we are sure there's no simpler
solution, one that uses existing facilities.

> > Per previous messages, rewriting in Lisp is either (a) not necessary,
> > or (b) will not help even if done.  So I don't think this discussion
> > concluded that run-window-configuration-change-hook is of any
> > importance that would justify leaving it in our sources in the long
> > run.
> 
> I'll argue for not removing the function while it has a use.

I'm not going to push to remove this, but I think your basic premise
here is wrong.  These hooks cannot be possibly tested in full from
batch mode anyway, so your ability to test them is limited, with or
without that function.

> Running the hook explicitly in my test may not be the same thing as
> redisplay running it; but then the thing I'm really testing is that
> when the hook runs, so-long does its thing

And you cannot test that by calling the hook function directly?  Why
not?

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Stefan Monnier]

>> I don't think long lines pose any performance problems in buffers
>> which are never displayed
>
> You are wrong: it will cause problems in any function that uses the
> display code, even if nothing is displayed.  For example,
> vertical-motion, next-line, posn-at-point, etc. all use the display
> code internally.

In theory you're right.  But I think in practice "buffers that aren't
displayed" is a pretty good approximation of "so-long won't help".

>> I've not been able to spot any down-sides to the change, so I'm pretty
>> sure it's the right thing to do.
> Well, one downside is that you now need to use a hook that was not
> really meant for that, or we'd need to introduce yet another hook.

I missed something: which new hook would be needed?


        Stefan

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Phil Sainty <psainty@orcon.net.nz>,  rudalics@gmx.at,  emacs-devel@gnu.org
> Date: Sat, 26 Oct 2019 08:11:21 -0400
> 
> >> I don't think long lines pose any performance problems in buffers
> >> which are never displayed
> >
> > You are wrong: it will cause problems in any function that uses the
> > display code, even if nothing is displayed.  For example,
> > vertical-motion, next-line, posn-at-point, etc. all use the display
> > code internally.
> 
> In theory you're right.  But I think in practice "buffers that aren't
> displayed" is a pretty good approximation of "so-long won't help".

I don't understand how practice makes theory unimportant in this case.
There's a leap in your logic that I couldn't follow.

> >> I've not been able to spot any down-sides to the change, so I'm pretty
> >> sure it's the right thing to do.
> > Well, one downside is that you now need to use a hook that was not
> > really meant for that, or we'd need to introduce yet another hook.
> 
> I missed something: which new hook would be needed?

This sub-thread was in response to Martin's question here:

  https://lists.gnu.org/archive/html/emacs-devel/2019-10/msg01013.html

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Stefan Monnier]

>> >> I don't think long lines pose any performance problems in buffers
>> >> which are never displayed
>> > You are wrong: it will cause problems in any function that uses the
>> > display code, even if nothing is displayed.  For example,
>> > vertical-motion, next-line, posn-at-point, etc. all use the display
>> > code internally.
>> In theory you're right.  But I think in practice "buffers that aren't
>> displayed" is a pretty good approximation of "so-long won't help".
> I don't understand how practice makes theory unimportant in this case.
> There's a leap in your logic that I couldn't follow.

The `so-long` package is a trade-off: it has advantages (supposedly
avoids some nasty slowdowns) and disadvantages (it works by disabling
some features).

It's virtually impossible to determine reliably when it's better to use
so-long and when it's better not to.  So theory is not super relevant
compared to actual practical experience.

I don't have an opinion on whether so-long should (by default) only be
enabled in buffers that are displayed, but it intuitively sound like
a potentially good heuristic to minimize the risk of breaking stuff.

>> I missed something: which new hook would be needed?
> This sub-thread was in response to Martin's question here:
>   https://lists.gnu.org/archive/html/emacs-devel/2019-10/msg01013.html

Ah, I see.  I thought (and still think) it was just a misunderstanding
on Martin's part, not an actual need (at least Phil didn't express
a need for it).

[ Or maybe this particular case just reminded Martin of other cases
  where such a hook could have been handy.  ]


        Stefan

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: psainty@orcon.net.nz,  rudalics@gmx.at,  emacs-devel@gnu.org
> Date: Sat, 26 Oct 2019 11:59:16 -0400
> 
> It's virtually impossible to determine reliably when it's better to use
> so-long and when it's better not to.  So theory is not super relevant
> compared to actual practical experience.

Agreed.  My point is that whatever the decision, I don't think I
understand (or agree) with it being different for displayed and
non-displayed buffers, for reasons I explained.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Stefan Monnier]

>> It's virtually impossible to determine reliably when it's better to use
>> so-long and when it's better not to.  So theory is not super relevant
>> compared to actual practical experience.
> Agreed.  My point is that whatever the decision, I don't think I
> understand (or agree) with it being different for displayed and
> non-displayed buffers, for reasons I explained.

Most of the slowdown is linked to code related to the display, in
my experience.

And if you look at the three functions you mention:
- next-line is very rarely called as a function (C-h f suggests to
  use forward-line instead in Lisp code), so it's virtually never
  used in non-displayed buffers.
- posn-at-point doesn't return useful information AFAICT in
  non-displayed buffers.
- for vertical-motion it's not as clear, but a quick grep suggests that
  it's also rather unusual to call it in a non-displayed buffer.


        Stefan

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 26 Oct 2019 13:14:22 -0400
> Cc: psainty@orcon.net.nz, rudalics@gmx.at, emacs-devel@gnu.org
> 
> - next-line is very rarely called as a function (C-h f suggests to
>   use forward-line instead in Lisp code), so it's virtually never
>   used in non-displayed buffers.
> - posn-at-point doesn't return useful information AFAICT in
>   non-displayed buffers.
> - for vertical-motion it's not as clear, but a quick grep suggests that
>   it's also rather unusual to call it in a non-displayed buffer.

Since when are 3 random examples brought up by 5 sec of digging into a
faulty memory enough to consider a good theory eating dust?

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Stefan Monnier]

> Since when are 3 random examples brought up by 5 sec of digging into a
> faulty memory enough to consider a good theory eating dust?

Obviously you've made up your mind.


        Stefan

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: psainty@orcon.net.nz,  rudalics@gmx.at,  emacs-devel@gnu.org
> Date: Sat, 26 Oct 2019 15:53:22 -0400
> 
> > Since when are 3 random examples brought up by 5 sec of digging into a
> > faulty memory enough to consider a good theory eating dust?
> 
> Obviously you've made up your mind.

Actually, no.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Phil Sainty]

On 27/10/19 12:28 AM, Eli Zaretskii wrote:
> Looks like the problem was specifically with setting the buffer
> read-only?  Why does so-long do that?

The reasoning for this default was that *editing* a long line might
be slow, and in the sorts of files which I was expecting so-long to
trigger for, the user probably doesn't want to be manually editing
them; so this is to protect against the user accidentally modifying
the line (and probably then needing to undo it), all while Emacs is
responding more slowly than usual.

The particular issue raised was certainly with the read-only state,
but it did make me think that other issues might well be possible,
and that NOT messing with invisible buffers would be a much safer
(and much less surprising) default behaviour from the point of view
of other libraries doing automated things to files behind the scenes.

(It's configurable, so the original approach can be restored if users
choose to.)


>> I don't think long lines pose any performance problems in buffers
>> which are never displayed
> 
> You are wrong: it will cause problems in any function that uses the
> display code, even if nothing is displayed.  For example,
> vertical-motion, next-line, posn-at-point, etc. all use the display
> code internally.

I'll have to have a play.

My initial feeling is that the *potential* to break such background
processing (which is liable to be hidden from the user) by triggering
so-long is a more significant risk than the potential for background
processing to run slowly because of long lines; so I'm still leaning
towards using the "visible buffers only" approach as the preferable
compromise here.


>> I've not been able to spot any down-sides to the change, so I'm pretty
>> sure it's the right thing to do.
> 
> Well, one downside is that you now need to use a hook that was not
> really meant for that, or we'd need to introduce yet another hook.
> Which is perfectly OK, as long as we are sure there's no simpler
> solution, one that uses existing facilities.

I think it's quite a nice use of the hook, to be honest (even if it's
not a use-case that was originally envisaged).  I was very pleased
when I realised how very simple and efficient this approach was.  The
behaviour I wanted arises almost effortlessly from setting a single
buffer-local value.  I definitely didn't see that as a downside.


>> Running the hook explicitly in my test may not be the same thing as
>> redisplay running it; but then the thing I'm really testing is that
>> when the hook runs, so-long does its thing
> 
> And you cannot test that by calling the hook function directly?  Why
> not?

Do you mean, in the case where I have done this...

(add-hook 'window-configuration-change-hook #'so-long nil :local)

...that my test could just call `so-long' directly, rather than
running the hook?

If I did that, the test would really be redundant.  It certainly
wouldn't be testing any part of the new behaviour.  Other existing
tests already assert what happens when `so-long' is called directly.
The entire purpose of this test was to confirm that `so-long' gets
called *in*directly.


-Phil

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Eli Zaretskii]

> Cc: rudalics@gmx.at, emacs-devel@gnu.org
> From: Phil Sainty <psainty@orcon.net.nz>
> Date: Sun, 27 Oct 2019 02:09:30 +1300
> 
> On 27/10/19 12:28 AM, Eli Zaretskii wrote:
> > Looks like the problem was specifically with setting the buffer
> > read-only?  Why does so-long do that?
> 
> The reasoning for this default was that *editing* a long line might
> be slow, and in the sorts of files which I was expecting so-long to
> trigger for, the user probably doesn't want to be manually editing
> them; so this is to protect against the user accidentally modifying
> the line (and probably then needing to undo it), all while Emacs is
> responding more slowly than usual.

It sounds strange to make the buffer read-only for this, because doing
that by itself doesn't make Emacs faster.  Maybe you should explain
the rationale somewhere in the docs.

> The particular issue raised was certainly with the read-only state,
> but it did make me think that other issues might well be possible,
> and that NOT messing with invisible buffers would be a much safer
> (and much less surprising) default behaviour from the point of view
> of other libraries doing automated things to files behind the scenes.

I don't insist, but I do think that having different behavior in
displayed and non-displayed buffers would surprise me.  YMMV, of
course.

> My initial feeling is that the *potential* to break such background
> processing (which is liable to be hidden from the user) by triggering
> so-long is a more significant risk than the potential for background
> processing to run slowly because of long lines; so I'm still leaning
> towards using the "visible buffers only" approach as the preferable
> compromise here.

I actually don't see why you'd need to compromise here.  But it's your
call.

> > Well, one downside is that you now need to use a hook that was not
> > really meant for that, or we'd need to introduce yet another hook.
> > Which is perfectly OK, as long as we are sure there's no simpler
> > solution, one that uses existing facilities.
> 
> I think it's quite a nice use of the hook, to be honest (even if it's
> not a use-case that was originally envisaged).  I was very pleased
> when I realised how very simple and efficient this approach was.  The
> behaviour I wanted arises almost effortlessly from setting a single
> buffer-local value.  I definitely didn't see that as a downside.

If you don't need a new hook (and Stefan says you shouldn't need one),
then there's no issue here.  I just wrote that under the assumption
that you'd ask for a new hook, as Martin seemed to indicate.

> >> Running the hook explicitly in my test may not be the same thing as
> >> redisplay running it; but then the thing I'm really testing is that
> >> when the hook runs, so-long does its thing
> > 
> > And you cannot test that by calling the hook function directly?  Why
> > not?
> 
> Do you mean, in the case where I have done this...
> 
> (add-hook 'window-configuration-change-hook #'so-long nil :local)
> 
> ...that my test could just call `so-long' directly, rather than
> running the hook?
> 
> If I did that, the test would really be redundant.

How is it not redundant now?  The hook is not called from the display
engine, so you cannot test that it does its job when it's needed.

> The entire purpose of this test was to confirm that `so-long' gets
> called *in*directly.

So you want to make sure that putting a function on
window-configuration-change-hook will cause that function to be called
when run-window-configuration-change-hook is called?  If that's the
purpose, then I guess it's okay, but then what does it have to do with
the real-life calls?  run-window-configuration-change-hook isn't
called then.

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Stefan Monnier]

>> The example is actually pretty close to the real use-case.  If so-long
>> sees a file with very long lines being visited, it (now) only triggers
>> its mitigations if and when that buffer is first displayed.  So if the
>> buffer is never displayed then nothing is done.
> Eli, do you think that Phil's use case would warrant a special hook
> say 'buffer-first-display-functions' triggered when a buffer's
> display-count changes from 0?  It would be stunningly simple to
> implement that.  OTOH, scanning all window changes to find out whether
> a buffer has been displayed for the first time just produces lots of
> wasted cyles.

I don't see much waste: put a function on the
window-configuration-change-hook and the first time it's called you know
the buffer is displayed so you remove the function from the hook.

There's no "scanning all window changes" involved.


        Stefan

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[martin rudalics]

 > I don't see much waste: put a function on the
 > window-configuration-change-hook and the first time it's called you know
 > the buffer is displayed so you remove the function from the hook.
 >
 > There's no "scanning all window changes" involved.

The premises were that (1) 'window-configuration-change-hook' is not
called in batch mode and must be therefore emulated (using some sort
of 'get-buffer-window-list' at least) and (2) a buffer might never get
displayed, so the function will remain on the hook (or what is run
there instead) forever.

And obviously Emacs 26

(progn
   (set-window-buffer nil foo)
   (set-window-buffer nil bar))

will trigger the hook for foo while it won't do so in Emacs 27.

martin

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[Stefan Monnier]

>> I don't see much waste: put a function on the
>> window-configuration-change-hook and the first time it's called you know
>> the buffer is displayed so you remove the function from the hook.
>> There's no "scanning all window changes" involved.
> The premises were that (1) 'window-configuration-change-hook' is not
> called in batch mode and must be therefore emulated (using some sort
> of 'get-buffer-window-list' at least)

I don't see any need to "emulate" it, usually.

Phil was only annoyed that it doesn't trigger in his tests which makes
it hard for him to test that the hook does its job.

Emulating the hook would largely defeat the purpose of the test: you end
up testing your emulation instead.

> and (2) a buffer might never get displayed, so the function will
> remain on the hook

Right.  Just like it would remain on your new hook.
I don't see this as a problem.


        Stefan

Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?

[martin rudalics]

 > Emulating the hook would largely defeat the purpose of the test: you end
 > up testing your emulation instead.

IIUC Phil is aware of that and only wanted to test whether the logic
of ‘window-configuration-change-hook’ permits to call his function in
all possible settings and whether doing so DTRT.  For that purpose, he
may have to run that hook much more often than needed in practice.

 >> and (2) a buffer might never get displayed, so the function will
 >> remain on the hook
 >
 > Right.  Just like it would remain on your new hook.
 > I don't see this as a problem.

It goes without saying that parts of the bookkeeping overhead of
'window-configuration-change-hook' remain unaffected by whether the
hook has been set or not (due to the buffer-local settings).  But the
more often you run it manually, the more expensive it gets.

Anyway, since apart from Phil's hypothetical use case nobody else will
probably profit from the hook I proposed, let's forget about it.

martin

so-long.el updates (was Re: 27.0.50: How can I test a buffer-local window-configuration-change-hook in batch mode?)

[Phil Sainty]

Thanks for all the input regarding the window change functions.

I've settled on keeping things more or less as I had them when I asked
the question, but I've updated the documentation to clarify that it's
possible for long lines in invisible buffers to cause performance
issues.

I've stuck with a default behaviour of "do nothing until the buffer is
displayed" as I believe that is the safest approach for end users in
general.

https://lists.gnu.org/archive/html/emacs-diffs/2019-11/msg00126.html

If there are no objections, I'll rebase & merge this and the rest of
the scratch/so-long-updates branch sometime soon.
https://lists.gnu.org/archive/html/emacs-diffs/2019-11/msg00119.html

The other notable changes in the branch are to make the unload-feature
support more complete, plus some backwards-compatibility improvements
so that users with old versions installed will not encounter problems
with the new version (which gets this closer to an ELPA release, too).

I've also added all of flymake-mode, flyspell-mode, and flycheck-mode
to so-long-minor-modes, so they will be disabled by default in any
buffer where so-long triggers.  I'm pretty sure that modes which are
liable to annotate the buffer in some way are sensible to switch off
by default.

Once these changes are merged, I'll ask people to do some testing in
their own environments, and I expect there would be adjustments to the
default config resulting from that; and then we could likely make an
ELPA release.


-Phil