ascii: Correct multiple bugs regarding output width.
authorBen Pfaff <blp@cs.stanford.edu>
Thu, 9 Jan 2020 07:22:05 +0000 (07:22 +0000)
committerBen Pfaff <blp@cs.stanford.edu>
Thu, 9 Jan 2020 07:25:48 +0000 (07:25 +0000)
The SET WIDTH setting was ignored.  This makes the code honor it for
output to a file.  For output to a terminal, the terminal's width is
used instead.

Honor changes to SET WIDTH as well.

The code to detect terminal size changes wasn't properly signal safe,
and it was scattered across multiple files.  This centralizes it and
makes it signal safe.

There were bugs in updates to min_hbreak in corner cases.  This fixes
them.

Thanks to Frans Houweling and John Darrington for reporting these
bugs.

doc/invoking.texi
src/output/ascii.c
src/ui/terminal/automake.mk
src/ui/terminal/main.c
src/ui/terminal/terminal-reader.c
src/ui/terminal/terminal.c [deleted file]
src/ui/terminal/terminal.h [deleted file]

index 88d4c1258a3d23e8b379b58c18e205fe321ebcc6..f64c92712f61825540b77645a7f400767c613b8c 100644 (file)
@@ -320,10 +320,10 @@ where @var{RRRR}, @var{GGGG} and @var{BBBB} are 4 character hexadecimal
 representations of the red, green and blue components respectively.
 If charts are disabled, this option has no effect.
 
-@item @option{-O width=@var{character-count}}
-Width of a page, in characters.  For screen output you may specify
-@code{auto} in place of a number to track the width of the terminal as
-it changes.  Default: @code{79}.
+@item @option{-O width=@var{columns}}
+Width of a page, in columns.  If unspecified or given as @code{auto},
+the default is the width of the terminal, for interactive output, or
+the WIDTH setting (@pxref{SET}), for output to a file.
 
 @item @option{-O box=@{ascii|unicode@}}
 Sets the characters used for lines in tables.
index abf564401fa361ed984ee5935f77c7a410d14003..242e75286f6710c21808fab33fc398e352ade2d0 100644 (file)
 #include <ctype.h>
 #include <errno.h>
 #include <limits.h>
+#include <signal.h>
 #include <stdint.h>
 #include <stdlib.h>
-#include <signal.h>
 #include <unilbrk.h>
 #include <unistd.h>
 #include <unistr.h>
 #include <uniwidth.h>
 
+#ifdef HAVE_TERMIOS_H
+# include <termios.h>
+#endif
+
+#ifdef GWINSZ_IN_SYS_IOCTL
+# include <sys/ioctl.h>
+#endif
+
 #include "data/file-name.h"
 #include "data/file-handle-def.h"
 #include "data/settings.h"
@@ -187,10 +195,15 @@ struct ascii_driver
     struct cell_color bg;
 #endif
 
+    /* How the page width is determined: */
+    enum {
+      FIXED_WIDTH,              /* Specified by configuration. */
+      VIEW_WIDTH,               /* From SET WIDTH. */
+      TERMINAL_WIDTH            /* From the terminal's width. */
+    } width_mode;
     int width;                  /* Page width. */
-    bool auto_width;            /* Use viewwidth as page width? */
 
-    int min_break[TABLE_N_AXES]; /* Min cell size to break across pages. */
+    int min_hbreak;             /* Min cell size to break across pages. */
 
     const ucs4_t *box;          /* Line & box drawing characters. */
 
@@ -209,6 +222,8 @@ static const struct output_driver_class ascii_driver_class;
 
 static void ascii_submit (struct output_driver *, const struct output_item *);
 
+static int get_terminal_width (void);
+
 static bool update_page_size (struct ascii_driver *, bool issue_error);
 static int parse_page_size (struct driver_option *);
 
@@ -224,12 +239,6 @@ static void ascii_draw_cell (void *, const struct table_cell *, int color_idx,
                              int spill[TABLE_N_AXES][2],
                              int clip[TABLE_N_AXES][2]);
 
-#if HAVE_DECL_SIGWINCH
-static struct ascii_driver *the_driver;
-
-static void winch_handler (int);
-#endif
-
 static struct ascii_driver *
 ascii_driver_cast (struct output_driver *driver)
 {
@@ -262,7 +271,6 @@ ascii_create (struct  file_handle *fh, enum settings_output_devices device_type,
               struct string_map *o)
 {
   enum { BOX_ASCII, BOX_UNICODE } box;
-  int min_break[TABLE_N_AXES];
   struct output_driver *d;
   struct ascii_driver *a;
 
@@ -275,20 +283,21 @@ ascii_create (struct  file_handle *fh, enum settings_output_devices device_type,
   a->chart_file_name = parse_chart_file_name (opt (d, o, "charts", fh_get_file_name (fh)));
   a->handle = fh;
 
-  min_break[H] = parse_int (opt (d, o, "min-hbreak", "-1"), -1, INT_MAX);
 
-  a->width = parse_page_size (opt (d, o, "width", "79"));
-  a->auto_width = a->width < 0;
-  a->min_break[H] = min_break[H] >= 0 ? min_break[H] : a->width / 2;
+  bool terminal = !strcmp (fh_get_file_name (fh), "-") && isatty (1);
+  a->width = parse_page_size (opt (d, o, "width", "-1"));
+  a->width_mode = (a->width > 0 ? FIXED_WIDTH
+                   : terminal ? TERMINAL_WIDTH
+                   : VIEW_WIDTH);
+  a->min_hbreak = parse_int (opt (d, o, "min-hbreak", "-1"), -1, INT_MAX);
+
 #ifdef HAVE_CAIRO
   parse_color (d, o, "background-color", "#FFFFFFFFFFFF", &a->bg);
   parse_color (d, o, "foreground-color", "#000000000000", &a->fg);
 #endif
 
-  const char *default_box = (!strcmp (fh_get_file_name (fh), "-")
-                             && isatty (STDOUT_FILENO)
-                             && (!strcmp (locale_charset (), "UTF-8")
-                                 || term_is_utf8_xterm ())
+  const char *default_box = (terminal && (!strcmp (locale_charset (), "UTF-8")
+                                          || term_is_utf8_xterm ())
                              ? "unicode" : "ascii");
   box = parse_enum (opt (d, o, "box", default_box),
                     "ascii", BOX_ASCII,
@@ -319,8 +328,6 @@ ascii_create (struct  file_handle *fh, enum settings_output_devices device_type,
       a->params.line_widths[H][i] = width;
       a->params.line_widths[V][i] = width;
     }
-  for (int i = 0; i < TABLE_N_AXES; i++)
-    a->params.min_break[i] = a->min_break[i];
   a->params.supports_margins = false;
   a->params.rtl = render_direction_rtl ();
 
@@ -335,19 +342,6 @@ ascii_create (struct  file_handle *fh, enum settings_output_devices device_type,
       goto error;
     }
 
-  if (isatty (fileno (a->file)))
-    {
-#if HAVE_DECL_SIGWINCH
-      struct sigaction action;
-      sigemptyset (&action.sa_mask);
-      action.sa_flags = 0;
-      action.sa_handler = winch_handler;
-      the_driver = a;
-      sigaction (SIGWINCH, &action, NULL);
-#endif
-      a->auto_width = true;
-    }
-
   return d;
 
 error:
@@ -389,28 +383,22 @@ parse_page_size (struct driver_option *option)
 static bool
 update_page_size (struct ascii_driver *a, bool issue_error)
 {
-  enum { MIN_WIDTH = 6, MIN_LENGTH = 6 };
-
-  if (a->auto_width)
-    {
-      a->params.size[H] = a->width = settings_get_viewwidth ();
-      a->params.min_break[H] = a->min_break[H] = a->width / 2;
-    }
-
-  if (a->width < MIN_WIDTH)
-    {
-      if (issue_error)
-        msg (ME,
-               _("ascii: page must be at least %d characters wide, but "
-                 "as configured is only %d characters"),
-               MIN_WIDTH,
-               a->width);
-      if (a->width < MIN_WIDTH)
-        a->params.size[H] = a->width = MIN_WIDTH;
-      return false;
-    }
+  enum { MIN_WIDTH = 6 };
+
+  int want_width = (a->width_mode == VIEW_WIDTH ? settings_get_viewwidth ()
+                    : a->width_mode == TERMINAL_WIDTH ? get_terminal_width ()
+                    : a->width);
+  bool ok = want_width >= MIN_WIDTH;
+  if (!ok && issue_error)
+    msg (ME, _("ascii: page must be at least %d characters wide, but "
+               "as configured is only %d characters"),
+         MIN_WIDTH, want_width);
+
+  a->width = ok ? want_width : MIN_WIDTH;
+  a->params.size[H] = a->width;
+  a->params.min_break[H] = a->min_hbreak >= 0 ? a->min_hbreak : a->width / 2;
 
-  return true;
+  return ok;
 }
 
 static void
@@ -996,10 +984,50 @@ ascii_test_flush (struct output_driver *driver)
       }
 }
 \f
+static sig_atomic_t terminal_changed = true;
+static int terminal_width;
+
 #if HAVE_DECL_SIGWINCH
 static void
 winch_handler (int signum UNUSED)
 {
-  update_page_size (the_driver, false);
+  terminal_changed = true;
 }
 #endif
+
+int
+get_terminal_width (void)
+{
+#if HAVE_DECL_SIGWINCH
+  static bool setup_signal;
+  if (!setup_signal)
+    {
+      setup_signal = true;
+
+      struct sigaction action = { .sa_handler = winch_handler };
+      sigemptyset (&action.sa_mask);
+      sigaction (SIGWINCH, &action, NULL);
+    }
+#endif
+
+  if (terminal_changed)
+    {
+      terminal_changed = false;
+
+#ifdef HAVE_TERMIOS_H
+      struct winsize ws;
+      if (!ioctl (0, TIOCGWINSZ, &ws))
+        terminal_width = ws.ws_col;
+      else
+#endif
+        {
+          if (getenv ("COLUMNS"))
+            terminal_width = atoi (getenv ("COLUMNS"));
+        }
+
+      if (terminal_width <= 0 || terminal_width > 1024)
+        terminal_width = 79;
+    }
+
+  return terminal_width;
+}
index 77c8a13ec94349b10506192d28c23287c18dadd2..5169110d2e79c7129a701137cfc8908f318bcce0 100644 (file)
@@ -23,9 +23,7 @@ src_ui_terminal_libui_la_SOURCES = \
        src/ui/terminal/terminal-opts.c \
        src/ui/terminal/terminal-opts.h \
        src/ui/terminal/terminal-reader.c \
-       src/ui/terminal/terminal-reader.h \
-       src/ui/terminal/terminal.c \
-       src/ui/terminal/terminal.h
+       src/ui/terminal/terminal-reader.h
 
 src_ui_terminal_libui_la_CFLAGS = $(NCURSES_CFLAGS)
 
index bbcc896958052b4c1a0b2c69042bfaed8629156b..a92d6c040ff4a3ff8cdef27d72b027d32a766a3c 100644 (file)
@@ -51,7 +51,6 @@
 #include "ui/source-init-opts.h"
 #include "ui/terminal/terminal-opts.h"
 #include "ui/terminal/terminal-reader.h"
-#include "ui/terminal/terminal.h"
 
 #include "gl/fatal-signal.h"
 #include "gl/progname.h"
@@ -92,7 +91,6 @@ main (int argc, char **argv)
   output_engine_push ();
   fh_init ();
   settings_init ();
-  terminal_check_size ();
   random_init ();
 
   lexer = lex_create ();
index d0660c66e185251fce1799109313a16225b260ad..2f2a533d1577f4d6cf46a95e3f60cd1b26ce7dd8 100644 (file)
@@ -56,7 +56,6 @@ static char *command_generator (const char *text, int state);
 #include "libpspp/version.h"
 #include "output/driver.h"
 #include "output/journal.h"
-#include "ui/terminal/terminal.h"
 
 #include "gl/minmax.h"
 #include "gl/xalloc.h"
@@ -158,15 +157,6 @@ terminal_reader_read (struct lex_reader *r_, char *buf, size_t n,
        }
       r->offset = 0;
       r->eof = ss_is_empty (r->s);
-
-      /* Check whether the size of the window has changed, so that
-         the output drivers can adjust their settings as needed.  We
-         only do this for the first line of a command, as it's
-         possible that the output drivers are actually in use
-         afterward, and we don't want to confuse them in the middle
-         of output. */
-      if (prompt_style == PROMPT_FIRST)
-        terminal_check_size ();
     }
 
   chunk = MIN (n, r->s.length - r->offset);
diff --git a/src/ui/terminal/terminal.c b/src/ui/terminal/terminal.c
deleted file mode 100644 (file)
index c8a605d..0000000
+++ /dev/null
@@ -1,73 +0,0 @@
-/* PSPP - a program for statistical analysis.
-   Copyright (C) 1997-9, 2000, 2006, 2007, 2010, 2017 Free Software Foundation, Inc.
-
-   This program is free software: you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation, either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>. */
-
-#include <config.h>
-
-#include "ui/terminal/terminal.h"
-
-#include <stdbool.h>
-#include <stdlib.h>
-
-#include "data/settings.h"
-#include "libpspp/compiler.h"
-
-#include "gl/error.h"
-
-#include "gettext.h"
-
-
-#ifdef HAVE_TERMIOS_H
-# include <termios.h>
-#endif
-
-#ifdef GWINSZ_IN_SYS_IOCTL
-# include <sys/ioctl.h>
-#endif
-
-#define _(msgid) gettext (msgid)
-
-
-/* Determines the size of the terminal, if possible, or at least
-   takes an educated guess. */
-void
-terminal_check_size (void)
-{
-  int view_width = 0;
-  int view_length = 0;
-
-#ifdef HAVE_TERMIOS_H
-  struct winsize ws;
-  if (0 == ioctl (0, TIOCGWINSZ, &ws))
-    {
-      view_width = ws.ws_col;
-      view_length = ws.ws_row;
-    }
-  else
-#endif
-    {
-      if (view_width <= 0 && getenv ("COLUMNS") != NULL)
-       view_width = atoi (getenv ("COLUMNS"));
-
-      if (view_length <= 0 && getenv ("LINES") != NULL)
-       view_length = atoi (getenv ("LINES"));
-    }
-
-  if (view_width > 0)
-    settings_set_viewwidth (view_width);
-
-  if (view_length > 0)
-    settings_set_viewlength (view_length);
-}
diff --git a/src/ui/terminal/terminal.h b/src/ui/terminal/terminal.h
deleted file mode 100644 (file)
index 1260e93..0000000
+++ /dev/null
@@ -1,23 +0,0 @@
-/* PSPP - a program for statistical analysis.
-   Copyright (C) 2007 Free Software Foundation, Inc.
-
-   This program is free software: you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation, either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>. */
-
-#ifndef UI_TERMINAL_TERMINAL_H
-#define UI_TERMINAL_TERMINAL_H 1
-
-void terminal_init (int **view_width_p, int **view_length_p);
-void terminal_check_size (void);
-
-#endif /* ui/terminal/terminal.h */