git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 13/13] move LESS/LV pager environment to Makefile

From
Jeff King <peff@peff.net>
Date
Feb 5, 2014, 18:08 UTC
Message-ID
<20140205180857.GM15218@sigill.intra.peff.net>
In-Reply-To
<20140205174823.GA15070@sigill.intra.peff.net>

We set the LESS and LV variables to sensible defaults if they are not already set. However, the code is brittle. There is no easy way to change the defaults at compile time, and we have to duplicate the code in git-sh-setup and in pager.c.

Let's turn it into a normal Makefile knob instead.
Signed-off-by: Jeff King <peff@peff.net>
---
Bits of this patch were liberally stolen from Junio's weather-balloon
patch. All bugs are mine. :)

Note that setup_pager_env in the C code still does do a little parsing, since we need to have the name of each variable in a separate buffer to pass to getenv(). I chose to do it this way so that we could introduce a standard "mkcarray" helper that could be used in other places. But it would also be easy to do all of that parsing at compile time and produce an array of structs like:

  { "LESS", "LESS=-FRSX" },
  { "LV", "LV=-c" }
 Makefile        | 22 +++++++++++++++++++++-
 git-sh-setup.sh |  9 +++++----
 pager.c         | 34 +++++++++++++++++++++++-----------
 script/mkcarray | 25 +++++++++++++++++++++++++
 4 files changed, 74 insertions(+), 16 deletions(-)
 create mode 100644 script/mkcarray
diff --git a/Makefile b/Makefile
index ad3100d..fff8e72 100644
--- a/Makefile
+++ b/Makefile
@@ -342,6 +342,14 @@ all::
 # Define DEFAULT_HELP_FORMAT to "man", "info" or "html"
 # (defaults to "man") if you want to have a different default when
 # "git help" is called without a parameter specifying the format.
+#
+# Define PAGER_ENV to a SP separated VAR=VAL pairs to define
+# default environment variables to be passed when a pager is spawned, e.g.
+#
+#    PAGER_ENV = LESS=-FRSX LV=-c
+#
+# to say "export LESS=-FRSX (and LV=-c) if the environment variable
+# LESS (and LV) is not set, respectively".
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1505,6 +1513,10 @@ ifeq ($(PYTHON_PATH),)
 NO_PYTHON = NoThanks
 endif
 
+ifndef PAGER_ENV
+PAGER_ENV = LESS=-FRSX LV=-c
+endif
+
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -1598,6 +1610,11 @@ MAKE/%-string.h: MAKE/% script/mkcstring
 		$(subst -,_,$*) <$< >$@+ && \
 		mv $@+ $@
 
+MAKE/%-array.h: MAKE/% script/mkcarray
+	$(QUIET_GEN)$(SHELL_PATH) script/mkcarray \
+		$(subst -,_,$*) <$< >$@+ && \
+		mv $@+ $@
+
 MAKE/%.sh: MAKE/% script/mksh
 	$(QUIET_GEN)$(SHELL_PATH) script/mksh \
 		$(subst -,_,$*) <$< >$@+ && \
@@ -1726,6 +1743,9 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 
 version.sp version.s version.o: MAKE/VERSION-string.h MAKE/USER-AGENT-string.h
 
+$(eval $(call make-var,PAGER-ENV,pager environment,$(PAGER_ENV)))
+pager.sp pager.s pager.o: MAKE/PAGER-ENV-array.h
+
 $(BUILT_INS): git$X
 	$(QUIET_BUILT_IN)$(RM) $@ && \
 	ln $< $@ 2>/dev/null || \
@@ -1776,7 +1796,7 @@ $(SCRIPT_LIB) : % : %.sh MAKE/SCRIPT-DEFINES
 	$(QUIET_GEN)$(cmd_munge_script) && \
 	mv $@+ $@
 
-git-sh-setup: MAKE/DIFF.sh
+git-sh-setup: MAKE/DIFF.sh MAKE/PAGER-ENV.sh
 
 git.res: git.rc GIT-VERSION-FILE
 	$(QUIET_RC)$(RC) \
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 627d289..be4a58a 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -158,10 +158,11 @@ git_pager() {
 	else
 		GIT_PAGER=cat
 	fi
-	: ${LESS=-FRSX}
-	: ${LV=-c}
-	export LESS LV
-
+	for vardef in $MAKE_PAGER_ENV
+	do
+		var=${vardef%%=*}
+		eval ": \${$vardef} && export $var"
+	done
 	eval "$GIT_PAGER" '"$@"'
 }
 
diff --git a/pager.c b/pager.c
index 0cc75a8..6db84c6 100644
--- a/pager.c
+++ b/pager.c
@@ -1,6 +1,8 @@
 #include "cache.h"
 #include "run-command.h"
 #include "sigchain.h"
+#include "argv-array.h"
+#include "MAKE/PAGER-ENV-array.h"
 
 #ifndef DEFAULT_PAGER
 #define DEFAULT_PAGER "less"
@@ -60,9 +62,26 @@ const char *git_pager(int stdout_is_tty)
 	return pager;
 }
 
+static void setup_pager_env(struct argv_array *env)
+{
+	int i;
+
+	for (i = 0; MAKE_PAGER_ENV[i]; i++) {
+		struct strbuf buf = STRBUF_INIT;
+		const char *p = MAKE_PAGER_ENV[i];
+		const char *eq = strchrnul(p, '=');
+
+		strbuf_add(&buf, p, eq - p);
+		if (!getenv(buf.buf))
+			argv_array_push(env, p);
+		strbuf_release(&buf);
+	}
+}
+
 void setup_pager(void)
 {
 	const char *pager = git_pager(isatty(1));
+	struct argv_array env = ARGV_ARRAY_INIT;
 
 	if (!pager || pager_in_use())
 		return;
@@ -80,17 +99,10 @@ void setup_pager(void)
 	pager_process.use_shell = 1;
 	pager_process.argv = pager_argv;
 	pager_process.in = -1;
-	if (!getenv("LESS") || !getenv("LV")) {
-		static const char *env[3];
-		int i = 0;
-
-		if (!getenv("LESS"))
-			env[i++] = "LESS=FRSX";
-		if (!getenv("LV"))
-			env[i++] = "LV=-c";
-		env[i] = NULL;
-		pager_process.env = env;
-	}
+
+	setup_pager_env(&env);
+	pager_process.env = argv_array_detach(&env, NULL);
+
 	if (start_command(&pager_process))
 		return;
 
diff --git a/script/mkcarray b/script/mkcarray
new file mode 100644
index 0000000..ed980ab
--- /dev/null
+++ b/script/mkcarray
@@ -0,0 +1,25 @@
+#!/bin/sh
+
+name=$1; shift
+
+quote() {
+	echo "$1" | sed 's/\\/\\\\/g; s/"/\\"/'
+}
+
+cat <<-EOF
+#ifndef ${name}_H
+#define ${name}_H
+
+static const char *MAKE_${name}[] = {
+EOF
+
+for i in $(cat); do
+	printf '\t"%s",\n' "$(quote "$i")"
+done
+
+cat <<EOF
+	NULL
+};
+
+#endif /* ${name}_H */
+EOF

-- 
1.8.5.2.500.g8060133
Previous: Jeff KingNext: Jeff King
Message 26 of 28 in “makefile refactoring”
  1. 0/13 makefile refactoringJeff King, Feb 5, 2014
  2. 01/13 Makefile: drop USE_GETTEXT_SCHEME from GIT-CFLAGSJeff King, Feb 5, 2014
  3. 02/13 Makefile: fix git-instaweb dependency on gitwebJeff King, Feb 5, 2014
  4. 03/13 Makefile: introduce make-var helper functionJeff King, Feb 5, 2014
  5. Eric SunshineFeb 6, 2014
  6. 04/13 Makefile: use tempfile/mv strategy for GIT-*Jeff King, Feb 5, 2014
  7. 05/13 Makefile: prefer printf to echo for GIT-*Jeff King, Feb 5, 2014
  8. 06/13 Makefile: store GIT-* sentinel files in MAKE/Jeff King, Feb 5, 2014
  9. Junio C HamanoFeb 5, 2014
  10. 07/13 Makefile: always create files via make-varJeff King, Feb 5, 2014
  11. 08/13 Makefile: introduce sq function for shell-quotingJeff King, Feb 5, 2014
  12. Junio C HamanoFeb 5, 2014
  13. 09/13 Makefile: add c-quote helper functionJeff King, Feb 5, 2014
  14. Junio C HamanoFeb 5, 2014
  15. Jeff KingFeb 5, 2014
  16. 10/13 Makefile: drop *_SQ variablesJeff King, Feb 5, 2014
  17. Junio C HamanoFeb 5, 2014
  18. 11/13 Makefile: auto-build C strings from make variablesJeff King, Feb 5, 2014
  19. Junio C HamanoFeb 5, 2014
  20. Jeff KingFeb 5, 2014
  21. 12/13 Makefile: teach scripts to include make variablesJeff King, Feb 5, 2014
  22. Junio C HamanoFeb 5, 2014
  23. Jeff KingFeb 5, 2014
  24. Thomas RastFeb 8, 2014
  25. Jeff KingFeb 10, 2014
  26. 13/13 move LESS/LV pager environment to MakefileJeff King, Feb 5, 2014
  27. Jeff KingFeb 5, 2014
  28. Jeff KingFeb 5, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.