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

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

The following reply was made to PR lib/56224; it has been noted by GNATS.

From: Michael Forney <mforney%mforney.org@localhost>
To: Brett Lymn <blymn%internode.on.net@localhost>
Cc: gnats-bugs%netbsd.org@localhost, blymn%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, 
	netbsd-bugs%netbsd.org@localhost
Subject: Re: lib/56224: libcurses: adding character to last line of
 non-scrolling window wraps instead of
Date: Thu, 10 Jun 2021 00:56:15 -0700

 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.