Re: Quad-PW on macOS

[Dr . Jürgen Sauermann]

    Hi Paul,
    
    thanks, included in SVN 1809.
    
    No need to #include config.h because Common.hh
    #includes it and there
    are reasons to include it in a central place (i.e. Common.hh) rather
    than in
    individual .cc files.
    
    Best Regards,
    Jürgen

On 12/30/24 07:13, Paul Rockwell wrote:

Jürgen,

May I suggest a couple of changes to src/main.cc?

The first thing is a. bug - there's no #include "config.h"  in main.cc  HAVE_IOCTL_TIOCGWINSZ  is undefined and code that is conditionalized based on its existence

won't be compiled. 

I also looked at the changes in context, and think that the code can be streamlined a bit.

* the signal handler code for SIGWINCH can be conditionalized based on whether HAVE_IOCTL_TIOCGWINSZ is set.  My thought is, if we aren't going to use SIGWINCH to change the

column width, why configure the signal handler?

* If the ioctl is available but the preferenece WINCH_sets_pw is not set, there's also no need to enable the signal handler.

* The signal handler now has code to make sure that ⎕PW is set between max and min valuse.

* There's overlap in the code in the SIGWINCH signal handler and the initial set up code based on whether preference WINCH_sets_pw is set or not

  later in the code. I simplified it to call the signal handler routine (essentiallly pretending that a signal was received at startup time).

Here's the diffs betwen my code and SVN1808 - it includes both my proposed code and the #include of config.h:

===================================================================

--- src/main.cc (revision 1808)

+++ src/main.cc (working copy)

@@ -41,6 +41,8 @@

 #include <sys/socket.h>

 #include <sys/types.h>

 

+#include "config.h"

+

 #if HAVE_IOCTL_TIOCGWINSZ   // plat form has ioctl(fd, TIOCGWINSZ. ...)

 # include <sys/ioctl.h>

 #endif

@@ -132,17 +134,28 @@

    got_WINCH = true;

 

 struct winsize wsize;

-   // TIOCGWINSZ is 0x5413 on GNU/Linux. We use 0x5413 instead of TIOCGWINSZ

-   // if TIOCGWINSZ is not #defined on some platform.

+

+#if HAVE_IOCTL_TIOCGWINSZ

+   // query the window size and set ⎕PW if we have ioctl TIOCGWINSZ

    //

-#ifndef TIOCGWINSZ

-# define TIOCGWINSZ 0x5413

+    

+   // MAX_Quad_PW is 10,000 or so, which is certainly larger than

+   // the number of screen columns. We trust TIOCGWINSZ only if

+   // it is resonably small. We allow it to be too small (and then

+   // increase it). but ignore it if it is too large.

+   //

+   wsize.ws_col = MAX_Quad_PW;   // invalidate the column count

+   if (0 == ioctl(STDIN_FILENO, TIOCGWINSZ, &wsize))

+      {

+        if (wsize.ws_col < MIN_Quad_PW)   // increase too small ws_col

+           wsize.ws_col = MIN_Quad_PW;

+        if (wsize.ws_col > MAX_Quad_PW)   // limit ws_col

+           wsize.ws_col = MAX_Quad_PW;

+        Workspace::set_PW(wsize.ws_col, LOC);

+      }

 #endif

-   if (0 != ioctl(STDIN_FILENO, TIOCGWINSZ, &wsize))   return;

-   if (wsize.ws_col < MIN_Quad_PW)   return;

-   if (wsize.ws_col > MAX_Quad_PW)   return;

-

-   Workspace::set_PW(wsize.ws_col, LOC);

+   // just return if we don't have ioctl TIOCGWINSZ

+   return;

 }

 //----------------------------------------------------------------------------

 /// old sigaction argument for SIGUSR1

@@ -451,34 +464,25 @@

    sigaction(SIGTERM,  &new_TERM_action,      &old_TERM_action);

    sigaction(SIGHUP,   &new_HUP_action,       &old_HUP_action);

    signal(SIGCHLD, SIG_IGN);   // do not create zombies

-   if (UserPreferences::uprefs.WINCH_sets_pw)

-      {

-        sigaction(SIGWINCH, &new_WINCH_action, &old_WINCH_action);

-        signal_WINCH_handler(0);   // pretend window size change

-      }

 

 #if HAVE_IOCTL_TIOCGWINSZ

-   // The platform supports reading back of the window size. If the user

-   // wants ⎕PW to be controlled by the window size, then she most likely

-   // also wants ⎕PW to be controlled by the window size at start-up.

+   // Enable the ability to change ⎕PW on window resize only if the

+   // platform supports ioctl TIOCGWINSZ

    //

+    

    if (UserPreferences::uprefs.WINCH_sets_pw)

       {

-        winsize win_size;

-        win_size.ws_col = MAX_Quad_PW;   // invalidate the column count

-        if (0 == ioctl(STDIN_FILENO, TIOCGWINSZ, &win_size))

-           {

-             // MAX_Quad_PW is 10,000 or so, which is certainly larger than

-             // the number of screen columns. We trust TIOCGWINSZ only if 

-             // it is resonably small. We allow it to be too small (and then

-             // increase it). but ignore it if it is too large.

-             //

-             if (win_size.ws_col < MIN_Quad_PW)   // increase too small ws_col

-                win_size.ws_col = MIN_Quad_PW;

-

-             if (win_size.ws_col < MAX_Quad_PW)   // ognmore large ws_col

-                Workspace::set_PW(win_size.ws_col, LOC);

-           }

+        // IF WINCH_sets_pw preference is enabled, set up a handler for the

+        // SIGWINCH signal.

+        sigaction(SIGWINCH, &new_WINCH_action, &old_WINCH_action);

+         

+        // The platform supports reading back of the window size. If the user

+        // wants ⎕PW to be controlled by the window size, then she most likely

+        // also wants ⎕PW to be controlled by the window size at start-up.

+        // We do this by by using the WINCH signal handler to pretend that

+        // there's a window size change

+        

+        signal_WINCH_handler(0);   // pretend window size change

       }

 #endif

- paul

On Dec 29, 2024, at 8:08 AM, Dr. Jürgen Sauermann <mail@xn--jrgen-sauermann-zvb.de> wrote:

Hi Paul,

thanks, fixed in SVN 1808.

Best Regards,
Jürgen