Skip to content

Commit

Permalink
Show paths of config files when configurations contain errors
Browse files Browse the repository at this point in the history
This addresses issue #3317, where as discussed we want to show the paths to
configuration files if they contain errors, to make it easier for the user to
locate them.
  • Loading branch information
Kintaro committed Jun 10, 2024
1 parent 65a988e commit be74716
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### New features

* Show paths to config files when configuration errors occur

### Fixed bugs

## [0.18.0] - 2024-06-05
Expand Down
12 changes: 11 additions & 1 deletion cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2828,11 +2828,21 @@ impl CliRunner {
let maybe_cwd_workspace_loader = WorkspaceLoader::init(find_workspace_dir(&cwd))
.map_err(|err| map_workspace_load_error(err, None));
layered_configs.read_user_config()?;
let mut repo_config_path = None;
if let Ok(loader) = &maybe_cwd_workspace_loader {
layered_configs.read_repo_config(loader.repo_path())?;
repo_config_path = Some(layered_configs.repo_config_path(loader.repo_path()));
}
let config = layered_configs.merge();
ui.reset(&config)?;
ui.reset(&config).map_err(|e| {
let user_config_path = layered_configs.user_config_path().unwrap_or(None);
let paths = [repo_config_path, user_config_path]
.into_iter()
.flatten()
.map(|path| format!("- {}", path.display()))
.join("\n");
e.hinted(format!("Check the following config files:\n{}", paths))
})?;

let string_args = expand_args(ui, &self.app, env::args_os(), &config)?;
let (matches, args) = parse_args(
Expand Down
10 changes: 9 additions & 1 deletion cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,20 @@ impl LayeredConfigs {
Ok(())
}

pub fn user_config_path(&self) -> Result<Option<PathBuf>, ConfigError> {
existing_config_path()
}

#[instrument]
pub fn read_repo_config(&mut self, repo_path: &Path) -> Result<(), ConfigError> {
self.repo = Some(read_config_file(&repo_path.join("config.toml"))?);
self.repo = Some(read_config_file(&self.repo_config_path(repo_path))?);
Ok(())
}

pub fn repo_config_path(&self, repo_path: &Path) -> PathBuf {
repo_path.join("config.toml")
}

pub fn parse_config_args(&mut self, toml_strs: &[String]) -> Result<(), ConfigError> {
let config = toml_strs
.iter()
Expand Down
22 changes: 22 additions & 0 deletions cli/tests/test_config_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,28 @@ fn test_config_path_syntax() {
"###);
}

#[test]
fn test_config_show_paths() {
let mut test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let user_config_path = test_env.config_path().join("config.toml");
test_env.set_config_path(user_config_path.to_owned());
let repo_path = test_env.env_root().join("repo");

test_env.jj_cmd_ok(
&repo_path,
&["config", "set", "--user", "ui.paginate", ":builtin"],
);
let stderr = test_env.jj_cmd_failure(test_env.env_root(), &["st"]);
insta::assert_snapshot!(stderr, @r###"
Config error: Invalid `ui.paginate`
Caused by: enum PaginationChoice does not have variant constructor :builtin
Hint: Check the following config files:
- $TEST_ENV/config/config.toml
For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md.
"###);
}

fn find_stdout_lines(keyname_pattern: &str, stdout: &str) -> String {
let key_line_re = Regex::new(&format!(r"(?m)^{keyname_pattern} = .*$")).unwrap();
key_line_re
Expand Down

0 comments on commit be74716

Please sign in to comment.