compute-motion inconsistency

compute-motion inconsistency

[Kim F. Storm]

The interface to compute-motion is inconsistent, and the
implementation is inconsistent too.

Problem is the "width" arg, quoting from the doc string:

| width is the number of columns available to display text;
| this affects handling of continuation lines.
| This is usually the value returned by `window-width', less one (to allow
| for the continuation glyph).

But on window based display, there is no continuation glyph, so
typically the value provided for the width arg is one too small.
I checked the caller of compute-motion (and compute_motion
at the C level), and the calls are not consistent either.

Some calls subtract 1 unconditionally,
some subtract 1 on non-window systems,
and some do not subtract 1 at all.

IIRC, the ones which does this correctly (the second category) I have
fixed myself while working on redisplay over the past years.

I will take care of fixing the remaining cases at the C level.

It is more tricky to fix it at the lisp level (I will do it for the
packages delivered with emacs, but we cannot fix it for external
packages).

For backwards compatibility, I suggest the following change
to compute-motion:

If "width" equals (1- (window-width)) on a window system,
automatically use (window-width) instead.

If "width" equals (window-width) on a non-window system,
automatically subtract 1 (for the continuation glyph).

Then we should change the doc string to read:

| width is the number of columns available to display text;
| this affects handling of continuation lines.
| This is usually the value returned by `window-width',
| or that value minus 1 (for backwards compatibility).


WDYT?

Re: compute-motion inconsistency

[David Kastrup]

storm@cua.dk (Kim F. Storm) writes:

> The interface to compute-motion is inconsistent, and the
> implementation is inconsistent too.
> 
> Problem is the "width" arg, quoting from the doc string:
> 
> | width is the number of columns available to display text;
> | this affects handling of continuation lines.
> | This is usually the value returned by `window-width', less one (to allow
> | for the continuation glyph).
> 
> But on window based display, there is no continuation glyph, so
> typically the value provided for the width arg is one too small.
> I checked the caller of compute-motion (and compute_motion
> at the C level), and the calls are not consistent either.
> 
> Some calls subtract 1 unconditionally,
> some subtract 1 on non-window systems,
> and some do not subtract 1 at all.
> 
> IIRC, the ones which does this correctly (the second category) I
> have fixed myself while working on redisplay over the past years.

I somewhat doubt that they do this correctly.  I think that things
differ again if one customizes the fringes off.

In my opinion, it is a complete mistake not to leave the continuation
glyph mess actually to compute-motion itself as soon as the WINDOW
parameter is actually present.

> For backwards compatibility, I suggest the following change to
> compute-motion:
> 
> If "width" equals (1- (window-width)) on a window system,
> automatically use (window-width) instead.
> 
> If "width" equals (window-width) on a non-window system,
> automatically subtract 1 (for the continuation glyph).

That's not "compatibility", that is madness.  I don't think that this
sort of change, which makes things utterly incomprehensible and
unpredictable for the sake of some backward compatibility idea, is
warranted.

I prefer an incompatible change where just window-width can be used,
always.  We had inconsistent behavior and usage before, tough.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: compute-motion inconsistency

[Kim F. Storm]

David Kastrup <dak@gnu.org> writes:

> storm@cua.dk (Kim F. Storm) writes:
> > 
> > IIRC, the ones which do this correctly (the second category) I
> > have fixed myself while working on redisplay over the past years.
> 
> I somewhat doubt that they do this correctly.  I think that things
> differ again if one customizes the fringes off.

Well, if you configure the fringes off, you don't get any continuation
or truncation glyphs on window based display.  Tough!

I thought about ways to recognize this, but the window based redisplay
code does not handle continuation/truncation glyphs, so it's a bit
tricky, and I decided against using (my) time on this.

I suggest to let the people who turn off the fringes:
 a) accept these inconveniences,
 b) implement this themselves.

> 
> In my opinion, it is a complete mistake not to leave the continuation
> glyph mess actually to compute-motion itself as soon as the WINDOW
> parameter is actually present.

Actually, that's what I suggested to do.

> 
> > For backwards compatibility, I suggest the following change to
> > compute-motion:
> > 
> > If "width" equals (1- (window-width)) on a window system,
> > automatically use (window-width) instead.
> > 
> > If "width" equals (window-width) on a non-window system,
> > automatically subtract 1 (for the continuation glyph).
> 
> That's not "compatibility", that is madness.  I don't think that this
> sort of change, which makes things utterly incomprehensible and
> unpredictable for the sake of some backward compatibility idea, is
> warranted.

So your "solution" is to do the following:

If "width" equals (1- (window-width)) on a window system, use that.
[This behave incorrectly, as it does now].

If "width" equals (window-width) on a window system, use that.
[This is what I suggusted to do].

If "width" equals (window-width) on a non-window system,
automatically subtract 1 (for the continuation glyph).
[This is what I suggusted to do].

If "width" equals (1- (window-width)) on a non-window system,
automatically subtract 1 (for the continuation glyph).
[This will behave incorrectly, whereas it work correctly now,
and is the documented required parameter].


So where my suggested "madness" behaves correctly for all 4
cases, your change will fix one case, leave one case unsolved,
and break one case -- the very case that was previously
documented as the required parameter value.

Maybe that's not madness in your book, but it definitely is
not "compatibility" in my book.
 
> I prefer an incompatible change where just window-width can be used,
> always.  We had inconsistent behavior and usage before, tough.

I don't prefer any specific solution -- that's why I asked: 

WDYT?

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: compute-motion inconsistency

[Stefan Monnier]

How about letting a nil value for WIDTH mean "the width of WINDOW" ?
That will simplify the call sites to boot.


        Stefan

Re: compute-motion inconsistency

[David Kastrup]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> How about letting a nil value for WIDTH mean "the width of WINDOW" ?

The _available_ width of window.

> That will simplify the call sites to boot.

Sounds like a plan.  Why don't I have ideas like that?  Then we can
keep the old meaning of "Do compute-motion as if the available
character slots in the window happened to be WIDTH" while making it
quite easy to fix all uses of it.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: compute-motion inconsistency

[Richard Stallman]

    How about letting a nil value for WIDTH mean "the width of WINDOW" ?
    That will simplify the call sites to boot.

I think that is the best solution, even though it requires
changing all the callers.  The resulting design is clean.

Re: compute-motion inconsistency

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

>     How about letting a nil value for WIDTH mean "the width of WINDOW" ?
>     That will simplify the call sites to boot.
> 
> I think that is the best solution, even though it requires
> changing all the callers.  The resulting design is clean.

Ok.  I will make that change and update the callers.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: compute-motion inconsistency

[Kim F. Storm]

storm@cua.dk (Kim F. Storm) writes:

> Richard Stallman <rms@gnu.org> writes:
>
> >     How about letting a nil value for WIDTH mean "the width of WINDOW" ?
> >     That will simplify the call sites to boot.
> >
> > I think that is the best solution, even though it requires
> > changing all the callers.  The resulting design is clean.
>
> Ok.  I will make that change and update the callers.

Done.  

I also extended the TOPOS argument so that a nil value causes the proper
usable height and width to be calculated.

--
Kim F. Storm  http://www.cua.dk

Re: compute-motion inconsistency

[Andreas Schwab]

I don't know if it is related, but I observe an errorneous behaviour of
latest CVS wrt. scrolling on a termcap frame when continued lines are
involved.  Basically there are too many lines scrolled, and the offset
appears to be related to the number of continued lines.  For example, if
you have a screenful of lines of 80 characters in a 80 character wide
window and you scroll forward the window will actually be scrolled by two
pages.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: compute-motion inconsistency

[Kim F. Storm]

Andreas Schwab <schwab@suse.de> writes:

> I don't know if it is related, but I observe an errorneous behaviour of
> latest CVS wrt. scrolling on a termcap frame when continued lines are
> involved.  

It is related...

>            Basically there are too many lines scrolled, and the offset
> appears to be related to the number of continued lines.  For example, if
> you have a screenful of lines of 80 characters in a 80 character wide
> window and you scroll forward the window will actually be scrolled by two
> pages.

I just installed a fix.  Please try again.

Re: compute-motion inconsistency

[Andreas Schwab]

storm@cua.dk (Kim F. Storm) writes:

> I just installed a fix.  Please try again.

Thanks, it works again.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."