Skip to content

Commit

Permalink
cli: use cwd-relative workspace config to resolve default command and…
Browse files Browse the repository at this point in the history
… aliases

This is basically the same as Mercurial's workaround. I don't know about Git,
but arguments order is very restricted in git, so -C path can be parsed prior
to alias expansion. In hg and jj, doing that would be messy and unreliable.

Closes #2414
  • Loading branch information
yuja committed Dec 23, 2023
1 parent 941e538 commit 30b5e88
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 6 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed bugs

* Command aliases can now be loaded from repository config relative to the
current working directory.
[#2414](https://github.com/martinvonz/jj/issues/2414)


## [0.12.0] - 2023-12-05

Expand Down
20 changes: 14 additions & 6 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2959,9 +2959,18 @@ impl CliRunner {
"Did you check-out a commit where the directory doesn't exist?",
)
})?;
// Use cwd-relative workspace configs to resolve default command and
// aliases. WorkspaceLoader::init() won't do any heavy lifting other
// than the path resolution.
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()?;
if let Ok(loader) = &maybe_cwd_workspace_loader {
layered_configs.read_repo_config(loader.repo_path())?;
}
let config = layered_configs.merge();
ui.reset(&config)?;

let string_args = expand_args(ui, &self.app, std::env::args_os(), &config)?;
let (matches, args) = parse_args(
ui,
Expand All @@ -2978,15 +2987,14 @@ impl CliRunner {
// Invalid -R path is an error. No need to proceed.
let loader = WorkspaceLoader::init(&cwd.join(path))
.map_err(|err| map_workspace_load_error(err, Some(path)))?;
// TODO: maybe show error/warning if command aliases expanded differently
layered_configs.read_repo_config(loader.repo_path())?;
Ok(loader)
} else {
WorkspaceLoader::init(find_workspace_dir(&cwd))
.map_err(|err| map_workspace_load_error(err, None))
maybe_cwd_workspace_loader
};
if let Ok(loader) = &maybe_workspace_loader {
// TODO: maybe show error/warning if repo config contained command alias
layered_configs.read_repo_config(loader.repo_path())?;
}

// Apply workspace configs and --config-toml arguments.
let config = layered_configs.merge();
ui.reset(&config)?;
let settings = UserSettings::from_config(config);
Expand Down
64 changes: 64 additions & 0 deletions cli/tests/test_alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::fs;

use itertools::Itertools as _;

use crate::common::TestEnvironment;
Expand Down Expand Up @@ -238,3 +240,65 @@ fn test_alias_invalid_definition() {
Error: Alias definition for "non-string-list" must be a string list
"###);
}

#[test]
fn test_alias_in_repo_config() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo1", "--git"]);
let repo1_path = test_env.env_root().join("repo1");
fs::create_dir(repo1_path.join("sub")).unwrap();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo2", "--git"]);
let repo2_path = test_env.env_root().join("repo2");
fs::create_dir(repo2_path.join("sub")).unwrap();

test_env.add_config(r#"aliases.l = ['log', '-r@', '--no-graph', '-T"user alias\n"']"#);
fs::write(
repo1_path.join(".jj/repo/config.toml"),
r#"aliases.l = ['log', '-r@', '--no-graph', '-T"repo1 alias\n"']"#,
)
.unwrap();

// In repo1 sub directory, aliases can be loaded from the repo1 config.
let stdout = test_env.jj_cmd_success(&repo1_path.join("sub"), &["l"]);
insta::assert_snapshot!(stdout, @r###"
repo1 alias
"###);

// In repo2 directory, no repo-local aliases exist.
let stdout = test_env.jj_cmd_success(&repo2_path, &["l"]);
insta::assert_snapshot!(stdout, @r###"
user alias
"###);

// Aliases can't be loaded from the -R path due to chicken and egg problem.
let (stdout, stderr) =
test_env.jj_cmd_ok(&repo2_path, &["l", "-R", repo1_path.to_str().unwrap()]);
insta::assert_snapshot!(stdout, @r###"
user alias
"###);
insta::assert_snapshot!(stderr, @"");

// Aliases are loaded from the cwd-relative workspace even with -R.
let (stdout, stderr) =
test_env.jj_cmd_ok(&repo1_path, &["l", "-R", repo2_path.to_str().unwrap()]);
insta::assert_snapshot!(stdout, @r###"
repo1 alias
"###);
insta::assert_snapshot!(stderr, @"");

// Config loaded from the cwd-relative workspace shouldn't persist. It's
// used only for command arguments expansion.
let stdout = test_env.jj_cmd_success(
&repo1_path,
&[
"config",
"list",
"aliases",
"-R",
repo2_path.to_str().unwrap(),
],
);
insta::assert_snapshot!(stdout, @r###"
aliases.l=["log", "-r@", "--no-graph", "-T\"user alias\\n\""]
"###);
}

0 comments on commit 30b5e88

Please sign in to comment.