bug#19381: The doc string for `sit-for' is wrong; redisplay isn't unconditionally done.

bug#19381: The doc string for `sit-for' is wrong; redisplay isn't unconditionally done.

[Alan Mackenzie]

Hello, Emacs.

The doc string for `sit-for', in its first (and most important) line
states unequivocally that redisplay is performed.  (That line is "Perform
redisplay, then wait for SECONDS seconds or until input is available.")

That's not what the function currently does - it doesn't perform
redisplay if input is already available when it is called.
A sentence lower down in the doc string contradicts its first line
("Redisplay does not happen if input is available before it starts.").

There is thus a clash between the doc-string and the code.  (The info
page agrees with the code).

It would seem at first sight that the obvious thing to do is correct the
first line of the doc string.  However there are lots (125) of instances
of "(sit-for 0)" in Emacs, and a lot of these are probably intended to
mean "perform redisplay NOW".  So it might be better to make `sit-for'
always do a redisplay.

However it's done, though, the clash between the doc string and
everything else should be resolved.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#19381: The doc string for `sit-for' is wrong; redisplay isn't unconditionally done.

[Stefan Monnier]

> of "(sit-for 0)" in Emacs, and a lot of these are probably intended to
                                                    ^^^^^^^^
I'd need to see evidence of it.


        Stefan

bug#19381: The doc string for `sit-for' is wrong; redisplay isn't unconditionally done.

[Alan Mackenzie]

Hello, Stefan.

On Sun, Dec 14, 2014 at 06:31:29PM -0500, Stefan Monnier wrote:
> > of "(sit-for 0)" in Emacs, and a lot of these are probably intended to
> > mean "perform redisplay NOW".  So it might be better to make
> > `sit-for' always do a redisplay.

> I'd need to see evidence of it.

I've extracted all instances of "(sit-for 0)" to which a comment is
appended (and stripped out the "(sit-for 0)" and numbered the lines)
with the command:

    find . -name '*.el' | xargs grep -n '^[^;]*(sit-for 0).*;' | \
    sed 's/\([^:]*:[^:]*:\)[^;]*\(.*\)/\1 \2/' | cat -b

The result is this:

     1	./ehelp.el:223: ;necessary if last command was end-of-buffer or
     2	./emacs-lisp/edebug.el:2504: ; Force update and continue.
     3	./play/gomoku.el:993: ; Display NOW
     4	./play/gomoku.el:1036: ; Display NOW
     5	./play/gomoku.el:1127: ; Display NOW
     6	./play/landmark.el:884: ; Display NOW
     7	./play/landmark.el:926: ; Display NOW
     8	./play/landmark.el:1014: ; Display NOW
     9	./obsolete/terminal.el:627: ;get display to update
    10	./isearch.el:2991: ;make sure (window-start) is credible
    11	./ffap.el:366: ; display point movement
    12	./ffap.el:1502: ; display
    13	./ffap.el:1536: ; redraw original screen
    14	./ffap.el:1618: ; display
    15	./progmodes/compile.el:1754: ; Force redisplay
    16	./textmodes/page-ext.el:639: ; otherwise forward-line fails if N > window height.
    17	./textmodes/ispell.el:2749: ; update display
    18	./textmodes/ispell.el:2753: ; update display showing inverse video.
    19	./net/rcirc.el:1657: ; displayed text before hook
    20	./shell.el:988: ; force redisplay
    21	./shell.el:1067: ; force redisplay
    22	./emulation/cua-rect.el:769: ; make window top/bottom reliable
    23	./emulation/viper-macs.el:201: ; this overcomes xemacs tty bug
    24	./vc/ediff-mult.el:2103: ; sometimes needed to synch the display and ensure that the
    25	./vc/ediff.el:945: ; sync before using window-start/end -- a precaution
    26	./vc/smerge-mode.el:403: ;Display the new highlighting.
    27	./vc/ediff-ptch.el:717: ; synchronize - let the user see diagnostics

That's ~20% of the instances.  Let's assume, for want of a better
hypothesis, that these 27 uses are representative of the 125 in total.
Of these, 1, 2, 10, 15, 16, 20, 21, 22, 25 state that redisplay is
forced, or give a reason why redisplay is needed.  That's 9 out of 27, a
third.  It is impossible to say from this whether the rest of the 27
absolutely need immediate redisplay or not.  Some of them might well do.

Extrapolating that to the 125 instances of "(sit-for 0)" suggests
around 42 of them (at least) want immediate redisplay.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#19381: The doc string for `sit-for' is wrong; redisplay isn't unconditionally done.

[Stefan Monnier]

> That's ~20% of the instances.

But that doesn't say if those instances would want to redisplay when
there's pending input.  The 0 argument only says that they don't want to
wait after performing the redisplay.


        Stefan

bug#19381: The doc string for `sit-for' is wrong; redisplay isn't unconditionally done.

[Stefan Monnier]

> The doc string for `sit-for', in its first (and most important) line
> states unequivocally that redisplay is performed.  (That line is "Perform
> redisplay, then wait for SECONDS seconds or until input is available.")
> That's not what the function currently does - it doesn't perform
> redisplay if input is already available when it is called.

I've changed it to say:

  "Redisplay, then wait for SECONDS seconds.  Stop when input is available.

So that the "when input is available" part is not so closely tied to
the "wait" part but can also apply to the "redisplay" part.

> It would seem at first sight that the obvious thing to do is correct the
> first line of the doc string.  However there are lots (125) of instances
> of "(sit-for 0)" in Emacs, and a lot of these are probably intended to
> mean "perform redisplay NOW".  So it might be better to make `sit-for'
> always do a redisplay.

While fixing the docstring, I also saw that the code of sit-for hints
at the solution to the above problem: if you just want to redisplay,
don't call (sit-for 0), but call `redisplay' instead (where the `force'
argument can be used to decide whether pending input should prevent
redisplay or not).


        Stefan

bug#19381: The doc string for `sit-for' is wrong; redisplay isn't unconditionally done.

[Alan Mackenzie]

Hello, Stefan.

On Mon, Dec 15, 2014 at 01:50:25PM -0500, Stefan Monnier wrote:
> > The doc string for `sit-for', in its first (and most important) line
> > states unequivocally that redisplay is performed.  (That line is "Perform
> > redisplay, then wait for SECONDS seconds or until input is available.")
> > That's not what the function currently does - it doesn't perform
> > redisplay if input is already available when it is called.

> I've changed it to say:

>   "Redisplay, then wait for SECONDS seconds.  Stop when input is available.

Thanks, that's better.  It's an awkward thing to have to squeeze into
one line.

> So that the "when input is available" part is not so closely tied to
> the "wait" part but can also apply to the "redisplay" part.

> > It would seem at first sight that the obvious thing to do is correct the
> > first line of the doc string.  However there are lots (125) of instances
> > of "(sit-for 0)" in Emacs, and a lot of these are probably intended to
> > mean "perform redisplay NOW".  So it might be better to make `sit-for'
> > always do a redisplay.

> While fixing the docstring, I also saw that the code of sit-for hints
> at the solution to the above problem: if you just want to redisplay,
> don't call (sit-for 0), but call `redisplay' instead (where the `force'
> argument can be used to decide whether pending input should prevent
> redisplay or not).

Yes.  There are 17 calls to `redisplay' in Emacs.  Of the 125 "(sit-for
0)"s, a lot of them will be wanting redisplay, but are being called in
the context of a user typing individual keys.  (This is certainly the
case in edebug.el, for example.)  So, it doesn't matter too much.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).