Re: [PATCH 1/7] Move term_columns() to pager.c and save terminal width before pager

[Jonathan Nieder <jrnieder@gmail.com>]

Nguyễn Thái Ngọc Duy wrote:

> term_columns() checks for terminal width via ioctl(1).

ioctl(2). :)

I like the idea of checking width before launching a pager
and exporting it, yes.

> --- a/cache.h
> +++ b/cache.h
> @@ -1045,6 +1045,7 @@ extern void setup_pager(void);
>  extern const char *pager_program;
>  extern int pager_in_use(void);
>  extern int pager_use_color;
> +extern int term_columns();

Should say (void) rather than leaving the parameter list unspecified,
no?

It will only be needed by a few translation units, I hope.  Putting
it in column.h can avoid some pointless recompilation.

[...]
> +++ b/pager.c
> @@ -12,6 +12,7 @@
>   */
>  
>  static int spawned_pager;
> +static int max_columns;

Could be stored in the COLUMNS environment variable, but an integer
variable like this is is simpler.

[...]
> @@ -80,6 +81,15 @@ void setup_pager(void)
>  
>  	spawned_pager = 1; /* means we are emitting to terminal */
>  
> +#ifdef TIOCGWINSZ
> +	{
> +		struct winsize ws;
> +		if (!ioctl(1, TIOCGWINSZ, &ws)) {
> +			if (ws.ws_col)
> +				max_columns = ws.ws_col;
> +		}
> +	}
> +#endif

Mm, repeated code.

How about something like this on top?  Not sure if the
"if (ws.ws_col)" is useful.  I've left it alone.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |    1 +
 cache.h  |    1 -
 column.h |    6 ++++++
 help.c   |    1 +
 pager.c  |   48 ++++++++++++++++++++++++++----------------------
 5 files changed, 34 insertions(+), 23 deletions(-)
 create mode 100644 column.h

diff --git a/Makefile b/Makefile
index 775ee83..ed9e94b 100644
--- a/Makefile
+++ b/Makefile
@@ -1956,6 +1956,7 @@ builtin/prune.o builtin/reflog.o reachable.o: reachable.h
 builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
 builtin/tar-tree.o archive-tar.o: tar.h
 connect.o transport.o http-backend.o: url.h
+pager.o help.o: column.h
 http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
 http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h
 
diff --git a/cache.h b/cache.h
index bcbd5f2..d83d68c 100644
--- a/cache.h
+++ b/cache.h
@@ -1045,7 +1045,6 @@ extern void setup_pager(void);
 extern const char *pager_program;
 extern int pager_in_use(void);
 extern int pager_use_color;
-extern int term_columns();
 
 extern const char *editor_program;
 extern const char *askpass_program;
diff --git a/column.h b/column.h
new file mode 100644
index 0000000..55d8067
--- /dev/null
+++ b/column.h
@@ -0,0 +1,6 @@
+#ifndef COLUMN_H
+#define COLUMN_H
+
+extern int term_columns(void);
+
+#endif
diff --git a/help.c b/help.c
index 1344208..62a479b 100644
--- a/help.c
+++ b/help.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "builtin.h"
 #include "exec_cmd.h"
+#include "column.h"
 #include "levenshtein.h"
 #include "help.h"
 #include "common-cmds.h"
diff --git a/pager.c b/pager.c
index ad447cf..e6f7d86 100644
--- a/pager.c
+++ b/pager.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "run-command.h"
 #include "sigchain.h"
+#include "column.h"
 
 #ifndef DEFAULT_PAGER
 #define DEFAULT_PAGER "less"
@@ -14,6 +15,21 @@
 static int spawned_pager;
 static int max_columns;
 
+#ifdef TIOCGWINSZ
+static int retrieve_terminal_width(void)
+{
+	struct winsize ws;
+	if (ioctl(1, TIOCGWINSZ, &ws))	/* e.g., ENOSYS */
+		return 0;
+	return ws.ws_col;
+}
+#else
+static int retrieve_terminal_width(void)
+{
+	return 0;
+}
+#endif
+
 #ifndef WIN32
 static void pager_preexec(void)
 {
@@ -75,21 +91,17 @@ const char *git_pager(int stdout_is_tty)
 void setup_pager(void)
 {
 	const char *pager = git_pager(isatty(1));
+	int width;
 
 	if (!pager)
 		return;
 
 	spawned_pager = 1; /* means we are emitting to terminal */
 
-#ifdef TIOCGWINSZ
-	{
-		struct winsize ws;
-		if (!ioctl(1, TIOCGWINSZ, &ws)) {
-			if (ws.ws_col)
-				max_columns = ws.ws_col;
-		}
-	}
-#endif
+	width = retrieve_terminal_width();
+	if (width)
+		max_columns = width;
+
 	/* spawn the pager */
 	pager_argv[0] = pager;
 	pager_process.use_shell = 1;
@@ -127,24 +139,16 @@ int pager_in_use(void)
 	return env ? git_config_bool("GIT_PAGER_IN_USE", env) : 0;
 }
 
-int term_columns()
+int term_columns(void)
 {
 	char *col_string = getenv("COLUMNS");
 	int n_cols;
 
 	if (col_string && (n_cols = atoi(col_string)) > 0)
 		return n_cols;
-
-	else if (spawned_pager && max_columns)
+	if (spawned_pager && max_columns)
 		return max_columns;
-#ifdef TIOCGWINSZ
-	else {
-		struct winsize ws;
-		if (!ioctl(1, TIOCGWINSZ, &ws)) {
-			if (ws.ws_col)
-				return ws.ws_col;
-		}
-	}
-#endif
-	return 80;
+
+	n_cols = retrieve_terminal_width();
+	return n_cols ? n_cols : 80;
 }
-- 
1.7.4