Bill-Gray / PDCursesMod

Public Domain Curses - a curses library for environments that don't fit the termcap/terminfo model, modified and extended from the 'official' version
https://www.projectpluto.com/win32a.htm
341 stars 73 forks source link

infinity cycle in screen initialization, when pdcurses app is used as pager #256

Closed okbob closed 1 year ago

okbob commented 1 year ago

I try to use pdcurses in application with redirected stdin (cat file| myapp)

there is some pseudocode like

f_tty = fopen("/dev/tty", "r+")
newterm(termname(), stdout, f_tty);

This code works only when f_tty is same like stdin. In other cases It hangs in cycle

    while( PDC_get_rows( ) < 1 && PDC_get_columns( ) < 1)
      ;     /* wait for screen to be drawn and size determined */

PDC_get_rows returns always -1

Setting correct resize_term(size.ws_row, size.ws_col) doesn't help

backtrace is:

Breakpoint 2, PDC_get_rows () at ../vt/pdcgetsc.c:25
25      PDC_LOG(("PDC_get_rows() - called\n"));
(gdb) bt
#0  PDC_get_rows () at ../vt/pdcgetsc.c:25
#1  0x00007f46d7f34267 in PDC_scr_open () at ../vt/pdcscrn.c:322
#2  0x00007f46d7f216ad in initscr () at ../pdcurses/initscr.c:164
#3  0x00007f46d7f21f23 in newterm (type=0x7f46d7f43260 <_termname.2> "pdcurses", outfd=0x7f46d7f01780 <_IO_2_1_stdout_>, 
    infd=0x8183b0) at ../pdcurses/initscr.c:334
#4  0x0000000000406def in main (argc=<optimized out>, argv=<optimized out>) at src/pspg.c:2961
(gdb) c
okbob commented 1 year ago

But now, I look on newterm function, and all is clean now

SCREEN *newterm(const char *type, FILE *outfd, FILE *infd)
{
    PDC_LOG(("newterm() - called\n"));

    INTENTIONALLY_UNUSED_PARAMETER( type);
    INTENTIONALLY_UNUSED_PARAMETER( outfd);
    INTENTIONALLY_UNUSED_PARAMETER( infd);
    return initscr() ? SP : NULL;
}

the all parameters are ignored :-/

Is there possibility how to use input from some specified stream instead just stdin?

Minimally the following code is partially wrong:

static void sigwinchHandler( int sig)
{
   struct winsize ws;

   INTENTIONALLY_UNUSED_PARAMETER( sig);
   if (ioctl(STDIN_FILENO, TIOCGWINSZ, &ws) != -1)
      if( PDC_rows != ws.ws_row || PDC_cols != ws.ws_col)
         {
         PDC_rows = ws.ws_row;
         PDC_cols = ws.ws_col;
         PDC_resize_occurred = TRUE;
         if (SP)
            SP->resized = TRUE;
         }
}
     ...

There should be STDOUT_FILENO used, because for you, the dimensions are interesting for output.

GitMensch commented 1 year ago

If it isn't then this function should likely check the input parameter and return ERR if it isn't NULL / stdin.

okbob commented 1 year ago

No, there is not any check. Unfortunately, for using pspg for MSWin I need fully implemented newterm function. I cannot to redirect stdin inside the pager.

This is fundamental functionality for pager. On second hand, the complex application written primary for ncurses like pspg https://github.com/okbob/pspg is working with pdcurses very well with mouse support (and without bigger changes). Unfortunately, it cannot be used like pager now.

Bill-Gray commented 1 year ago

Um. An interesting point... I had not really considered the use for that redirection in PDCurses, which does not follow the terminfo model that ncurses uses.

At least for input, I think we can get this to work. The SCREEN structure would hold a new FILE *input_fp member (within the 'opaque' structure), which would default to NULL. When set to a non-NULL FILE pointer via newterm(), we would read characters from that file instead of doing platform-dependent checking for keyboard input.

It occurs to me that one might have redirected stdin by running, say,

./curses_program < /tmp/input_file.txt

Right now, that works with ncurses and with no flavor (except maybe DOS ones?) of PDCurses or PDCursesMod. The VT and framebuffer platforms, for example, assume input comes from a terminal rather than a file. So our check for reading a character should be

   if( SP->opaque->input_fp)
      fread() from SP->opaque->input_fd;
   else if( !isatty( STDIN_FILENO))
      fread() from STDIN;
   else    /* it's a for-real terminal */
      read the character using the platform-dependent code;

If that makes sense to you, I don't think it would be difficult to implement it, first for VT (which, I gather, is your current focus). We could then add it to other platforms.

We also have the question of what it means if you redirect the output. I don't have an answer for that one yet. The question has yet to arise. If it does, it will warrant discussion under a new issue.

In the meantime, I agree with @GitMensch that we should return ERR for non-NULL outfd parameters. (And an assert( NULL == outfd); would be good, with similar error-checking and/or assert()s for other terminfo function that are stubs in PDCursesMod.... such an assert() here would have told you right away that we weren't handling a non-NULL infd parameter, for example.)

GitMensch commented 1 year ago

If that makes sense to you, I don't think it would be difficult to implement it, first for VT (which, I gather, is your current focus). We could then add it to other platforms.

It makes at least sense to me ;-)

We also have the question of what it means if you redirect the output. I don't have an answer for that one yet.

Currently redirecting the output raises an error because col/lines is zero or the type does not match, see PDC_scr_open(). This has come up before "multiple times", related are also #15 and #50.

okbob commented 1 year ago

ne 18. 12. 2022 v 17:32 odesílatel Simon Sobisch @.***> napsal:

If that makes sense to you, I don't think it would be difficult to implement it, first for VT (which, I gather, is your current focus). We could then add it to other platforms.

It makes at least sense to me ;-)

:-)

I'll check it immediately

We also have the question of what it means if you redirect the output. I don't have an answer for that one yet.

I was wrong, there is redirected input (in this case)

When you use write pager, like "less" or "more", you cannot to read from stdin, but you have to read from tty

example:

classic app mode - pspg -f xxx .. both stdin, stdout are attached to tty

pager mode - cat xxx | pspg .. stdin is attached to pipe, but interactive input should be redirected to tty.

The pattern how to solve is using newterm function instead initscr, where you can explicitly specify input stream,

Theoretically, it can be the same with output too.

Regards

Pavel

Currently redirecting the output raises an error because col/lines is zero or the type does not match, see PDC_scr_open(). This has come up before "multiple times", related are also #15 https://github.com/Bill-Gray/PDCursesMod/issues/15 and #50 https://github.com/Bill-Gray/PDCursesMod/issues/50.

— Reply to this email directly, view it on GitHub https://github.com/Bill-Gray/PDCursesMod/issues/256#issuecomment-1356832641, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEFO4YWSA6FBCYU6W2VHELWN44BDANCNFSM6AAAAAATCODGOM . You are receiving this because you authored the thread.Message ID: @.***>

Bill-Gray commented 1 year ago

Currently redirecting the output raises an error because col/lines is zero or the type does not match

Yup, that's what I'm seeing. The situation is more complicated than my proposed remedy suggests.

For the initial issue raised by Pavel, we should have no serious problem. We will read from the actual terminal when we want (for example) to find out how many lines and columns the terminal has, and read from the file specified by infd otherwise.

Input file redirection complicates this, because we lose access to the terminal. We send a command to inquire about the screen size, and the terminal replies, but we don't "see" it because our input is coming from the redirected input. A bit of searching for how one gets a file descriptor to the "real" terminal hasn't turned up anything useful yet. I assume there's a way to do it, though, and will continue looking.

Interestingly, one can get output redirection to work by simply modifying the line

    const int stdout_fd = 0;

in the put_to_stdout() function in vt/pdcdisp.c so that the variable is equal to 2. This is actually the file descriptor for stderr. Do this, and run a command such as ./testcurs > /tmp/output.txt, and all is well : the PDCursesMod output goes to the (unredirected, still to the terminal) stderr, and if you have any printf() or similar calls in your program, they're appropriately re-directed to /tmp/output.txt.

Properly speaking, what we need is some sort of const int stdout_fd = get_terminal_output_file_descriptor();, i.e., the problem parallels that of getting the actual input file descriptor. At the very least, that function might be along the lines of

   if( isatty( STDOUT_FILENO))     /* stdout is a tty;  use stdout */
      return( STDOUT_FILENO);
   else if( isatty(STDERR_FILENO))    /* stdout isn't a tty,  but stderr is;  use stderr */
      return( STDERR_FILENO);
   else         /* both are redirected,  and we're out of options */
      perror( "Both stdout and stderr are redirected");

In Windows, we'd use

if( FILE_TYPE_CHAR == GetFileType( GetStdHandle( STD_OUTPUT_HANDLE)))

to test for tty-ness. (I think. Just gave it a try under Wine, and it failed. But Wine's console support isn't perfect.)

okbob commented 1 year ago

changing STDIN to 2 - redirect to stderr - and to tty allows to start pspg in pager mode. But it crashes on assert

#5  0x00007fd54bd5d656 in __GI___assert_fail (assertion=0x7fd54bf35222 "(c[0] & 0xc0) == 0x80", file=0x7fd54bf351af "../vt/pdckbd.c", line=546, 
    function=0x7fd54bf35498 <__PRETTY_FUNCTION__.2> "PDC_get_key") at assert.c:101
#6  0x00007fd54bf2e04e in PDC_get_key () at ../vt/pdckbd.c:546
#7  0x00007fd54bf19dbf in wgetch (win=0xcc6190) at ../pdcurses/getch.c:541
Bill-Gray commented 1 year ago

Sorry, I've mixed issues here. The idea was to support redirected output by changing stdout_fd. (I've updated my previous comment to make some of that clearer.) I still don't know of any suitable trickery for handling redirected input.

The issue you initially raised (and which it sounds as if is still your focus) is what to do if newterm() is called, and the input stream is something other than stdin. In that case, we should still read from stdin during initwin(), because when we "read a character" at that point, we're getting information about the terminal (screen size, for example). Once initwin() has done its thing, we should ignore the keyboard and "read characters" from fdin.

To address a further issue you didn't raise (sorry!), I can't think of a use case for fdout != stdout, and therefore have no idea what the "right" way of handling it would be.

okbob commented 1 year ago

here is minimalistic very ugly hack patch. With this patch, the pspg can be used as pager. Unfortunately, it fully breaks keyboard - only basic ascii keys (pspg commands) works. I think so reason is missing settings for input stream (I opening stream just for any key):

[pavel@localhost vt]$ git diff
diff --git a/vt/pdckbd.c b/vt/pdckbd.c
index c9befcd9..0671d20d 100644
--- a/vt/pdckbd.c
+++ b/vt/pdckbd.c
@@ -38,7 +38,7 @@ static bool check_key( int *c)
 {
     bool rval;
 #ifndef USE_CONIO
-    const int STDIN = 0;
+    const int STDIN = 2;
     struct timeval timeout;
     fd_set rdset;
     extern int PDC_n_ctrl_c;
@@ -65,7 +65,14 @@ static bool check_key( int *c)
        {
        rval = TRUE;
        if( c)
-          *c = getchar( );
+       {
+          FILE *f;
+
+          f = fdopen(dup(2), "r+");
+          *c = fgetc(f);
+          fclose(f);
+
+        }
        }
     else
        rval = FALSE;
diff --git a/vt/pdcscrn.c b/vt/pdcscrn.c
index b6104f58..7061c608 100644
--- a/vt/pdcscrn.c
+++ b/vt/pdcscrn.c
@@ -100,7 +100,7 @@ static int set_win10_for_vt_codes( const bool setting_mode)

 int PDC_rows = -1, PDC_cols = -1;
 bool PDC_resize_occurred = FALSE;
-const int STDIN = 0;
+const int STDIN = 2;
 chtype PDC_capabilities = 0;
 static mmask_t _stored_trap_mbe;

@@ -212,7 +212,7 @@ static void sigwinchHandler( int sig)
    struct winsize ws;

    INTENTIONALLY_UNUSED_PARAMETER( sig);
-   if (ioctl(STDIN_FILENO, TIOCGWINSZ, &ws) != -1)
+   if (ioctl(STDERR_FILENO, TIOCGWINSZ, &ws) != -1)
       if( PDC_rows != ws.ws_row || PDC_cols != ws.ws_col)
          {
          PDC_rows = ws.ws_row;

So probably there are not any other dependencies, and reading from tty device should to work.

Bill-Gray commented 1 year ago

Thank you. You're right, it doesn't quite work, but I think it may be putting us on the right track. The idea of duplicating stderr and opening it for reading had not occurred to me... actually, I'd always thought of stderr (and stdout) as being things you wrote to, not read from.

okbob commented 1 year ago

út 20. 12. 2022 v 20:20 odesílatel Bill Gray @.***> napsal:

Thank you. You're right, it doesn't quite work, but I think it may be putting us on the right track. The idea of duplicating stderr and opening it for reading had not occurred to me... actually, I'd always thought of stderr (and stdout) as being things you wrote to, not read from.

I read about this trick when I wrote pspg. It is not my idea. The stream by itself is not important, you need to connect the correct device.

pspg uses code (and similar code is used by less, more, ... )

bool
open_tty_stream(void)
{

#ifndef __APPLE__

    f_tty = fopen("/dev/tty", "r+");

#endif

    if (!f_tty)
    {
        f_tty = fopen(ttyname(fileno(stdout)), "r");
        if (!f_tty)
        {
            if (isatty(fileno(stderr)))
                f_tty = stderr;
        }
        else
            close_f_tty = true;
    }
    else
        close_f_tty = true;

    return f_tty != NULL;
}

Now this code is well tested (and used) on almost all Unix- like systems, and in almost all situations.

— Reply to this email directly, view it on GitHub https://github.com/Bill-Gray/PDCursesMod/issues/256#issuecomment-1360047212, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEFO4YAX3L6QZGMWP67D23WOIBIJANCNFSM6AAAAAATCODGOM . You are receiving this because you authored the thread.Message ID: @.***>

okbob commented 1 year ago

I am able to run pspg against pdcurses for VT on Linux with attached patch, and it is works pretty well (all functionality, mouse, full keyboard).

diff --git a/pdcurses/initscr.c b/pdcurses/initscr.c
index 02c69c4f..df834a5a 100644
--- a/pdcurses/initscr.c
+++ b/pdcurses/initscr.c
@@ -148,6 +148,8 @@ MOUSE_STATUS Mouse_status;
 extern RIPPEDOFFLINE linesripped[5];
 extern char linesrippedoff;

+extern FILE *__infd;
+
 WINDOW *initscr(void)
 {
     int i;
@@ -161,6 +163,9 @@ WINDOW *initscr(void)
     if (!SP)
         return NULL;

+    if (!__infd)
+        __infd = stdin;
+
     if (PDC_scr_open() == ERR)
     {
         fprintf(stderr, "initscr(): Unable to create SP\n");
@@ -330,7 +335,7 @@ SCREEN *newterm(const char *type, FILE *outfd, FILE *infd)

     INTENTIONALLY_UNUSED_PARAMETER( type);
     INTENTIONALLY_UNUSED_PARAMETER( outfd);
-    INTENTIONALLY_UNUSED_PARAMETER( infd);
+    __infd = infd;
     return initscr() ? SP : NULL;
 }

diff --git a/vt/pdckbd.c b/vt/pdckbd.c
index c9befcd9..5154274a 100644
--- a/vt/pdckbd.c
+++ b/vt/pdckbd.c
@@ -33,12 +33,12 @@ https://www.gnu.org/software/screen/manual/html_node/Control-Sequences.html
 */

 extern bool PDC_resize_occurred;
+FILE      *__infd = NULL;

 static bool check_key( int *c)
 {
     bool rval;
 #ifndef USE_CONIO
-    const int STDIN = 0;
     struct timeval timeout;
     fd_set rdset;
     extern int PDC_n_ctrl_c;
@@ -58,14 +58,14 @@ static bool check_key( int *c)
        return( TRUE);
        }
     FD_ZERO( &rdset);
-    FD_SET( STDIN, &rdset);
+    FD_SET( fileno(__infd), &rdset);
     timeout.tv_sec = 0;
     timeout.tv_usec = 0;
-    if( select( STDIN + 1, &rdset, NULL, NULL, &timeout) > 0)
+    if( select( fileno(__infd) + 1, &rdset, NULL, NULL, &timeout) > 0)
        {
        rval = TRUE;
        if( c)
-          *c = getchar( );
+          *c = fgetc(__infd);
        }
     else
        rval = FALSE;
diff --git a/vt/pdcscrn.c b/vt/pdcscrn.c
index b6104f58..d7ecbae7 100644
--- a/vt/pdcscrn.c
+++ b/vt/pdcscrn.c
@@ -8,6 +8,7 @@
 #include <termios.h>
 #include <sys/ioctl.h>

+extern FILE *__infd;
 static struct termios orig_term;
 #endif

@@ -100,7 +101,6 @@ static int set_win10_for_vt_codes( const bool setting_mode)

 int PDC_rows = -1, PDC_cols = -1;
 bool PDC_resize_occurred = FALSE;
-const int STDIN = 0;
 chtype PDC_capabilities = 0;
 static mmask_t _stored_trap_mbe;

@@ -111,14 +111,14 @@ void PDC_reset_prog_mode( void)
 #ifdef USE_TERMIOS
     struct termios term;

-    tcgetattr( STDIN, &orig_term);
+    tcgetattr( fileno(__infd), &orig_term);
     memcpy( &term, &orig_term, sizeof( term));
     term.c_lflag &= ~(ICANON | ECHO);
     term.c_iflag &= ~ICRNL;
     term.c_cc[VSUSP] = _POSIX_VDISABLE;   /* disable Ctrl-Z */
     term.c_cc[VSTOP] = _POSIX_VDISABLE;   /* disable Ctrl-S */
     term.c_cc[VSTART] = _POSIX_VDISABLE;   /* disable Ctrl-Q */
-    tcsetattr( STDIN, TCSANOW, &term);
+    tcsetattr( fileno(__infd), TCSANOW, &term);
 #endif
 #ifndef _WIN32
     if( !PDC_is_ansi)
@@ -190,7 +190,7 @@ void PDC_scr_close( void)
    set_win10_for_vt_codes( FALSE);
 #else
    #if !defined( DOS)
-      tcsetattr( STDIN, TCSANOW, &orig_term);
+      tcsetattr( fileno(__infd), TCSANOW, &orig_term);
    #endif
 #endif
    PDC_doupdate( );
@@ -212,7 +212,7 @@ static void sigwinchHandler( int sig)
    struct winsize ws;

    INTENTIONALLY_UNUSED_PARAMETER( sig);
-   if (ioctl(STDIN_FILENO, TIOCGWINSZ, &ws) != -1)
+   if (ioctl( fileno(__infd), TIOCGWINSZ, &ws) != -1)
       if( PDC_rows != ws.ws_row || PDC_cols != ws.ws_col)
          {
          PDC_rows = ws.ws_row;
@@ -283,7 +283,7 @@ int PDC_scr_open(void)
     if (!SP || PDC_init_palette( ))
         return ERR;

-    setbuf( stdin, NULL);
+    setbuf(__infd, NULL);
 #ifdef USE_TERMIOS
     sigemptyset(&sa.sa_mask);
     sa.sa_flags = 0;

fix-newterm.patch.txt

I am not able to test other platforms, and I am sure, so this patch is not well integrated to pdcurses well. Just it shows, so there is an possibility to work correctly, and necessity changes are not too big.

I tested it on VT because Linux and VT is my native platform, but I did this work as initial step for porting pspgf to mswin.

Bill-Gray commented 1 year ago

Finally got around to fixing (I'm fairly sure) your original issue : newterm( (ignored), stdout, non-stdin) will now work. Still need to pull your code in from your above comment to allow one to do 'traditional' command-line input redirection, though.

Also, the fix I made applies only to VT and Linux framebuffer/DRM at present; once we've confidence in it, it should be easy enough to extend it to other platforms.

okbob commented 1 year ago

pá 6. 1. 2023 v 3:43 odesílatel Bill Gray @.***> napsal:

Finally got around to fixing (I'm fairly sure) your original issue : newterm( (ignored), stdout, non-stdin) will now work. Still need to pull your code in from your above comment to allow one to do 'traditional' command-line input redirection, though.

Also, the fix I made applies only to VT and Linux framebuffer/DRM at present; once we've confidence in it, it should be easy enough to extend it to other platforms.

Unfortunately it doesn't work, because not the input from tty doesn't work.

0x00007f4fe6dc6021 in __GI___libc_read (fd=5, buf=0x84cae0, nbytes=4096) at ../sysdeps/unix/sysv/linux/read.c:26 Downloading 0.00 MB source file /usr/src/debug/glibc-2.36-8.fc37.x86_64/io/../sysdeps/unix/sysv/linux/read.c 26 return SYSCALL_CANCEL (read, fd, buf, nbytes); (gdb) bt

0 0x00007f4fe6dc6021 in __GI___libc_read (fd=5, buf=0x84cae0,

nbytes=4096) at ../sysdeps/unix/sysv/linux/read.c:26

1 0x00007f4fe6d4eee6 in _IO_new_file_underflow (fp=0x80e3b0) at

/usr/src/debug/glibc-2.36-8.fc37.x86_64/libio/libioP.h:947

2 0x00007f4fe6d4ff06 in GIIO_default_uflow (fp=0x80e3b0) at

/usr/src/debug/glibc-2.36-8.fc37.x86_64/libio/libioP.h:947

3 0x00007f4fe6ed1555 in check_key (c=0x7ffd2a5eb0b8) at ../vt/pdckbd.c:53

4 0x00007f4fe6ed1acc in PDC_get_key () at ../vt/pdckbd.c:386

5 0x00007f4fe6ed202a in PDC_get_key () at ../vt/pdckbd.c:517

6 0x00007f4fe6ebddef in wgetch (win=0x836010) at ../pdcurses/getch.c:541

7 0x000000000042d0d6 in get_ncurses_event @.***=0x7ffd2a5eb620,

@.***=0x7ffd2a5eb22e,

I tested last version, and now it starts and doesn't hangs on initialization, but the input is broken

when I use pspg in pager mode, it doesn't work ever. when I use pspg in classic mode, it works, but using mouse click hangs in read

So last changes fixes issue with start, but now, it works worse than before

Regards

Pavel

— Reply to this email directly, view it on GitHub https://github.com/Bill-Gray/PDCursesMod/issues/256#issuecomment-1373070314, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEFO435JIYSQ3MKNTP66BTWQ6BFVANCNFSM6AAAAAATCODGOM . You are receiving this because you authored the thread.Message ID: @.***>

okbob commented 1 year ago

pá 6. 1. 2023 v 5:11 odesílatel Pavel Stehule @.***> napsal:

pá 6. 1. 2023 v 3:43 odesílatel Bill Gray @.***> napsal:

Finally got around to fixing (I'm fairly sure) your original issue : newterm( (ignored), stdout, non-stdin) will now work. Still need to pull your code in from your above comment to allow one to do 'traditional' command-line input redirection, though.

Also, the fix I made applies only to VT and Linux framebuffer/DRM at present; once we've confidence in it, it should be easy enough to extend it to other platforms.

Unfortunately it doesn't work, because not the input from tty doesn't work.

0x00007f4fe6dc6021 in __GI___libc_read (fd=5, buf=0x84cae0, nbytes=4096) at ../sysdeps/unix/sysv/linux/read.c:26 Downloading 0.00 MB source file /usr/src/debug/glibc-2.36-8.fc37.x86_64/io/../sysdeps/unix/sysv/linux/read.c 26 return SYSCALL_CANCEL (read, fd, buf, nbytes); (gdb) bt

0 0x00007f4fe6dc6021 in __GI___libc_read (fd=5, buf=0x84cae0,

nbytes=4096) at ../sysdeps/unix/sysv/linux/read.c:26

1 0x00007f4fe6d4eee6 in _IO_new_file_underflow (fp=0x80e3b0) at

/usr/src/debug/glibc-2.36-8.fc37.x86_64/libio/libioP.h:947

2 0x00007f4fe6d4ff06 in GIIO_default_uflow (fp=0x80e3b0) at

/usr/src/debug/glibc-2.36-8.fc37.x86_64/libio/libioP.h:947

3 0x00007f4fe6ed1555 in check_key (c=0x7ffd2a5eb0b8) at ../vt/pdckbd.c:53

4 0x00007f4fe6ed1acc in PDC_get_key () at ../vt/pdckbd.c:386

5 0x00007f4fe6ed202a in PDC_get_key () at ../vt/pdckbd.c:517

6 0x00007f4fe6ebddef in wgetch (win=0x836010) at ../pdcurses/getch.c:541

7 0x000000000042d0d6 in get_ncurses_event @.***=0x7ffd2a5eb620,

@.***=0x7ffd2a5eb22e,

I tested last version, and now it starts and doesn't hangs on initialization, but the input is broken

when I use pspg in pager mode, it doesn't work ever. when I use pspg in classic mode, it works, but using mouse click hangs in read

it is strange - using mouse wheel is ok, but mouse click breaks synchronizations, and it falls to infinity cycle of read

So last changes fixes issue with start, but now, it works worse than before

you can try to use pspg for check

pspg -f tests/pg_class.txt # -- classic mode cat tests/pg_class.txt | pspg -- pager mode

There are PDCurses.md about compilation pspg with pdcurses

Regards

Pavel

Regards

Pavel

— Reply to this email directly, view it on GitHub https://github.com/Bill-Gray/PDCursesMod/issues/256#issuecomment-1373070314, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEFO435JIYSQ3MKNTP66BTWQ6BFVANCNFSM6AAAAAATCODGOM . You are receiving this because you authored the thread.Message ID: @.***>

Bill-Gray commented 1 year ago

Ah, hang on, I see what you mean... I was being quite dense here; it's nowhere near as simple as what I added.

A workaround you might consider : the WinGUI, SDL1/2, and X11 ports ought to work with paging programs without much trouble. They look for keyboard/mouse events via the Win32 API or SDLn or X11 libraries, rather than reading from stdin. So stdin redirection doesn't affect them.

For example, I patched testcurs.c with

--- a/demos/testcurs.c
+++ b/demos/testcurs.c
@@ -162,6 +162,13 @@ int main(int argc, char *argv[])
                 case 'z':
                     report_mouse_movement = TRUE;
                     break;
+                case 'i':
+                    {
+                    char buff[80];
+
+                    if( fgets( buff, sizeof( buff), stdin))
+                        printf( "Got '%s'\n", buff);
+                    }
                 default:
                     break;

and ran testcurs -i < junk.txt with WinGUI. The first line of junk.txt got read from stdin and was duly printed to the terminal. But testcurs got its input from the keyboard and mouse, unaffected by the redirection. Similarly for type junk.txt | testcurs -i. I tried the same experiment with SDL2 and X11 successfully.

WinCon says "Redirection is not supported" (this is a known issue, and causes grief in some situations.) I think the DOS port would work, since it reads keys and mouse via BIOS rather than stdin, but haven't tested it.

So for your original goal of getting pspg working on Windows, use of WinGUI actually ought to do the trick. However, I can imagine other situations where one might want to read from stdin while having PDCursesMod grab the keyboard and mouse input, and it'd be nice if it were possible in the VT and Linux framebuffer/DRM ports. (And very much so in WinCon.)

okbob commented 1 year ago

There is little bit unhappy written initialization. Almost all code is called from initscr. But when new_term is used, then it can be called before initscr or without initscr. Current state is wrong. The input stream is initialized (inside initscr is stdin used always), and in next step (too late) is set SP->opaque->input_fd from new_term. Storing output_fd, input_fd in opaque structure is not too happy, because you need to reset buffer in PDC_scr_open but opaque is initialized later when colour attributes are initialized.

With patch the pdcurses works with pspg well in both modes:

diff --git a/curspriv.h b/curspriv.h
index c8625e96..71015ca7 100644
--- a/curspriv.h
+++ b/curspriv.h
@@ -155,7 +155,9 @@ struct _opaque_screen_t
    WINDOW **window_list;
    unsigned trace_flags;
    bool want_trace_fflush;
-   FILE *output_fd, *input_fd;
 };

+extern FILE *output_fd;
+extern FILE *input_fd;
+
 #endif /* __CURSES_INTERNALS__ */
diff --git a/pdcurses/initscr.c b/pdcurses/initscr.c
index 054c844a..672b784c 100644
--- a/pdcurses/initscr.c
+++ b/pdcurses/initscr.c
@@ -143,6 +143,9 @@ int LINES = 0;                        /* current terminal height */
 int COLS = 0;                         /* current terminal width */
 int TABSIZE = 8;

+FILE *input_fd = NULL;
+FILE *output_fd = NULL;
+
 MOUSE_STATUS Mouse_status;

 extern RIPPEDOFFLINE linesripped[5];
@@ -161,6 +164,11 @@ WINDOW *initscr(void)
     if (!SP)
         return NULL;

+    if (!input_fd)
+        input_fd = stdin;
+    if (!output_fd)
+        output_fd = stdout;
+
     if (PDC_scr_open() == ERR)
     {
         fprintf(stderr, "initscr(): Unable to create SP\n");
@@ -330,11 +338,11 @@ SCREEN *newterm(const char *type, FILE *outfd, FILE *infd)

     PDC_LOG(("newterm() - called\n"));
     INTENTIONALLY_UNUSED_PARAMETER( type);
+
+    /* These global variables should be set before initscr */
+    output_fd = outfd;
+    input_fd = infd;
     win = initscr( );
-    if( win && outfd != stdout)
-        SP->opaque->output_fd = outfd;
-    if( win && infd != stdin)
-        SP->opaque->input_fd = infd;
     return win ? SP : NULL;
 }

diff --git a/vt/pdckbd.c b/vt/pdckbd.c
index 050e7974..0f5508cc 100644
--- a/vt/pdckbd.c
+++ b/vt/pdckbd.c
@@ -38,24 +38,12 @@ static bool check_key( int *c)
 {
     bool rval;
 #ifndef USE_CONIO
-    const int STDIN = 0;
     struct timeval timeout;
     fd_set rdset;
     extern int PDC_n_ctrl_c;

     if( PDC_resize_occurred)
        return( TRUE);
-    if( SP->opaque->input_fd)
-    {
-        rval = !feof( SP->opaque->input_fd);
-        if( rval && c)
-        {
-            *c = fgetc( SP->opaque->input_fd);
-            if( *c == EOF)
-                rval = FALSE;
-        }
-        return( rval);
-    }
 #ifdef LINUX_FRAMEBUFFER_PORT
     PDC_check_for_blinking( );
 #endif
@@ -69,14 +57,14 @@ static bool check_key( int *c)
        return( TRUE);
        }
     FD_ZERO( &rdset);
-    FD_SET( STDIN, &rdset);
+    FD_SET( fileno( input_fd), &rdset);
     timeout.tv_sec = 0;
     timeout.tv_usec = 0;
-    if( select( STDIN + 1, &rdset, NULL, NULL, &timeout) > 0)
+    if( select( fileno( input_fd) + 1, &rdset, NULL, NULL, &timeout) > 0)
        {
        rval = TRUE;
        if( c)
-          *c = getchar( );
+          *c = fgetc( input_fd);
        }
     else
        rval = FALSE;
diff --git a/vt/pdcscrn.c b/vt/pdcscrn.c
index 380b125f..a0daa90e 100644
--- a/vt/pdcscrn.c
+++ b/vt/pdcscrn.c
@@ -100,7 +100,6 @@ static int set_win10_for_vt_codes( const bool setting_mode)

 int PDC_rows = -1, PDC_cols = -1;
 bool PDC_resize_occurred = FALSE;
-const int STDIN = 0;
 chtype PDC_capabilities = 0;
 static mmask_t _stored_trap_mbe;

@@ -111,14 +110,14 @@ void PDC_reset_prog_mode( void)
 #ifdef USE_TERMIOS
     struct termios term;

-    tcgetattr( STDIN, &orig_term);
+    tcgetattr( fileno( input_fd), &orig_term);
     memcpy( &term, &orig_term, sizeof( term));
     term.c_lflag &= ~(ICANON | ECHO);
     term.c_iflag &= ~ICRNL;
     term.c_cc[VSUSP] = _POSIX_VDISABLE;   /* disable Ctrl-Z */
     term.c_cc[VSTOP] = _POSIX_VDISABLE;   /* disable Ctrl-S */
     term.c_cc[VSTART] = _POSIX_VDISABLE;   /* disable Ctrl-Q */
-    tcsetattr( STDIN, TCSANOW, &term);
+    tcsetattr( fileno( input_fd), TCSANOW, &term);
 #endif
 #ifndef _WIN32
     if( !PDC_is_ansi)
@@ -190,7 +189,7 @@ void PDC_scr_close( void)
    set_win10_for_vt_codes( FALSE);
 #else
    #if !defined( DOS)
-      tcsetattr( STDIN, TCSANOW, &orig_term);
+      tcsetattr( fileno( input_fd), TCSANOW, &orig_term);
    #endif
 #endif
    PDC_doupdate( );
@@ -286,7 +285,7 @@ int PDC_scr_open(void)
     if (!SP || PDC_init_palette( ))
         return ERR;

-    setbuf( stdin, NULL);
+    setbuf( input_fd, NULL);
 #ifdef USE_TERMIOS
     sigemptyset(&sa.sa_mask);
     sa.sa_flags = 0;
okbob commented 1 year ago

should be fully fixed by 9c16db9