From: Ben Pfaff Date: Thu, 9 Jan 2020 07:22:05 +0000 (+0000) Subject: ascii: Correct multiple bugs regarding output width. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1e8aa549d8b7b19f814bbb08995aedb6a96630f6;p=pspp ascii: Correct multiple bugs regarding output width. 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. --- diff --git a/doc/invoking.texi b/doc/invoking.texi index 88d4c1258a..f64c92712f 100644 --- a/doc/invoking.texi +++ b/doc/invoking.texi @@ -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. diff --git a/src/output/ascii.c b/src/output/ascii.c index abf564401f..242e75286f 100644 --- a/src/output/ascii.c +++ b/src/output/ascii.c @@ -19,14 +19,22 @@ #include #include #include +#include #include #include -#include #include #include #include #include +#ifdef HAVE_TERMIOS_H +# include +#endif + +#ifdef GWINSZ_IN_SYS_IOCTL +# include +#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) } } +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; +} diff --git a/src/ui/terminal/automake.mk b/src/ui/terminal/automake.mk index 77c8a13ec9..5169110d2e 100644 --- a/src/ui/terminal/automake.mk +++ b/src/ui/terminal/automake.mk @@ -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) diff --git a/src/ui/terminal/main.c b/src/ui/terminal/main.c index bbcc896958..a92d6c040f 100644 --- a/src/ui/terminal/main.c +++ b/src/ui/terminal/main.c @@ -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 (); diff --git a/src/ui/terminal/terminal-reader.c b/src/ui/terminal/terminal-reader.c index d0660c66e1..2f2a533d15 100644 --- a/src/ui/terminal/terminal-reader.c +++ b/src/ui/terminal/terminal-reader.c @@ -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 index c8a605d8f2..0000000000 --- a/src/ui/terminal/terminal.c +++ /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 . */ - -#include - -#include "ui/terminal/terminal.h" - -#include -#include - -#include "data/settings.h" -#include "libpspp/compiler.h" - -#include "gl/error.h" - -#include "gettext.h" - - -#ifdef HAVE_TERMIOS_H -# include -#endif - -#ifdef GWINSZ_IN_SYS_IOCTL -# include -#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 index 1260e9321c..0000000000 --- a/src/ui/terminal/terminal.h +++ /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 . */ - -#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 */