Re: Ncurses: issue with "rep" capability and --enable-bsdpad (BSD_TPUTS)

2020-05-29 Thread Nicholas Marriott
Hi

I have applied this now, thanks!

> Is there a specific reason why OpenBSD still uses an old ncurses-based
> version?

Updating ncurses in OpenBSD takes quite a lot of time because it is big
and quite integrated - so for example the file layout is where OpenBSD
would put stuff not where upstream does, there are bits in various
places (libcurses, libform, libpanel, libmenu, tic, infocmp, tset,
terminfo.src), quite a lot of local changes (many of them cosmetic), the
build process has a lot of scripts and fiddly stuff. Plus it is used by
a lot of programs both in base and ports, and kind of has to be done all
in one go.

But we could definitely do with an upgrade now... I've started it a
couple of times but never had time to finish. So if you or anyone wants
to try to do it, let me know :-).

Next time it would be nice to see if we could streamline and document it
the upgrade process so we don't end up so far behind so often.



On Mon, May 25, 2020 at 07:02:00PM +0200, Hiltjo Posthuma wrote:
> Hi,
> 
> I've noticed an issue when testing the "rep" capability to st (terminal
> emulator) on OpenBSD.
> 
> After some digging I found the issue is when BSD_TPUTS is defined
> (ncurses_cfg.h).  This enables code for BSD prefix padding and a workaround 
> for
> nethack and ancient BSD terminals as the C comment says in the file
> tinfo/lib_tputs.c.
> 
> I noticed while using the lynx browser, it will incorrectly print repeated
> digit characters. For example:
> 
>   echo "Z000" | lynx -stdin
> 
> will print: ""
> 
> 
> To reproduce it:
> 
> * Use a terminal with the "rep" capability supported and exposed:
>   rep=%p1%c\E[%p2%{1}%-%db
> 
>   infocmp | grep rep=
> 
>   The sequence is documented in terminfo(5).
> 
> * Compile ncurses with BSD_TPUTS defined (or ./configure --enable-bsdpad on 
> Linux).
>   BSD_TPUTS is set by default on OpenBSD in lib/libcurses/ncurses_cfg.h
> 
> I could reproduce the issue on Void Linux also on the latest ncurses snapshot,
> but a fix has been sent upstream and added in 20200523:
> 
> https://github.com/ThomasDickey/ncurses-snapshots/blob/master/NEWS
> 
> "+ add a check in EmitRange to guard against repeat_char emitting digits which
>   could be interpreted as BSD-style padding when --enable-bsdpad is configured
>   (report/patch by Hiltjo Posthuma)."
> 
> 
> A small program to reproduce it:
> 
> #include 
> #include 
> #include 
> 
> int
> main(void)
> {
>   WINDOW *win;
>   
>   win = initscr();
>   printw("Z000");
> 
>   refresh();
>   
>   sleep(5);
> 
>   return 0;
> }
> 
> This program will incorrectly print "" also.
> 
> The backported below patch from ncurses snapshot 20200523 to -current fixes 
> it:
> 
> 
> diff --git a/lib/libcurses/tty/tty_update.c b/lib/libcurses/tty/tty_update.c
> index bd2f088adf6..3d10b76cce2 100644
> --- a/lib/libcurses/tty/tty_update.c
> +++ b/lib/libcurses/tty/tty_update.c
> @@ -540,7 +540,11 @@ EmitRange(const NCURSES_CH_T * ntext, int num)
>   } else {
>   return 1;   /* cursor stays in the middle */
>   }
> - } else if (repeat_char && runcount > SP->_rep_cost) {
> + } else if (repeat_char &&
> +#if BSD_TPUTS
> +   !isdigit(UChar(CharOf(ntext0))) &&
> +#endif
> +   runcount > SP->_rep_cost) {
>   bool wrap_possible = (SP->_curscol + runcount >= 
> screen_columns);
>   int rep_count = runcount;
>  
> 
> Is there a specific reason why OpenBSD still uses an old ncurses-based 
> version?
> 
> -- 
> Kind regards,
> Hiltjo
> 



Ncurses: issue with "rep" capability and --enable-bsdpad (BSD_TPUTS)

2020-05-25 Thread Hiltjo Posthuma
Hi,

I've noticed an issue when testing the "rep" capability to st (terminal
emulator) on OpenBSD.

After some digging I found the issue is when BSD_TPUTS is defined
(ncurses_cfg.h).  This enables code for BSD prefix padding and a workaround for
nethack and ancient BSD terminals as the C comment says in the file
tinfo/lib_tputs.c.

I noticed while using the lynx browser, it will incorrectly print repeated
digit characters. For example:

echo "Z000" | lynx -stdin

will print: ""


To reproduce it:

* Use a terminal with the "rep" capability supported and exposed:
  rep=%p1%c\E[%p2%{1}%-%db

  infocmp | grep rep=

  The sequence is documented in terminfo(5).

* Compile ncurses with BSD_TPUTS defined (or ./configure --enable-bsdpad on 
Linux).
  BSD_TPUTS is set by default on OpenBSD in lib/libcurses/ncurses_cfg.h

I could reproduce the issue on Void Linux also on the latest ncurses snapshot,
but a fix has been sent upstream and added in 20200523:

https://github.com/ThomasDickey/ncurses-snapshots/blob/master/NEWS

"+ add a check in EmitRange to guard against repeat_char emitting digits which
  could be interpreted as BSD-style padding when --enable-bsdpad is configured
  (report/patch by Hiltjo Posthuma)."


A small program to reproduce it:

#include 
#include 
#include 

int
main(void)
{
WINDOW *win;

win = initscr();
printw("Z000");

refresh();

sleep(5);

return 0;
}

This program will incorrectly print "" also.

The backported below patch from ncurses snapshot 20200523 to -current fixes it:


diff --git a/lib/libcurses/tty/tty_update.c b/lib/libcurses/tty/tty_update.c
index bd2f088adf6..3d10b76cce2 100644
--- a/lib/libcurses/tty/tty_update.c
+++ b/lib/libcurses/tty/tty_update.c
@@ -540,7 +540,11 @@ EmitRange(const NCURSES_CH_T * ntext, int num)
} else {
return 1;   /* cursor stays in the middle */
}
-   } else if (repeat_char && runcount > SP->_rep_cost) {
+   } else if (repeat_char &&
+#if BSD_TPUTS
+   !isdigit(UChar(CharOf(ntext0))) &&
+#endif
+   runcount > SP->_rep_cost) {
bool wrap_possible = (SP->_curscol + runcount >= 
screen_columns);
int rep_count = runcount;
 

Is there a specific reason why OpenBSD still uses an old ncurses-based version?

-- 
Kind regards,
Hiltjo