diff --git a/CHANGELOG.md b/CHANGELOG.md index 3404c54673..7c660fde78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * The `commit_summary_no_branches` template is superseded by `templates.branch_list`. +* `jj split` will now refuse to split an empty commit. + ### Deprecations ### New features diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index 9661a50b45..ef4faa2af7 100644 --- a/cli/src/commands/split.rs +++ b/cli/src/commands/split.rs @@ -18,7 +18,7 @@ use jj_lib::repo::Repo; use tracing::instrument; use crate::cli_util::{CommandHelper, RevisionArg}; -use crate::command_error::CommandError; +use crate::command_error::{user_error_with_hint, CommandError}; use crate::description_util::{description_template_for_commit, edit_description}; use crate::ui::Ui; @@ -36,6 +36,9 @@ use crate::ui::Ui; /// change description for each commit. If the change did not have a /// description, the second part will not get a description, and you will be /// asked for a description only for the first part. +/// +/// Splitting an empty commit is not supported because the same effect can be +/// achieved with `jj new`. #[derive(clap::Args, Clone, Debug)] pub(crate) struct SplitArgs { /// Interactively choose which parts to split. This is the default if no @@ -64,6 +67,13 @@ pub(crate) fn cmd_split( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let commit = workspace_command.resolve_single_rev(&args.revision)?; + if commit.is_empty(workspace_command.repo().as_ref())? { + return Err(user_error_with_hint( + format!("Refusing to split empty commit {}.", commit.id().hex()), + "Use `jj new` if you want to create another empty commit.", + )); + } + workspace_command.check_rewritable([commit.id()])?; let matcher = workspace_command .parse_file_patterns(&args.paths)? diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index a6bd3ef0e5..216e367464 100644 --- a/cli/src/commit_templater.rs +++ b/cli/src/commit_templater.rs @@ -641,13 +641,7 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm map.insert("empty", |language, _build_ctx, self_property, function| { template_parser::expect_no_arguments(function)?; let repo = language.repo; - let out_property = self_property.and_then(|commit| { - if let [parent] = &commit.parents()[..] { - return Ok(parent.tree_id() == commit.tree_id()); - } - let parent_tree = commit.parent_tree(repo)?; - Ok(*commit.tree_id() == parent_tree.id()) - }); + let out_property = self_property.and_then(|commit| Ok(commit.is_empty(repo)?)); Ok(L::wrap_boolean(out_property)) }); map.insert("root", |language, _build_ctx, self_property, function| { diff --git a/cli/tests/test_split_command.rs b/cli/tests/test_split_command.rs index 051fe24f72..8c9116f2c2 100644 --- a/cli/tests/test_split_command.rs +++ b/cli/tests/test_split_command.rs @@ -523,3 +523,18 @@ fn test_split_siblings_with_merge_child() { ◉ zzzzzzzzzzzz true "###); } + +// Make sure `jj split` would refuse to split an empty commit. +#[test] +fn test_split_empty() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let workspace_path = test_env.env_root().join("repo"); + test_env.jj_cmd_ok(&workspace_path, &["describe", "--message", "abc"]); + + let stderr = test_env.jj_cmd_failure(&workspace_path, &["split"]); + insta::assert_snapshot!(stderr, @r###" + Error: Refusing to split empty commit 82b6292b775dc4e5c5e6f402faa599dad02d02a0. + Hint: Use `jj new` if you want to create another empty commit. + "###); +} diff --git a/lib/src/commit.rs b/lib/src/commit.rs index d1a8d202fa..f8880471de 100644 --- a/lib/src/commit.rs +++ b/lib/src/commit.rs @@ -116,6 +116,16 @@ impl Commit { merge_commit_trees(repo, &self.parents()) } + /// Returns whether commit's content is empty. Commit description is not + /// taken into consideration. + pub fn is_empty(&self, repo: &dyn Repo) -> BackendResult { + if let [parent] = &self.parents()[..] { + return Ok(parent.tree_id() == self.tree_id()); + } + let parent_tree = self.parent_tree(repo)?; + Ok(*self.tree_id() == parent_tree.id()) + } + pub fn has_conflict(&self) -> BackendResult { if let MergedTreeId::Merge(tree_ids) = self.tree_id() { Ok(!tree_ids.is_resolved())