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

[Brett Lymn <blymn%internode.on.net@localhost>]

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

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

 On Thu, Jun 10, 2021 at 03:55:02AM +0000, Michael Forney wrote:
 >  
 >  Thanks for the patch, I can confirm that this does solve the issue
 >  for the application in question (catgirl).
 >  
 
 Excellent, that is a good start.
 
 >  
 >  1. It seems that now the string is truncated by one extra character
 >  (the last column is empty). Since this last character doesn't extend
 >  beyond the end of the line, my expectation is that it should not
 >  be truncated. And indeed, this is how it behaves in ncurses.
 >  
 
 OK, I will fix that - my checks must be off by one.
 
 >  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.
 
 >  3. This seems incorrect to me:
 >  
 
 Yes, it is.  My bad.  I will rework that.  That probable explains why
 the wide char version of the slk test failed after my updates.
 
 >  
 >  Below is an alternative fix I've been testing that seems to solve
 >  these problems. The first thing it does is make _cursesi_waddbytes
 >  break from the loop and return ERR when any of the _cursesi_addwchar
 >  calls returns ERR (not just the last one). This is consistent with
 >  waddnwstr. The second thing it does is delay wrapping the cursor x
 >  position back to zero until after we return ERR if scrolling is not
 >  allowed.
 >  
 
 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 :)
 
 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.
 
 -- 
 Brett Lymn
 --
 Sent from my NetBSD device.
 
 "We are were wolves",
 "You mean werewolves?",
 "No we were wolves, now we are something else entirely",
 "Oh"