pager: address review feedback for less --header support

- Remove static globals; store header_lines/header_width in
  struct child_process instead
- Invert pager_open/pager_open_header: pager_open() now calls
  pager_open_header(0, 0) as a thin wrapper
- Append --header to existing LESS value when set, instead of
  silently skipping header setup
- Flatten nested if-blocks into single setenv() call
- Use int consistently instead of mixing int and size_t
- Document that header_lines=0 means no header regardless of width

diff --git a/include/pager.h b/include/pager.h
index 950f97836e6121c45f0e842f7e94022a0eaae261..8a2edb3ae994cd2389f4351389f65e878fedfcae 100644 (file)
--- a/include/pager.h
+++ b/include/pager.h
@@ -7,10 +7,8 @@
 #ifndef UTIL_LINUX_PAGER
 #define UTIL_LINUX_PAGER
 
-#include <stddef.h>
-
 void pager_open(void);
-void pager_open_header(int header_lines, size_t first_col_width);
+void pager_open_header(int header_lines, int first_col_width);
 void pager_close(void);
 
 #endif

diff --git a/lib/pager.c b/lib/pager.c
index 12defcab1f5ec88d2a2a9038f6694e4100922a95..355defaf79681275eb553539052e03c0ed1654f8 100644 (file)
--- a/lib/pager.c
+++ b/lib/pager.c
@@ -33,6 +33,9 @@ struct child_process {
        pid_t pid;
        int in;
 
+       int header_lines;
+       int header_width;
+
        int org_err;
        int org_out;
        struct sigaction orig_sigchld;
@@ -53,33 +56,31 @@ static inline void close_pair(int fd[2])
        close(fd[1]);
 }
 
-static int pager_header_lines;
-static size_t pager_header_width;
-
 static void pager_preexec(void)
 {
-       if (getenv("LESS") == NULL) {
-               if (pager_header_lines > 0) {
-                       char less_env[256];
-
-                       if (pager_header_width > 0)
-                               snprintf(less_env, sizeof(less_env),
-                                        "FRSX --header %d,%zu",
-                                        pager_header_lines,
-                                        pager_header_width);
-                       else
-                               snprintf(less_env, sizeof(less_env),
-                                        "FRSX --header %d",
-                                        pager_header_lines);
-
-                       if (setenv("LESS", less_env, 0) != 0)
-                               warn(_("failed to set the %s environment variable"), "LESS");
-               } else {
-                       if (setenv("LESS", "FRSX", 0) != 0)
-                               warn(_("failed to set the %s environment variable"), "LESS");
-               }
-       }
-
+       const char *less_env = getenv("LESS");
+       char less_val[256];
+       int header_lines = pager_process.header_lines;
+       int header_width = pager_process.header_width;
+
+       if (header_lines > 0 && header_width > 0)
+               snprintf(less_val, sizeof(less_val),
+                        "%s --header %d,%d",
+                        less_env ? less_env : "FRSX",
+                        header_lines, header_width);
+       else if (header_lines > 0)
+               snprintf(less_val, sizeof(less_val),
+                        "%s --header %d",
+                        less_env ? less_env : "FRSX",
+                        header_lines);
+       else if (less_env == NULL)
+               snprintf(less_val, sizeof(less_val), "FRSX");
+       else
+               goto skip_less;
+
+       if (setenv("LESS", less_val, 1) != 0)
+               warn(_("failed to set the %s environment variable"), "LESS");
+skip_less:
        if (getenv("LV") == NULL && setenv("LV", "-c", 0) != 0)
                warn(_("failed to set the %s environment variable"), "LV");
 }
@@ -259,10 +260,26 @@ static void __setup_pager(void)
 /* Setup pager and redirect output, the pager may be closed by pager_close().
  */
 void pager_open(void)
+{
+       pager_open_header(0, 0);
+}
+
+/* Setup pager with "less --header" support to freeze header lines and
+ * optionally freeze the first column. The @header_lines specifies the
+ * number of header lines to freeze (typically 1 for table header).
+ * The @first_col_width specifies the number of character columns to
+ * freeze (width of first column including separator), or 0 to not
+ * freeze any column. If @header_lines is 0, no header freezing takes
+ * place regardless of @first_col_width.
+ */
+void pager_open_header(int header_lines, int first_col_width)
 {
        if (pager_process.pid)
                return;         /* already running */
 
+       pager_process.header_lines = header_lines;
+       pager_process.header_width = first_col_width;
+
        pager_process.org_out = dup(STDOUT_FILENO);
        pager_process.org_err = dup(STDERR_FILENO);
 
@@ -278,22 +295,6 @@ void pager_open(void)
        }
 }
 
-/* Setup pager with "less --header" support to freeze header lines and
- * optionally freeze the first column. The @header_lines specifies the
- * number of header lines to freeze (typically 1 for table header).
- * The @first_col_width specifies the number of character columns to
- * freeze (width of first column including separator), or 0 to not
- * freeze any column.
- */
-void pager_open_header(int header_lines, size_t first_col_width)
-{
-       pager_header_lines = header_lines;
-       pager_header_width = first_col_width;
-       pager_open();
-       pager_header_lines = 0;
-       pager_header_width = 0;
-}
-
 /* Close pager and restore original std{out,err}.
  */
 void pager_close(void)