From 2f2836b21d21aa1f8e0a828b4f925338f0af1123 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Thu, 9 May 2024 13:16:39 -0700
Subject: [PATCH] config: set `$LESS` and `$LESSCHARSET` even if `$PAGER` is
 set

Our current default for `ui.pager` is this:

```toml
ui.pager = { command = ["less", "-FRX"], env = { LESSCHARSET = "utf-8" } }
```

If the user has `$PAGER` set, we take that value and replace the above
table by a scalar set to the value from the environment variable. That
means that anyone who has set `$PAGER` to just `less` will lose both
the `-FRX` and the charset, making e.g. colored output from `jj`
result in escaped ANSI codes rendered by `less`. The lack of those
options might not matter for other tools they use so they might not
have realized that they wanted those options.

This patch attempts to improve the situation by changing the default
`ui.pager` to pass `-FRX` via `$LESS`, and by setting the value from
`$PAGER` in `ui.pager.command`, so the rest of the config is left
alone.

The default config will still be ignored if you set the scalar
`ui.pager` to e.g. `less`, since that overrides the table.

Closes #2926
---
 cli/src/config.rs        | 23 ++++++++++++++++++++++-
 cli/src/config/misc.toml |  2 +-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/cli/src/config.rs b/cli/src/config.rs
index 453d7dabed..e574df9b89 100644
--- a/cli/src/config.rs
+++ b/cli/src/config.rs
@@ -324,7 +324,28 @@ fn env_base() -> config::Config {
         builder = builder.set_override("ui.color", "never").unwrap();
     }
     if let Ok(value) = env::var("PAGER") {
-        builder = builder.set_override("ui.pager", value).unwrap();
+        builder = builder
+            .set_override(
+                "ui.pager.command",
+                config::Value::new(
+                    None,
+                    config::ValueKind::Array(vec![config::Value::new(
+                        None,
+                        config::ValueKind::String(value),
+                    )]),
+                ),
+            )
+            .unwrap();
+    }
+    // Override $LESS and $LESSCHARSET in `ui.pager.env` if they're set in the
+    // environment
+    if let Ok(value) = env::var("LESS") {
+        builder = builder.set_override("ui.pager.env.LESS", value).unwrap();
+    }
+    if let Ok(value) = env::var("LESSCHARSET") {
+        builder = builder
+            .set_override("ui.pager.env.LESSCHARSET", value)
+            .unwrap();
     }
     if let Ok(value) = env::var("VISUAL") {
         builder = builder.set_override("ui.editor", value).unwrap();
diff --git a/cli/src/config/misc.toml b/cli/src/config/misc.toml
index dc279216ac..480d19e5c0 100644
--- a/cli/src/config/misc.toml
+++ b/cli/src/config/misc.toml
@@ -13,7 +13,7 @@ allow-filesets = false
 always-allow-large-revsets = false
 diff-instructions = true
 paginate = "auto"
-pager = { command = ["less", "-FRX"], env = { LESSCHARSET = "utf-8" } }
+pager = { command = ["less"], env = { LESS = "-FRX", LESSCHARSET = "utf-8" } }
 log-word-wrap = false
 log-synthetic-elided-nodes = true