cli: if $JJ_CONFIG points to a directory, read all files in it

It's annoying especially for tests to not be able to append to a
config file without knowing the contents (as you have to do with
TOML). Let's read all files in a directory if `$JJ_CONFIG` points to a
directory. Mercurial does that for its `$HGRCPATH` variable.

CHANGELOG.md

@@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   mode). For example, use `jj move --to @-- foo` to move the changes to file
   (or directory) `foo` in the working copy to the grandparent commit.
 
+* The `$JJ_CONFIG` environment variable can now point to a directory. If it
+  does, all files in the directory will be read, in alphabetical order.
+
 ### Fixed bugs
 
 * Errors are now printed to stderr (they used to be printed to stdout).

src/main.rs

@@ -21,8 +21,12 @@ use jujutsu_lib::settings::UserSettings;
 
 fn config_path() -> Option<PathBuf> {
     if let Ok(config_path) = env::var("JJ_CONFIG") {
+        // TODO: We should probably support colon-separated (std::env::split_paths)
+        // paths here
         Some(PathBuf::from(config_path))
     } else {
+        // TODO: Should we drop the final `/config.toml` and read all files in the
+        // directory?
         dirs::config_dir().map(|config_dir| config_dir.join("jj").join("config.toml"))
     }
 }
@@ -31,11 +35,29 @@ fn read_config() -> Result<UserSettings, config::ConfigError> {
     let mut config_builder = config::Config::builder();
 
     if let Some(config_path) = config_path() {
-        config_builder = config_builder.add_source(
-            config::File::from(config_path)
-                .required(false)
-                .format(config::FileFormat::Toml),
-        );
+        let mut files = vec![];
+        if config_path.is_dir() {
+            if let Ok(read_dir) = config_path.read_dir() {
+                // TODO: Walk the directory recursively?
+                for dir_entry in read_dir.flatten() {
+                    let path = dir_entry.path();
+                    if path.is_file() {
+                        files.push(path);
+                    }
+                }
+            }
+            files.sort();
+        } else {
+            files.push(config_path);
+        }
+        for file in files {
+            // TODO: Accept other formats and/or accept only certain file extensions?
+            config_builder = config_builder.add_source(
+                config::File::from(file)
+                    .required(false)
+                    .format(config::FileFormat::Toml),
+            );
+        }
     };
 
     // TODO: Make the config from environment a separate source instead? Seems

tests/common/mod.rs

@@ -14,7 +14,6 @@
 
 use std::cell::RefCell;
 use std::collections::HashMap;
-use std::io::Write;
 use std::path::{Path, PathBuf};
 
 use tempfile::TempDir;
@@ -23,8 +22,9 @@ pub struct TestEnvironment {
     _temp_dir: TempDir,
     env_root: PathBuf,
     home_dir: PathBuf,
-    config_path: PathBuf,
+    config_dir: PathBuf,
     env_vars: HashMap<String, String>,
+    config_file_number: RefCell<i64>,
     command_number: RefCell<i64>,
 }
 
@@ -34,15 +34,16 @@ impl Default for TestEnvironment {
         let env_root = tmp_dir.path().canonicalize().unwrap();
         let home_dir = env_root.join("home");
         std::fs::create_dir(&home_dir).unwrap();
-        let config_path = env_root.join("config.toml");
-        std::fs::write(&config_path, b"").unwrap();
+        let config_dir = env_root.join("config");
+        std::fs::create_dir(&config_dir).unwrap();
         let env_vars = HashMap::new();
         Self {
             _temp_dir: tmp_dir,
             env_root,
             home_dir,
-            config_path,
+            config_dir,
             env_vars,
+            config_file_number: RefCell::new(0),
             command_number: RefCell::new(0),
         }
     }
@@ -62,7 +63,7 @@ impl TestEnvironment {
         let timestamp = chrono::DateTime::parse_from_rfc3339("2001-02-03T04:05:06+07:00").unwrap();
         let mut command_number = self.command_number.borrow_mut();
         *command_number += 1;
-        cmd.env("JJ_CONFIG", self.config_path.to_str().unwrap());
+        cmd.env("JJ_CONFIG", self.config_dir.to_str().unwrap());
         let timestamp = timestamp + chrono::Duration::seconds(*command_number);
         cmd.env("JJ_TIMESTAMP", timestamp.to_rfc3339());
         cmd.env("JJ_USER", "Test User");
@@ -90,17 +91,18 @@ impl TestEnvironment {
         &self.home_dir
     }
 
-    pub fn config_path(&self) -> &Path {
-        &self.config_path
-    }
-
-    pub fn write_config(&self, content: &[u8]) {
-        let mut config_file = std::fs::File::options()
-            .append(true)
-            .open(&self.config_path)
-            .unwrap();
-        config_file.write_all(content).unwrap();
-        config_file.flush().unwrap();
+    pub fn add_config(&self, content: &[u8]) {
+        // Concatenating two valid TOML files does not (generally) result in a valid
+        // TOML file, so we use create a new file every time instead.
+        let mut config_file_number = self.config_file_number.borrow_mut();
+        *config_file_number += 1;
+        let config_file_number = *config_file_number;
+        std::fs::write(
+            self.config_dir
+                .join(format!("config{config_file_number:04}.toml")),
+            content,
+        )
+        .unwrap();
     }
 
     pub fn add_env_var(&mut self, key: &str, val: &str) {
@@ -115,7 +117,7 @@ impl TestEnvironment {
         // Simplified TOML escaping, hoping that there are no '"' or control characters
         // in it
         let escaped_diff_editor_path = diff_editor_path.to_str().unwrap().replace('\\', r"\\");
-        self.write_config(
+        self.add_config(
             format!(
                 r###"
         [ui]

tests/test_global_opts.rs

@@ -68,7 +68,7 @@ fn test_color_config() {
     let mut test_env = TestEnvironment::default();
 
     // Test that color is used if it's requested in the config file
-    test_env.write_config(
+    test_env.add_config(
         br#"[ui]
 color="always""#,
     );
@@ -95,13 +95,9 @@ fn test_invalid_config() {
     // Test that we get a reasonable error if the config is invalid (#55)
     let test_env = TestEnvironment::default();
 
-    std::fs::write(
-        test_env.config_path(),
-        "[section]key = value-missing-quotes",
-    )
-    .unwrap();
+    test_env.add_config(b"[section]key = value-missing-quotes");
     let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["init", "repo"]);
-    insta::assert_snapshot!(stderr, @"Invalid config: expected newline, found an identifier at line 1 column 10 in config.toml
+    insta::assert_snapshot!(stderr.replace('\\', "/"), @"Invalid config: expected newline, found an identifier at line 1 column 10 in config/config0001.toml
 ");
 }