Skip to content

Commit

Permalink
give a warning when trying to redefine a built-in command
Browse files Browse the repository at this point in the history
previously, aliases to built-in commands were silently ignored. this matches git's behavior, but seems unhelpful, especially if the user doesn't know that a command with that name already exists.
give a warning rather than silently ignoring it.
  • Loading branch information
jyn514 committed May 17, 2024
1 parent 8e0ac1d commit 0d3e949
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Deprecations

- Attempting to alias a built-in command now gives a warning, rather than being silently ignored.

### New features

* `jj branch list`/`tag list` now accept `-T`/`--template` option. The tag list
Expand Down
13 changes: 12 additions & 1 deletion cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2600,6 +2600,7 @@ fn resolve_default_command(
}

fn resolve_aliases(
ui: &Ui,
config: &config::Config,
app: &Command,
mut string_args: Vec<String>,
Expand All @@ -2616,6 +2617,7 @@ fn resolve_aliases(
}
}
}

let mut resolved_aliases = HashSet::new();
let mut real_commands = HashSet::new();
for command in app.get_subcommands() {
Expand All @@ -2624,6 +2626,15 @@ fn resolve_aliases(
real_commands.insert(alias.to_string());
}
}
for alias in aliases_map.keys() {
if real_commands.contains(alias) {
writeln!(
ui.warning_default(),
"Cannot define an alias that overrides the built-in command '{alias}'"
)?;
}
}

loop {
let app_clone = app.clone().allow_external_subcommands(true);
let matches = app_clone.try_get_matches_from(&string_args).ok();
Expand Down Expand Up @@ -2713,7 +2724,7 @@ pub fn expand_args(
}

let string_args = resolve_default_command(ui, config, app, string_args)?;
resolve_aliases(config, app, string_args)
resolve_aliases(ui, config, app, string_args)
}

pub fn parse_args(
Expand Down
9 changes: 4 additions & 5 deletions cli/tests/test_alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,10 @@ fn test_alias_cannot_override_builtin() {
let repo_path = test_env.env_root().join("repo");

test_env.add_config(r#"aliases.log = ["rebase"]"#);
// Alias should be ignored
let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-r", "root()"]);
insta::assert_snapshot!(stdout, @r###"
◉ zzzzzzzz root() 00000000
"###);
// Alias should give a warning
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r", "root()"]);
insta::assert_snapshot!(stdout, @"◉ zzzzzzzz root() 00000000\n");
insta::assert_snapshot!(stderr, @"Warning: Cannot define an alias that overrides the built-in command 'log'\n");
}

#[test]
Expand Down

0 comments on commit 0d3e949

Please sign in to comment.