Merge pull request #1971 from hauleth/fix/append-to-manpath-instead-of-overriding

fix: instead of overriding MANPATHs set by OS, append to them

src/os/env.zig

@@ -20,6 +20,19 @@ pub fn appendEnv(
     if (current.len == 0) return try alloc.dupe(u8, value);
 
     // Otherwise we must prefix.
+    return try appendEnvAlways(alloc, current, value);
+}
+
+/// Always append value to environment, even when it is empty.
+/// This is useful because some env vars (like MANPATH) want there
+/// to be an empty prefix to preserve existing values.
+///
+/// The returned value is always allocated so it must be freed.
+pub fn appendEnvAlways(
+    alloc: Allocator,
+    current: []const u8,
+    value: []const u8,
+) ![]u8 {
     return try std.fmt.allocPrint(alloc, "{s}{s}{s}", .{
         current,
         PATH_SEP,

src/termio/Exec.zig

@@ -648,17 +648,17 @@ const Subprocess = struct {
                     break :man;
                 };
 
-                if (env.get("MANPATH")) |manpath| {
+                // Always append with colon in front, as it mean that if
-                    // Append to the existing MANPATH. It's very unlikely that our bundle's
+                // `MANPATH` is empty, then it should be treated as an extra
-                    // resources directory already appears here so we don't spend the time
+                // path instead of overriding all paths set by OS.
-                    // searching for it.
+                try env.put(
-                    try env.put(
+                    "MANPATH",
-                        "MANPATH",
+                    try internal_os.appendEnvAlways(
-                        try internal_os.appendEnv(alloc, manpath, dir),
+                        alloc,
-                    );
+                        env.get("MATHPATH") orelse "",
-                } else {
+                        dir,
-                    try env.put("MANPATH", dir);
+                    ),
-                }
+                );
             }
         }