diff --git a/docs/templates.md b/docs/templates.md index 9a3dca1d6..0b75e9cf9 100644 --- a/docs/templates.md +++ b/docs/templates.md @@ -18,7 +18,7 @@ The following keywords can be used in `jj log`/`jj obslog` templates. * `description: String` * `change_id: ChangeId` * `commit_id: CommitId` -* `parent_commit_ids: List` +* `parents: List` * `author: Signature` * `committer: Signature` * `working_copies: String`: For multi-workspace repository, indicate @@ -75,6 +75,11 @@ The following functions are defined. No methods are defined. +### Commit type + +This type cannot be printed. All commit keywords are accessible as 0-argument +methods. + ### CommitId / ChangeId type The following methods are defined. @@ -93,7 +98,7 @@ The following methods are defined. * `.join(separator: Template) -> Template`: Concatenate elements with the given `separator`. * `.map(|item| expression) -> ListTemplate`: Apply template `expression` - to each element. Example: `parent_commit_ids.map(|id| id.short())` + to each element. Example: `parents.map(|c| c.commit_id().short())` ### ListTemplate type diff --git a/src/commit_templater.rs b/src/commit_templater.rs index c0cc9d44c..8188b2030 100644 --- a/src/commit_templater.rs +++ b/src/commit_templater.rs @@ -31,7 +31,7 @@ use crate::template_parser::{ self, FunctionCallNode, TemplateAliasesMap, TemplateParseError, TemplateParseResult, }; use crate::templater::{ - self, IntoTemplate, PlainTextFormattedProperty, Template, TemplateFunction, TemplateProperty, + IntoTemplate, PlainTextFormattedProperty, Template, TemplateFunction, TemplateProperty, TemplatePropertyFn, }; use crate::text_util; @@ -61,18 +61,21 @@ impl<'repo> TemplateLanguage<'repo> for CommitTemplateLanguage<'repo, '_> { CommitTemplatePropertyKind::Core(property) => { template_builder::build_core_method(self, build_ctx, property, function) } - CommitTemplatePropertyKind::CommitOrChangeId(property) => { - build_commit_or_change_id_method(self, build_ctx, property, function) + CommitTemplatePropertyKind::Commit(property) => { + build_commit_method(self, build_ctx, property, function) } - CommitTemplatePropertyKind::CommitOrChangeIdList(property) => { - template_builder::build_formattable_list_method( + CommitTemplatePropertyKind::CommitList(property) => { + template_builder::build_unformattable_list_method( self, build_ctx, property, function, - |item| self.wrap_commit_or_change_id(item), + |item| self.wrap_commit(item), ) } + CommitTemplatePropertyKind::CommitOrChangeId(property) => { + build_commit_or_change_id_method(self, build_ctx, property, function) + } CommitTemplatePropertyKind::ShortestIdPrefix(property) => { build_shortest_id_prefix_method(self, build_ctx, property, function) } @@ -83,6 +86,20 @@ impl<'repo> TemplateLanguage<'repo> for CommitTemplateLanguage<'repo, '_> { // If we need to add multiple languages that support Commit types, this can be // turned into a trait which extends TemplateLanguage. impl<'repo> CommitTemplateLanguage<'repo, '_> { + fn wrap_commit( + &self, + property: impl TemplateProperty + 'repo, + ) -> CommitTemplatePropertyKind<'repo> { + CommitTemplatePropertyKind::Commit(Box::new(property)) + } + + fn wrap_commit_list( + &self, + property: impl TemplateProperty> + 'repo, + ) -> CommitTemplatePropertyKind<'repo> { + CommitTemplatePropertyKind::CommitList(Box::new(property)) + } + fn wrap_commit_or_change_id( &self, property: impl TemplateProperty + 'repo, @@ -90,13 +107,6 @@ impl<'repo> CommitTemplateLanguage<'repo, '_> { CommitTemplatePropertyKind::CommitOrChangeId(Box::new(property)) } - fn wrap_commit_or_change_id_list( - &self, - property: impl TemplateProperty> + 'repo, - ) -> CommitTemplatePropertyKind<'repo> { - CommitTemplatePropertyKind::CommitOrChangeIdList(Box::new(property)) - } - fn wrap_shortest_id_prefix( &self, property: impl TemplateProperty + 'repo, @@ -107,8 +117,9 @@ impl<'repo> CommitTemplateLanguage<'repo, '_> { enum CommitTemplatePropertyKind<'repo> { Core(CoreTemplatePropertyKind<'repo, Commit>), + Commit(Box + 'repo>), + CommitList(Box> + 'repo>), CommitOrChangeId(Box + 'repo>), - CommitOrChangeIdList(Box> + 'repo>), ShortestIdPrefix(Box + 'repo>), } @@ -143,12 +154,11 @@ impl<'repo> IntoTemplateProperty<'repo, Commit> for CommitTemplatePropertyKind<' fn try_into_template(self) -> Option + 'repo>> { match self { CommitTemplatePropertyKind::Core(property) => property.try_into_template(), + CommitTemplatePropertyKind::Commit(_) => None, + CommitTemplatePropertyKind::CommitList(_) => None, CommitTemplatePropertyKind::CommitOrChangeId(property) => { Some(property.into_template()) } - CommitTemplatePropertyKind::CommitOrChangeIdList(property) => { - Some(property.into_template()) - } CommitTemplatePropertyKind::ShortestIdPrefix(property) => { Some(property.into_template()) } @@ -171,6 +181,20 @@ fn build_commit_keyword<'repo>( .ok_or_else(|| TemplateParseError::no_such_keyword(name, span)) } +fn build_commit_method<'repo>( + language: &CommitTemplateLanguage<'repo, '_>, + _build_ctx: &BuildContext>, + self_property: impl TemplateProperty + 'repo, + function: &FunctionCallNode, +) -> TemplateParseResult> { + if let Some(property) = build_commit_keyword_opt(language, self_property, function.name) { + template_parser::expect_no_arguments(function)?; + Ok(property) + } else { + Err(TemplateParseError::no_such_method("Commit", function)) + } +} + fn build_commit_keyword_opt<'repo>( language: &CommitTemplateLanguage<'repo, '_>, property: impl TemplateProperty + 'repo, @@ -201,15 +225,7 @@ fn build_commit_keyword_opt<'repo>( "commit_id" => language.wrap_commit_or_change_id(wrap_fn(property, |commit| { CommitOrChangeId::Commit(commit.id().to_owned()) })), - "parent_commit_ids" => { - language.wrap_commit_or_change_id_list(wrap_fn(property, move |commit| { - commit - .parent_ids() - .iter() - .map(|id| CommitOrChangeId::Commit(id.to_owned())) - .collect() - })) - } + "parents" => language.wrap_commit_list(wrap_fn(property, |commit| commit.parents())), "author" => language.wrap_signature(wrap_fn(property, |commit| commit.author().clone())), "committer" => { language.wrap_signature(wrap_fn(property, |commit| commit.committer().clone())) @@ -381,12 +397,6 @@ impl Template<()> for CommitOrChangeId { } } -impl Template<()> for Vec { - fn format(&self, _: &(), formatter: &mut dyn Formatter) -> io::Result<()> { - templater::format_joined(&(), formatter, self, " ") - } -} - fn build_commit_or_change_id_method<'repo>( language: &CommitTemplateLanguage<'repo, '_>, build_ctx: &BuildContext>, diff --git a/tests/test_commit_template.rs b/tests/test_commit_template.rs index 7b7b8cc07..00e68b1fb 100644 --- a/tests/test_commit_template.rs +++ b/tests/test_commit_template.rs @@ -18,7 +18,7 @@ use regex::Regex; pub mod common; #[test] -fn test_log_parent_commit_ids() { +fn test_log_parents() { let test_env = TestEnvironment::default(); test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); let repo_path = test_env.env_root().join("repo"); @@ -27,7 +27,7 @@ fn test_log_parent_commit_ids() { test_env.jj_cmd_success(&repo_path, &["new", "@-"]); test_env.jj_cmd_success(&repo_path, &["new", "@", "@-"]); - let template = r#"commit_id ++ "\nP: " ++ parent_commit_ids ++ "\n""#; + let template = r#"commit_id ++ "\nP: " ++ parents.map(|c| c.commit_id()) ++ "\n""#; let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", template]); insta::assert_snapshot!(stdout, @r###" @ c067170d4ca1bc6162b64f7550617ec809647f84 @@ -40,16 +40,39 @@ fn test_log_parent_commit_ids() { P: "###); - let template = r#"parent_commit_ids.map(|id| id.shortest(4))"#; + let template = r#"parents.map(|c| c.commit_id().shortest(4))"#; let stdout = test_env.jj_cmd_success( &repo_path, &["log", "-T", template, "-r@", "--color=always"], ); insta::assert_snapshot!(stdout, @r###" - @ 4db4 230d + @ 4db4 230d │ ~ "###); + + // Commit object isn't printable + let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-T", "parents"]); + insta::assert_snapshot!(stderr, @r###" + Error: Failed to parse template: --> 1:1 + | + 1 | parents + | ^-----^ + | + = Expected expression of type "Template" + "###); + + // Redundant argument passed to keyword method + let template = r#"parents.map(|c| c.commit_id(""))"#; + let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-T", template]); + insta::assert_snapshot!(stderr, @r###" + Error: Failed to parse template: --> 1:29 + | + 1 | parents.map(|c| c.commit_id("")) + | ^^ + | + = Function "commit_id": Expected 0 arguments + "###); } #[test]