lib: Fix behavior of PAGER environment var (#24)

lib/pager.sh

@@ -20,13 +20,13 @@ is_pager_disabled() {
 
 # Prints the detected pager name.
 pager_name() {
-	_detect_pager
+	_detect_pager 1>&2
 	echo "$_SCRIPT_PAGER_NAME"
 }
 
 # Prints the detected pager version.
 pager_version() {
-	_detect_pager
+	_detect_pager 1>&2
 	echo "$_SCRIPT_PAGER_VERSION"
 }
 
@@ -37,13 +37,7 @@ pager_version() {
 #     pager_exec echo hi
 pager_exec() {
 	if [[ -n "$1" ]]; then
-		if [[ -n "$SCRIPT_PAGER_CMD" ]]; then
+		"$@" | "${SCRIPT_PAGER_CMD[@]}"
-			"$@" | "${SCRIPT_PAGER_CMD[@]}" "${SCRIPT_PAGER_ARGS[@]}"
-			return $?
-		else
-			"$@"
-			return $?
-		fi
 	fi
 }
 
@@ -64,7 +58,7 @@ pager_display() {
 # Detect the pager information.
 # shellcheck disable=SC2120
 _detect_pager() {
-	if [[ "$_SCRIPT_PAGER_DETECTED" = "true" && "$1" != "--force" ]]; then return; fi
+	if [[ "$_SCRIPT_PAGER_DETECTED" = "true" ]]; then return; fi
 	_SCRIPT_PAGER_DETECTED=true
 
 	# If the pager command is empty, the pager is disabled.
@@ -97,7 +91,8 @@ _detect_pager() {
 # 2. Use PAGER with special arguments for less
 # 3. Use PAGER
 _configure_pager() {
-	SCRIPT_PAGER_CMD=("$PAGER")
+	# shellcheck disable=SC2206
+	SCRIPT_PAGER_CMD=($PAGER)
 	SCRIPT_PAGER_ARGS=()
 
 	# Prefer the bat pager.
@@ -111,9 +106,9 @@ _configure_pager() {
 
 	# Add arguments for the less pager.
 	if is_pager_less; then
-		SCRIPT_PAGER_ARGS=(-R --quit-if-one-screen)
+		SCRIPT_PAGER_CMD=("${SCRIPT_PAGER_CMD[0]}" -R --quit-if-one-screen)
 		if [[ "$(pager_version)" -lt 500 ]]; then
-			SCRIPT_PAGER_ARGS+=(--no-init)
+			SCRIPT_PAGER_CMD+=(--no-init)
 		fi
 	fi
 }

test/suite/lib_pager.sh

@@ -6,69 +6,95 @@ use_pager() {
 	unset BAT_PAGER
 	export PAGER="$1"
 
-	SCRIPT_PAGER_CMD=("$PAGER")
-	SCRIPT_PAGER_ARGS=()
-
-	_detect_pager --force
 	_configure_pager
+	_detect_pager
 }
 
 use_bat_pager() {
 	unset PAGER
 	export BAT_PAGER="$1"
 
-	SCRIPT_PAGER_CMD=($BAT_PAGER)
-	SCRIPT_PAGER_ARGS=()
-
-	_detect_pager --force
 	_configure_pager
+	_detect_pager
 }
 
 test:less_detection() {
 	description "Identify less"
 
-	use_pager "less"             && expect_equal "$(pager_name)" "less"
+	(use_pager "less"             && expect_equal "$(pager_name)" "less")
-	use_pager "less_but_renamed" && expect_equal "$(pager_name)" "less"
+	(use_pager "less_but_renamed" && expect_equal "$(pager_name)" "less")
-	use_pager "tput"             && expect_equal "$(pager_name)" "tput"
+	(use_pager "tput"             && expect_equal "$(pager_name)" "tput")
 }
 
 test:less_version() {
 	description "Identify less version"
 
-	export MOCK_LESS_VERSION=473
+	(
-	use_pager "less" && expect_equal "$(pager_version)" "473"
+		export MOCK_LESS_VERSION=473
+		use_pager "less" && expect_equal "$(pager_version)" "473"
+	)
 
-	export MOCK_LESS_VERSION=551
+	(
-	use_pager "less" && expect_equal "$(pager_version)" "551"
+		export MOCK_LESS_VERSION=551
+		use_pager "less" && expect_equal "$(pager_version)" "551"
+	)
 }
 
 test:less_args() {
 	description "Automatically select appropriate less args"
 
-	export MOCK_LESS_VERSION=473
+	# When pager is "less".
-	use_pager "less"
+	(
-	expect array_contains "-R" in "${SCRIPT_PAGER_ARGS[@]}"
+		export MOCK_LESS_VERSION=473
-	expect array_contains "--quit-if-one-screen" in "${SCRIPT_PAGER_ARGS[@]}"
+		use_pager "less"
-	expect array_contains "--no-init" in "${SCRIPT_PAGER_ARGS[@]}"
+		expect array_contains "-R" in "${SCRIPT_PAGER_CMD[@]}"
+		expect array_contains "--quit-if-one-screen" in "${SCRIPT_PAGER_CMD[@]}"
+		expect array_contains "--no-init" in "${SCRIPT_PAGER_CMD[@]}"
+	)
 
-	export MOCK_LESS_VERSION=551
+	# When pager is equivalent of "less".
-	use_pager "less"
+	(
-	expect array_contains "-R" in "${SCRIPT_PAGER_ARGS[@]}"
+		export MOCK_LESS_VERSION=551
-	expect array_contains "--quit-if-one-screen" in "${SCRIPT_PAGER_ARGS[@]}"
+		use_pager "less"
-	expect ! array_contains "--no-init" in "${SCRIPT_PAGER_ARGS[@]}"
+		expect array_contains "-R" in "${SCRIPT_PAGER_CMD[@]}"
+		expect array_contains "--quit-if-one-screen" in "${SCRIPT_PAGER_CMD[@]}"
+		expect ! array_contains "--no-init" in "${SCRIPT_PAGER_CMD[@]}"
+	)
+}
+
+test:less_args_not_less() {
+	description "Don't give non-less pagers the less args"
 
 	use_pager "not_less"
-	expect_equal "${#SCRIPT_PAGER_ARGS[@]}" 0
+	echo "${SCRIPT_PAGER_CMD[@]}"
+	expect_equal "${#SCRIPT_PAGER_CMD[@]}" 1
 }
 
 test:env_bat_pager() {
 	description "Check that BAT_PAGER is being used"
 
 	use_bat_pager "not_less"
-	expect_equal "$SCRIPT_PAGER_CMD" "not_less"
+	expect_equal "${SCRIPT_PAGER_CMD[0]}" "not_less"
 
 	use_bat_pager "not_less but not_more"
-	expect_equal "$SCRIPT_PAGER_CMD" "not_less"
+	expect_equal "${SCRIPT_PAGER_CMD[0]}" "not_less"
 	expect_equal "${SCRIPT_PAGER_CMD[1]}" "but"
 	expect_equal "${SCRIPT_PAGER_CMD[2]}" "not_more"
 }
+
+test:args_copied_from_pager() {
+	description "Check that the pager args are correct with PAGER."
+
+	use_pager "less --some-argument"
+	expect_equal "${SCRIPT_PAGER_CMD[0]}" "less"
+	expect_not_equal "${SCRIPT_PAGER_CMD[1]}" "--some-argument"
+}
+
+test:args_copied_from_bat_pager() {
+	description "Check that the pager args are correct with PAGER."
+
+	use_bat_pager "less --some-argument"
+	assert_equal "${#SCRIPT_PAGER_CMD[@]}" 2
+	expect_equal "${SCRIPT_PAGER_CMD[0]}" "less"
+	expect_equal "${SCRIPT_PAGER_CMD[1]}" "--some-argument"
+}