Re: fix for readline terminal size problems when window is resized with open pager - Mailing list pgsql-hackers

Merlin Moncure <mmoncure@gmail.com> writes:
> On Wed, Dec 16, 2015 at 1:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> So I now think that print.c shouldn't be involved at all, and the right
>> thing to do is just have gets_interactive() invoke the resize function
>> immediately before calling readline().  That will still leave a window
>> for SIGWINCH to be missed, but it's a pretty tiny one.

> right -- agreed.

I tried that and found out that the first call dumps core because readline
hasn't been initialized yet.  To fix that, we need an explicit call to
rl_initialize() instead of depending on readline() to do it for us.
So I arrive at the attached modified patch.

>> And somebody oughta report this as a libreadline bug ...

> See https://bugs.python.org/issue23735 for background.  Apparently
> this is expected behavior (and we are far from the only ones
> complaining about it):

> "And so we reach where we are. If a SIGWINCH arrives while readline is
> not active, and the application using callback mode does not catch it and
> tell readline about the updated screen dimensions (rl_set_screen_size()
> exists for this purpose), readline will not know about the new size."

Meh.  At the very least, this is a serious documentation failure on their
part, because their docs about interrupt handling look exactly the same
as before, to wit saying exactly the opposite of this.

            regards, tom lane

diff --git a/configure b/configure
index 8c61390..5772d0e 100755
*** a/configure
--- b/configure
*************** if test x"$pgac_cv_var_rl_completion_app
*** 13232,13238 ****
  $as_echo "#define HAVE_RL_COMPLETION_APPEND_CHARACTER 1" >>confdefs.h

  fi
!   for ac_func in rl_completion_matches rl_filename_completion_function
  do :
    as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
  ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
--- 13232,13238 ----
  $as_echo "#define HAVE_RL_COMPLETION_APPEND_CHARACTER 1" >>confdefs.h

  fi
!   for ac_func in rl_completion_matches rl_filename_completion_function rl_reset_screen_size
  do :
    as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
  ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.in b/configure.in
index b5868b0..44f832f 100644
*** a/configure.in
--- b/configure.in
*************** LIBS="$LIBS_including_readline"
*** 1635,1641 ****

  if test "$with_readline" = yes; then
    PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER
!   AC_CHECK_FUNCS([rl_completion_matches rl_filename_completion_function])
    AC_CHECK_FUNCS([append_history history_truncate_file])
  fi

--- 1635,1641 ----

  if test "$with_readline" = yes; then
    PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER
!   AC_CHECK_FUNCS([rl_completion_matches rl_filename_completion_function rl_reset_screen_size])
    AC_CHECK_FUNCS([append_history history_truncate_file])
  fi

diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c
index e377104..c0c5524 100644
*** a/src/bin/psql/input.c
--- b/src/bin/psql/input.c
*************** gets_interactive(const char *prompt)
*** 65,70 ****
--- 65,81 ----
      {
          char       *result;

+         /*
+          * Some versions of readline don't notice SIGWINCH signals that arrive
+          * when not actively reading input.  The simplest fix is to always
+          * re-read the terminal size.  This leaves a window for SIGWINCH to be
+          * missed between here and where readline() enables libreadline's
+          * signal handler, but that's probably short enough to be ignored.
+          */
+ #ifdef HAVE_RL_RESET_SCREEN_SIZE
+         rl_reset_screen_size();
+ #endif
+
          /* Enable SIGINT to longjmp to sigint_interrupt_jmp */
          sigint_interrupt_enabled = true;

*************** initializeInput(int flags)
*** 330,335 ****
--- 341,347 ----
          char        home[MAXPGPATH];

          useReadline = true;
+         rl_initialize();
          initialize_readline();

          useHistory = true;
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index a20e0cd..16a272e 100644
*** a/src/include/pg_config.h.in
--- b/src/include/pg_config.h.in
***************
*** 428,433 ****
--- 428,436 ----
  /* Define to 1 if you have the `rl_filename_completion_function' function. */
  #undef HAVE_RL_FILENAME_COMPLETION_FUNCTION

+ /* Define to 1 if you have the `rl_reset_screen_size' function. */
+ #undef HAVE_RL_RESET_SCREEN_SIZE
+
  /* Define to 1 if you have the <security/pam_appl.h> header file. */
  #undef HAVE_SECURITY_PAM_APPL_H