Re: lib/56224: libcurses: adding character to last line of non-scrolling window wraps instead of

[Michael Forney <mforney%mforney.org@localhost>]

Thanks for the follow-up!

On 2021-06-09, Brett Lymn <blymn%internode.on.net@localhost> wrote:
>>  2. I was thinking that this needed to be fixed in _cursesi_addwchar,
>>  not _cursesi_addwbytes. Otherwise, addwstr and addch at the last
>>  column still wrap to column 0 (though, addwstr already truncates
>>  since it returns on the first ERR, whereas _curses_waddbytes ignores
>>  all errors except for the last character).
>>
>
> No, _cursesi_addwchar is used by other routines that really should be
> inheriting the same behaviour, rather than fix each one individually I
> went for fixing it in the common routine so they all behaved the same.
> I think addwstr is the outlier in this instance.  I will look at fixing
> that.

Hmm... either I'm misunderstanding you, or you me. I agree that the
fix should be in _cursesi_addwchar rather than fixing each one
individually. _cursesi_waddbytes ends up at _cursesi_addwchar, but not
everything ends up at _cursesi_waddbytes. Fixing the wrap in
_cursesi_addwchar should fix it for everything.

> I may have read the SUSv2 section on the scrolling behaviour incorrectly
> but my interpretation is that trying to add a string at the bottom of
> the screen that would cause a scroll when scrollok is false should
> result in a silent truncation not ERR which is why I did what I did.
> The effect of my change is a slk routine that used to strangely return
> ERR no longer does so... I would like to keep that behaviour :)

Yeah, it doesn't seem to provide very many details on when exactly ERR
should be returned.

The current behavior is that the addch functions return ERR when
adding at the bottom right corner, and I think it makes sense to
return ERR from addstr functions when they encounter ERR from addch.
This is what addwstr already does and is at least consistent with
ncurses.

I am not familiar with the slk routines, so I can't comment on the
interaction with those. Perhaps they could ignore ERR from addstr if
they don't care about it?

> The patch you have proposed does not look right, with your fix you will
> have possibly not reset x to 0 but ISPASTEOL will be unset so the unset
> of the ISPASTEOL flag needs to be moved too which does make some sense
> to have that after a possible error return.  I will have a look at doing
> that.

Yeah, I was not quite sure how __ISPASTEOL fit into the picture. I
agree that it only makes sense to clear it when the cursor is actually
wrapped.

One version of a patch I was testing had _cursesi_addwchar set
__ISPASTEOL when it fails to move the cursor past the end of the line
(line 598 of addbytes.c in current trunk), since this is what happens
in _curses_addbyte (with DISABLE_WCHAR, line 331). However, this had a
bad interaction with mvaddch, since the flag isn't cleared during
wmove. I do not quite understand why __ISPASTEOL is a per-line flag
instead of a per-window flag, since it seems to be a property of the
cursor, not the line. But perhaps this is a discussion for another
day.