diff --git a/Cargo.lock b/Cargo.lock index 1055f79..3d53791 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -237,8 +237,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c673075a2e0e5f4a1dde27ce9dee1ea4558c7ffe648f576438a20ca1d2acc4b0" dependencies = [ "iana-time-zone", + "js-sys", "num-traits", "serde", + "wasm-bindgen", "windows-link", ] @@ -1655,6 +1657,7 @@ dependencies = [ "assert_cmd", "assert_fs", "async-trait", + "chrono", "clap", "etcetera", "git-conventional", diff --git a/Cargo.toml b/Cargo.toml index f41707a..b3fd06a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,7 @@ thiserror = "2.0.18" tokio = { version = "1.49.0", features = ["macros", "rt-multi-thread"] } textwrap = "0.16.2" unicode-width = "0.2.2" +chrono = "0.4.44" [dev-dependencies] assert_cmd = "2.1.2" diff --git a/README.md b/README.md index 40437c3..f92482e 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,16 @@ jj-cz The tool detects whether you're in a Jujutsu repository, guides you through the commit message, and applies it to your current change. +You can also set the revision message of a few revisions at once, or +target a single revision other than the current one. + +```sh +jj-cz @- xs develop +``` + +No explicit revision is simply the equivalent of `jj-cz @`, like +`jj desc`. + ## Requirements - A Jujutsu repository @@ -41,8 +51,44 @@ what `jj-cz` alone would be good for without `jj`. | 130 | Interrupted | ## Installation +### From crates.io +Simply run the following command: -You can install jj-cz with Cargo by building it from source. +``` +cargo install jj-cz +``` + +Done! `jj-cz` is now available! + +### With Nix Flakes +Notice how there’s a `flake.nix` file? This means you can run the +project using this repository as one of your flakes inputs. In fact, +that’s how I install it in my own NixOS configuration! Add this +repository to your configuration: + +```nix +{ + inputs = { + nixpkgs.url = "github:nixos/nixpkgs?ref=nixos-unstable"; + + jj-cz = { + url = "git+https://labs.phundrak.com/phundrak/jj-cz"; + inputs.nixpkgs.follows = "nixpkgs"; + }; + }; +} +``` + +And tadah! you can now install +`inputs.jj-cz.packages.${pkgs.stdenv.hostPlatform.system}.default` +among your other packages. Take a look at my +[`jujutsu.nix`](https://labs.phundrak.com/phundrak/nix-config/src/branch/main/users/modules/dev/vcs/jujutsu.nix) +module if you need some inspiration. + +### From source + +You can also install `jj-cz` with Cargo by building it from source. +Just make sure Rust is available on your machine (duh!). ```sh cargo install --path . diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 77c6ced..cefcb56 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -14,4 +14,19 @@ use clap::Parser; Jujutsu repository.\n\n\ This tool requires an interactive terminal (TTY)." )] -pub struct Cli; +pub struct Cli { + /// The revision(s) whose description to edit (default: @) + #[arg(value_name = "REVSETS")] + revsets: Vec, +} + +impl Cli { + /// Returns the revsets to operate on, defaulting to `["@"]` if none provided + pub fn revsets(&self) -> Vec<&str> { + if self.revsets.is_empty() { + vec!["@"] + } else { + self.revsets.iter().map(|s| s.as_str()).collect() + } + } +} diff --git a/src/commit/types/body.rs b/src/commit/types/body.rs index 885ab3e..6aac53e 100644 --- a/src/commit/types/body.rs +++ b/src/commit/types/body.rs @@ -32,7 +32,7 @@ impl Body { mod tests { use super::*; - /// Default produces Body(None) — no body + /// Default produces Body(None) - no body #[test] fn default_produces_none() { assert_eq!(Body::default(), Body(None)); @@ -71,7 +71,7 @@ mod tests { ); } - /// Leading and internal whitespace is preserved — users may write + /// Leading and internal whitespace is preserved - users may write /// indented lists, ASCII art, file trees, etc. #[test] fn from_preserves_leading_whitespace() { diff --git a/src/commit/types/breaking_change.rs b/src/commit/types/breaking_change.rs index c28b170..e22d551 100644 --- a/src/commit/types/breaking_change.rs +++ b/src/commit/types/breaking_change.rs @@ -66,7 +66,7 @@ where mod tests { use super::*; - /// Empty string produces Yes(None) — no footer, only '!' in the header + /// Empty string produces Yes(None) - no footer, only '!' in the header #[test] fn from_empty_string_yields_yes_none() { assert_eq!(BreakingChange::from(String::new()), BreakingChange::Yes); diff --git a/src/commit/types/description.rs b/src/commit/types/description.rs index 3164ef2..e50893d 100644 --- a/src/commit/types/description.rs +++ b/src/commit/types/description.rs @@ -6,7 +6,7 @@ impl Description { /// Soft limit for description length. /// /// Descriptions over this length are warned about at the prompt layer but - /// are not rejected here — the hard limit is the 72-character total first + /// are not rejected here - the hard limit is the 72-character total first /// line enforced by [`crate::ConventionalCommit`]. pub const MAX_LENGTH: usize = 50; diff --git a/src/commit/types/message.rs b/src/commit/types/message.rs index f74e8cc..7a89df1 100644 --- a/src/commit/types/message.rs +++ b/src/commit/types/message.rs @@ -10,7 +10,7 @@ pub enum CommitMessageError { /// The formatted message is not parseable as a conventional commit /// - /// This should never occur in normal use — it indicates a bug in the + /// This should never occur in normal use - it indicates a bug in the /// formatting logic. #[error("output failed git-conventional validation: {reason}")] InvalidConventionalFormat { reason: String }, @@ -932,7 +932,7 @@ mod tests { /// Breaking change footer does not count toward the 72-character first-line limit #[test] fn breaking_change_footer_does_not_count_toward_line_limit() { - // First line is short; the note itself is long — should still be accepted. + // First line is short; the note itself is long - should still be accepted. let long_note = "x".repeat(200); let result = ConventionalCommit::new( CommitType::Fix, diff --git a/src/error.rs b/src/error.rs index fadee9a..a287312 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,3 +1,5 @@ +use jj_lib::revset::{RevsetEvaluationError, RevsetParseError, RevsetResolutionError}; + use crate::commit::types::{CommitMessageError, DescriptionError, ScopeError}; #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] @@ -25,6 +27,10 @@ pub enum Error { Cancelled, #[error("Non-interactive terminal detected")] NonInteractive, + #[error("Failed to resolve revision '{revset}': {context}")] + RevsetResolutionError { revset: String, context: String }, + #[error("Revision set '{revset}' resolves to multiple commits; specify a single revision")] + MultipleRevisions { revset: String }, } impl From for Error { @@ -46,7 +52,38 @@ impl From for Error { } impl From for Error { - fn from(_value: std::io::Error) -> Self { + fn from(_: std::io::Error) -> Self { Self::FailedGettingCurrentDir } } + +impl From> for Error { + fn from(_: std::sync::PoisonError) -> Self { + Self::JjOperation { + context: "internal lock poisoned".to_string(), + } + } +} + +impl Error { + pub fn from_revset_parse_error(revset: &str, error: RevsetParseError) -> Self { + Self::RevsetResolutionError { + revset: revset.to_string(), + context: error.to_string(), + } + } + + pub fn from_revset_resolution_error(revset: &str, error: RevsetResolutionError) -> Self { + Self::RevsetResolutionError { + revset: revset.to_string(), + context: error.to_string(), + } + } + + pub fn from_revset_evaluation_error(revset: &str, error: RevsetEvaluationError) -> Self { + Self::RevsetResolutionError { + revset: revset.to_string(), + context: error.to_string(), + } + } +} diff --git a/src/jj/lib_executor.rs b/src/jj/lib_executor.rs index aa0503f..a328f2c 100644 --- a/src/jj/lib_executor.rs +++ b/src/jj/lib_executor.rs @@ -3,13 +3,24 @@ //! This implementation uses jj-lib 0.39.0 directly for repository detection //! and commit description, replacing the earlier shell-out approach. -use std::path::{Path, PathBuf}; +use std::{ + collections::HashMap, + path::{Path, PathBuf}, + sync::{Arc, Mutex}, +}; use etcetera::BaseStrategy; use jj_lib::{ + backend::CommitId, config::{ConfigSource, StackedConfig}, - ref_name::WorkspaceName, - repo::{Repo, StoreFactories}, + fileset::FilesetAliasesMap, + ref_name::WorkspaceNameBuf, + repo::{ReadonlyRepo, Repo, StoreFactories}, + repo_path::RepoPathUiConverter, + revset::{ + self, RevsetAliasesMap, RevsetDiagnostics, RevsetExtensions, RevsetParseContext, + RevsetWorkspaceContext, SymbolResolver, SymbolResolverExtension, + }, settings::UserSettings, workspace::{Workspace, default_working_copy_factories}, }; @@ -21,20 +32,58 @@ use crate::jj::JjExecutor; #[derive(Debug)] pub struct JjLib { working_dir: PathBuf, + repo: Mutex>, + workspace_name: WorkspaceNameBuf, + workspace_root: PathBuf, } impl JjLib { /// Create a new JjLib instance using the current working directory - pub fn new() -> Result { + pub async fn new() -> Result { let working_dir = std::env::current_dir()?; - Ok(Self { working_dir }) + let (repo, workspace_name, workspace_root) = Self::load_repo(&working_dir).await?; + Ok(Self { + working_dir, + repo: repo.into(), + workspace_name, + workspace_root, + }) } /// Create a new JjLib instance with a specific working directory - pub fn with_working_dir(path: impl AsRef) -> Self { - Self { + pub async fn with_working_dir(path: impl AsRef) -> Result { + let (repo, workspace_name, workspace_root) = Self::load_repo(path.as_ref()).await?; + Ok(Self { working_dir: path.as_ref().to_path_buf(), - } + repo: repo.into(), + workspace_name, + workspace_root, + }) + } + + /// Load the repo from the given working directory + async fn load_repo( + working_dir: &Path, + ) -> Result<(Arc, WorkspaceNameBuf, PathBuf), Error> { + let settings = Self::load_settings()?; + let store_factories = StoreFactories::default(); + let wc_factories = default_working_copy_factories(); + + let workspace = Workspace::load(&settings, working_dir, &store_factories, &wc_factories) + .map_err(|_| Error::NotARepository)?; + let repo = + workspace + .repo_loader() + .load_at_head() + .await + .map_err(|e| Error::JjOperation { + context: e.to_string(), + })?; + Ok(( + repo, + workspace.workspace_name().to_owned(), + workspace.workspace_root().to_path_buf(), + )) } fn load_settings() -> Result { @@ -100,6 +149,51 @@ impl JjLib { paths } + + /// Resolve a revset string to a commit ID + fn get_commit_id(&self, revset: &str) -> Result { + let context = RevsetParseContext { + workspace: Some(RevsetWorkspaceContext { + workspace_name: &self.workspace_name, + path_converter: &RepoPathUiConverter::Fs { + cwd: self.working_dir.clone(), + base: self.workspace_root.clone(), + }, + }), + aliases_map: &RevsetAliasesMap::new(), + fileset_aliases_map: &FilesetAliasesMap::new(), + local_variables: HashMap::new(), + user_email: "", + date_pattern_context: chrono::Local::now().into(), + default_ignored_remote: None, + use_glob_by_default: false, + extensions: &RevsetExtensions::default(), + }; + let mut diagnostic = RevsetDiagnostics::new(); + let repo = self.repo.lock()?.clone(); + let symbol_resolver = + SymbolResolver::new(&*repo, &([] as [Box; 0])); + let revision = revset::parse(&mut diagnostic, revset, &context) + .map_err(|e| Error::from_revset_parse_error(revset, e))? + .resolve_user_expression(&*repo, &symbol_resolver) + .map_err(|e| Error::from_revset_resolution_error(revset, e))? + .evaluate(&*repo) + .map_err(|e| Error::from_revset_evaluation_error(revset, e))?; + let mut iter = revision.iter(); + let commit_id = iter + .next() + .ok_or(Error::RevsetResolutionError { + revset: revset.to_string(), + context: "No matching revision".to_string(), + })? + .map_err(|e| Error::from_revset_evaluation_error(revset, e))?; + if iter.next().is_some() { + return Err(Error::MultipleRevisions { + revset: revset.to_string(), + }); + } + Ok(commit_id) + } } #[async_trait::async_trait(?Send)] @@ -117,49 +211,20 @@ impl JjExecutor for JjLib { .is_ok()) } - async fn describe(&self, message: &str) -> Result<(), Error> { - let settings = Self::load_settings()?; - let store_factories = StoreFactories::default(); - let wc_factories = default_working_copy_factories(); - - let workspace = Workspace::load( - &settings, - &self.working_dir, - &store_factories, - &wc_factories, - ) - .map_err(|_| Error::NotARepository)?; - - let repo = - workspace - .repo_loader() - .load_at_head() - .await - .map_err(|e| Error::JjOperation { - context: e.to_string(), - })?; - + async fn describe(&self, revset: &str, message: &str) -> Result<(), Error> { + let commit_id = self.get_commit_id(revset)?; + let repo = self.repo.lock()?.clone(); let mut tx = repo.start_transaction(); - - let wc_commit_id = tx + let commit = tx .repo() - .view() - .get_wc_commit_id(WorkspaceName::DEFAULT) - .ok_or_else(|| Error::JjOperation { - context: "No working copy commit found".to_string(), - })? - .clone(); - - let wc_commit = - tx.repo() - .store() - .get_commit(&wc_commit_id) - .map_err(|e| Error::JjOperation { - context: e.to_string(), - })?; + .store() + .get_commit(&commit_id) + .map_err(|e| Error::JjOperation { + context: e.to_string(), + })?; tx.repo_mut() - .rewrite_commit(&wc_commit) + .rewrite_commit(&commit) .set_description(message) .write() .await @@ -174,14 +239,28 @@ impl JjExecutor for JjLib { context: format!("{e:?}"), })?; - tx.commit("jj-cz: update commit description") + let new_repo = tx + .commit("jj-cz: update commit description") .await .map_err(|e| Error::JjOperation { context: e.to_string(), })?; + *self.repo.lock()? = new_repo; Ok(()) } + + async fn get_description(&self, revset: &str) -> Result { + let commit_id = self.get_commit_id(revset)?; + let repo = self.repo.lock()?.clone(); + let commit = repo + .store() + .get_commit(&commit_id) + .map_err(|e| Error::JjOperation { + context: e.to_string(), + })?; + Ok(commit.description().trim_end().to_string()) + } } #[cfg(test)] @@ -197,7 +276,6 @@ mod tests { .map_err(|e| format!("Failed to init jj repo: {e}")) } - /// Get the current commit description from a jj repository using jj-lib async fn get_commit_description(dir: &Path) -> Result { let settings = JjLib::load_settings().map_err(|e| e.to_string())?; let store_factories = StoreFactories::default(); @@ -214,7 +292,7 @@ mod tests { let wc_commit_id = repo .view() - .get_wc_commit_id(WorkspaceName::DEFAULT) + .get_wc_commit_id(jj_lib::ref_name::WorkspaceName::DEFAULT) .ok_or_else(|| "No working copy commit found".to_string())? .clone(); @@ -233,7 +311,7 @@ mod tests { .await .expect("Failed to init jj repo"); - let executor = JjLib::with_working_dir(temp_dir.path()); + let executor = JjLib::with_working_dir(temp_dir.path()).await.unwrap(); let result = executor.is_repository().await; assert!(result.is_ok()); @@ -244,11 +322,8 @@ mod tests { async fn is_repository_returns_false_outside_jj_repo() { let temp_dir = assert_fs::TempDir::new().unwrap(); - let executor = JjLib::with_working_dir(temp_dir.path()); - let result = executor.is_repository().await; - - assert!(result.is_ok()); - assert!(!result.unwrap()); + let executor = JjLib::with_working_dir(temp_dir.path()).await; + assert!(executor.is_err()); } #[tokio::test] @@ -259,9 +334,9 @@ mod tests { .expect("Failed to init jj repo"); let test_message = "test: initial commit"; - let executor = JjLib::with_working_dir(temp_dir.path()); + let executor = JjLib::with_working_dir(temp_dir.path()).await.unwrap(); - let result = executor.describe(test_message).await; + let result = executor.describe("@", test_message).await; assert!(result.is_ok(), "describe failed: {result:?}"); let actual = get_commit_description(temp_dir.path()) @@ -278,9 +353,9 @@ mod tests { .expect("Failed to init jj repo"); let test_message = "feat: add feature with special chars !@#$%^&*()"; - let executor = JjLib::with_working_dir(temp_dir.path()); + let executor = JjLib::with_working_dir(temp_dir.path()).await.unwrap(); - let result = executor.describe(test_message).await; + let result = executor.describe("@", test_message).await; assert!(result.is_ok()); let actual = get_commit_description(temp_dir.path()) @@ -297,9 +372,9 @@ mod tests { .expect("Failed to init jj repo"); let test_message = "docs: add unicode support 🎉 🚀"; - let executor = JjLib::with_working_dir(temp_dir.path()); + let executor = JjLib::with_working_dir(temp_dir.path()).await.unwrap(); - let result = executor.describe(test_message).await; + let result = executor.describe("@", test_message).await; assert!(result.is_ok()); let actual = get_commit_description(temp_dir.path()) @@ -316,9 +391,9 @@ mod tests { .expect("Failed to init jj repo"); let test_message = "feat: add feature\n\nThis is a multiline\ndescription"; - let executor = JjLib::with_working_dir(temp_dir.path()); + let executor = JjLib::with_working_dir(temp_dir.path()).await.unwrap(); - let result = executor.describe(test_message).await; + let result = executor.describe("@", test_message).await; assert!(result.is_ok()); let actual = get_commit_description(temp_dir.path()) @@ -329,13 +404,21 @@ mod tests { #[tokio::test] async fn describe_fails_outside_repo() { + // with_working_dir returns Err when not in a repo let temp_dir = assert_fs::TempDir::new().unwrap(); + let executor = JjLib::with_working_dir(temp_dir.path()).await; + assert!(executor.is_err()); - let executor = JjLib::with_working_dir(temp_dir.path()); - let result = executor.describe("test: should fail").await; + let valid_dir = assert_fs::TempDir::new().unwrap(); + init_jj_repo(valid_dir.path()) + .await + .expect("Failed to init jj repo"); + let executor = JjLib::with_working_dir(valid_dir.path()).await.unwrap(); + let result = executor + .describe("this-bookmark-does-not-exist", "test: should fail") + .await; assert!(result.is_err()); - assert!(matches!(result.unwrap_err(), Error::NotARepository)); } #[tokio::test] @@ -345,10 +428,10 @@ mod tests { .await .expect("Failed to init jj repo"); - let executor = JjLib::with_working_dir(temp_dir.path()); + let executor = JjLib::with_working_dir(temp_dir.path()).await.unwrap(); executor - .describe("feat: first commit") + .describe("@", "feat: first commit") .await .expect("First describe failed"); let desc1 = get_commit_description(temp_dir.path()) @@ -357,7 +440,7 @@ mod tests { assert_eq!(desc1, "feat: first commit"); executor - .describe("feat: updated commit") + .describe("@", "feat: updated commit") .await .expect("Second describe failed"); let desc2 = get_commit_description(temp_dir.path()) @@ -366,11 +449,87 @@ mod tests { assert_eq!(desc2, "feat: updated commit"); } - #[test] - fn jj_lib_implements_jj_executor_trait() { - let lib = JjLib::with_working_dir(std::path::Path::new(".")); - fn accepts_executor(_: impl JjExecutor) {} - accepts_executor(lib); + #[tokio::test] + async fn get_description_returns_empty_for_fresh_commit() { + let temp_dir = assert_fs::TempDir::new().unwrap(); + init_jj_repo(temp_dir.path()) + .await + .expect("Failed to init jj repo"); + + let executor = JjLib::with_working_dir(temp_dir.path()).await.unwrap(); + let desc = executor + .get_description("@") + .await + .expect("get_description failed"); + + assert_eq!(desc, ""); + } + + #[tokio::test] + async fn get_description_reflects_describe_on_same_executor() { + let temp_dir = assert_fs::TempDir::new().unwrap(); + init_jj_repo(temp_dir.path()) + .await + .expect("Failed to init jj repo"); + + let executor = JjLib::with_working_dir(temp_dir.path()).await.unwrap(); + let message = "feat: test get_description"; + + executor + .describe("@", message) + .await + .expect("describe failed"); + let desc = executor + .get_description("@") + .await + .expect("get_description failed"); + + assert_eq!(desc, message); + } + + #[tokio::test] + async fn multiple_revisions_error_for_multi_commit_revset() { + let temp_dir = assert_fs::TempDir::new().unwrap(); + init_jj_repo(temp_dir.path()) + .await + .expect("Failed to init jj repo"); + + let executor = JjLib::with_working_dir(temp_dir.path()).await.unwrap(); + let result = executor.describe("@ | root()", "test").await; + + assert!(matches!(result, Err(Error::MultipleRevisions { .. }))); + } + + #[tokio::test] + async fn empty_revset_returns_resolution_error() { + let temp_dir = assert_fs::TempDir::new().unwrap(); + init_jj_repo(temp_dir.path()) + .await + .expect("Failed to init jj repo"); + + let executor = JjLib::with_working_dir(temp_dir.path()).await.unwrap(); + let result = executor.describe("none()", "test").await; + + assert!(matches!(result, Err(Error::RevsetResolutionError { .. }))); + } + + #[tokio::test] + async fn invalid_revset_syntax_returns_resolution_error() { + let temp_dir = assert_fs::TempDir::new().unwrap(); + init_jj_repo(temp_dir.path()) + .await + .expect("Failed to init jj repo"); + + let executor = JjLib::with_working_dir(temp_dir.path()).await.unwrap(); + let result = executor.describe("(((invalid", "test").await; + + assert!(matches!(result, Err(Error::RevsetResolutionError { .. }))); + } + + #[tokio::test] + async fn jj_lib_implements_jj_executor_trait() { + fn assert_implements() {} + assert_implements::(); } mod user_config_paths_tests { diff --git a/src/jj/mock.rs b/src/jj/mock.rs index 13cd8cb..5edbc81 100644 --- a/src/jj/mock.rs +++ b/src/jj/mock.rs @@ -15,6 +15,10 @@ pub struct MockJjExecutor { is_repo_response: Result, /// Response to return from describe() describe_response: Result<(), Error>, + /// Track described revsets + described_revsets: Mutex>, + /// Track response to return from get_description() + get_description_response: Result, /// Track calls to is_repository() is_repo_called: AtomicBool, /// Track calls to describe() with the message passed @@ -26,6 +30,8 @@ impl Default for MockJjExecutor { Self { is_repo_response: Ok(true), describe_response: Ok(()), + described_revsets: Mutex::new(Vec::new()), + get_description_response: Ok(String::new()), is_repo_called: AtomicBool::new(false), describe_calls: Mutex::new(Vec::new()), } @@ -50,6 +56,12 @@ impl MockJjExecutor { self } + /// Configure get_description() to return a specific value + pub fn with_get_description_response(mut self, response: Result) -> Self { + self.get_description_response = response; + self + } + /// Check if is_repository() was called pub fn was_is_repo_called(&self) -> bool { self.is_repo_called @@ -60,6 +72,11 @@ impl MockJjExecutor { pub fn describe_messages(&self) -> Vec { self.describe_calls.lock().unwrap().clone() } + + /// Get all revsets visited + pub fn described_revsets(&self) -> Vec { + self.described_revsets.lock().unwrap().clone() + } } #[async_trait(?Send)] @@ -73,7 +90,11 @@ impl JjExecutor for MockJjExecutor { } } - async fn describe(&self, message: &str) -> Result<(), Error> { + async fn describe(&self, revset: &str, message: &str) -> Result<(), Error> { + self.described_revsets + .lock() + .unwrap() + .push(revset.to_string()); self.describe_calls .lock() .unwrap() @@ -83,6 +104,10 @@ impl JjExecutor for MockJjExecutor { Err(e) => Err(e.clone()), } } + + async fn get_description(&self, _revset: &str) -> Result { + self.get_description_response.clone() + } } #[cfg(test)] @@ -130,7 +155,7 @@ mod tests { #[tokio::test] async fn mock_describe_records_message() { let mock = MockJjExecutor::new(); - let result = mock.describe("test message").await; + let result = mock.describe("@", "test message").await; assert!(result.is_ok()); let messages = mock.describe_messages(); assert_eq!(messages.len(), 1); @@ -141,8 +166,8 @@ mod tests { #[tokio::test] async fn mock_describe_records_multiple_messages() { let mock = MockJjExecutor::new(); - mock.describe("first message").await.unwrap(); - mock.describe("second message").await.unwrap(); + mock.describe("@", "first message").await.unwrap(); + mock.describe("@", "second message").await.unwrap(); let messages = mock.describe_messages(); assert_eq!(messages.len(), 2); assert_eq!(messages[0], "first message"); @@ -153,7 +178,7 @@ mod tests { #[tokio::test] async fn mock_describe_returns_error() { let mock = MockJjExecutor::new().with_describe_response(Err(Error::RepositoryLocked)); - let result = mock.describe("test").await; + let result = mock.describe("@", "test").await; assert!(result.is_err()); assert!(matches!(result.unwrap_err(), Error::RepositoryLocked)); } @@ -164,7 +189,7 @@ mod tests { let mock = MockJjExecutor::new().with_describe_response(Err(Error::JjOperation { context: "transaction failed".to_string(), })); - let result = mock.describe("test").await; + let result = mock.describe("@", "test").await; assert!(result.is_err()); match result.unwrap_err() { Error::JjOperation { context } => { @@ -208,7 +233,7 @@ mod tests { #[tokio::test] async fn mock_describe_accepts_empty_message() { let mock = MockJjExecutor::new(); - let result = mock.describe("").await; + let result = mock.describe("@", "").await; assert!(result.is_ok()); assert_eq!(mock.describe_messages()[0], ""); } @@ -218,7 +243,7 @@ mod tests { async fn mock_describe_accepts_long_message() { let mock = MockJjExecutor::new(); let long_message = "a".repeat(1000); - let result = mock.describe(&long_message).await; + let result = mock.describe("@", &long_message).await; assert!(result.is_ok()); assert_eq!(mock.describe_messages()[0].len(), 1000); } diff --git a/src/jj/mod.rs b/src/jj/mod.rs index 848b4a5..636fe7f 100644 --- a/src/jj/mod.rs +++ b/src/jj/mod.rs @@ -14,7 +14,13 @@ pub trait JjExecutor: Send + Sync { async fn is_repository(&self) -> Result; /// Set the description of the current change - async fn describe(&self, message: &str) -> Result<(), Error>; + /// + /// The revset parameter should resolve to a single commit (e.g., + /// `"@"`, `"xs"`, bookmark name) + async fn describe(&self, revset: &str, message: &str) -> Result<(), Error>; + + /// Get the current description of a specific revision + async fn get_description(&self, revset: &str) -> Result; } #[cfg(test)] diff --git a/src/lib.rs b/src/lib.rs index 4c110e9..09835e6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,3 @@ -mod cli; mod commit; mod error; mod jj; diff --git a/src/main.rs b/src/main.rs index c34f42d..0e19867 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,6 +22,8 @@ fn error_to_exit_code(error: &Error) -> i32 { Error::NonInteractive => EXIT_ERROR, Error::FailedGettingCurrentDir => EXIT_ERROR, Error::FailedReadingConfig { .. } => EXIT_ERROR, + Error::RevsetResolutionError { .. } => EXIT_ERROR, + Error::MultipleRevisions { .. } => EXIT_ERROR, } } @@ -33,42 +35,38 @@ fn is_interactive_terminal() -> bool { #[tokio::main] async fn main() { - // Parse CLI arguments; --help and --version are handled automatically by clap - cli::Cli::parse(); - + let cli = cli::Cli::parse(); if !is_interactive_terminal() { eprintln!("❌ Error: jj-cz requires an interactive terminal (TTY)"); eprintln!(" This tool cannot be used in non-interactive mode or when piping input."); eprintln!(" Use --help for usage information."); process::exit(EXIT_ERROR); } - - // Create the jj executor - let executor = match JjLib::new() { + let executor = match JjLib::new().await { Ok(e) => e, Err(e) => { eprintln!("❌ Error: {}", e); process::exit(EXIT_ERROR); } }; - - // Create and run the workflow let workflow = CommitWorkflow::new(executor); - let result = workflow.run().await; + for revset in cli.revsets() { + let result = workflow.run_for_revset(revset).await; + handle_result(result); + } - // Handle the result - match result { - Ok(()) => { - println!("✅ Commit message applied successfully!"); - process::exit(EXIT_SUCCESS); - } - Err(Error::Cancelled) => { - println!("🟡 Operation cancelled by user."); - process::exit(EXIT_CANCELLED); - } - Err(e) => { - eprintln!("❌ Error: {}", e); - process::exit(error_to_exit_code(&e)); + fn handle_result(result: Result<(), Error>) { + match result { + Ok(()) => println!("✅ Commit message applied successfully!"), + Err(Error::Cancelled) => { + println!("🟡 Operation cancelled by user."); + process::exit(EXIT_CANCELLED); + } + Err(e) => { + eprintln!("❌ Error: {}", e); + process::exit(error_to_exit_code(&e)); + } } } + process::exit(EXIT_SUCCESS); } diff --git a/src/prompts/prompter.rs b/src/prompts/prompter.rs index 44b2e43..523baad 100644 --- a/src/prompts/prompter.rs +++ b/src/prompts/prompter.rs @@ -107,7 +107,7 @@ impl Prompter for RealPrompts { continue; } - // parse() only fails on empty — already handled above + // parse() only fails on empty - already handled above let Ok(desc) = Description::parse(trimmed) else { println!("❌ Description cannot be empty. Please provide a description."); continue; @@ -269,7 +269,7 @@ mod tests { } /// A single CJK character (display width 2) is padded as if it occupies 2 columns, - /// not 1 — so the right-hand padding is 70 spaces, not 71 + /// not 1 - so the right-hand padding is 70 spaces, not 71 #[test] fn format_message_box_single_cjk_char() { let result = format_message_box("字"); diff --git a/src/prompts/workflow.rs b/src/prompts/workflow.rs index 1d21371..787f9c1 100644 --- a/src/prompts/workflow.rs +++ b/src/prompts/workflow.rs @@ -54,10 +54,12 @@ impl CommitWorkflow { /// - User cancels the workflow /// - Repository operation fails /// - Message validation fails - pub async fn run(&self) -> Result<(), Error> { + pub async fn run_for_revset(&self, revset: &str) -> Result<(), Error> { if !self.executor.is_repository().await? { return Err(Error::NotARepository); } + // For future reference + let _existing_desc = self.executor.get_description(revset).await.ok(); let commit_type = self.type_selection()?; loop { let scope = self.scope_input()?; @@ -67,7 +69,7 @@ impl CommitWorkflow { match self.preview_and_confirm(commit_type, scope, description, breaking_change, body) { Ok(conventional_commit) => { self.executor - .describe(&conventional_commit.to_string()) + .describe(revset, &conventional_commit.to_string()) .await?; return Ok(()); } @@ -205,7 +207,7 @@ mod tests { async fn workflow_returns_not_a_repository() { let mock = MockJjExecutor::new().with_is_repo_response(Ok(false)); let workflow = CommitWorkflow::new(mock); - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!(result.is_err()); assert!(matches!(result.unwrap_err(), Error::NotARepository)); } @@ -215,7 +217,7 @@ mod tests { async fn workflow_returns_repository_error() { let mock = MockJjExecutor::new().with_is_repo_response(Err(Error::NotARepository)); let workflow = CommitWorkflow::new(mock); - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!(result.is_err()); assert!(matches!(result.unwrap_err(), Error::NotARepository)); } @@ -285,7 +287,7 @@ mod tests { // Verify the mock behaves as expected assert!(mock.is_repository().await.is_ok()); - assert!(mock.describe("test").await.is_err()); + assert!(mock.describe("@", "test").await.is_err()); // Also test with a working mock let working_mock = MockJjExecutor::new(); @@ -323,7 +325,7 @@ mod tests { let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); // Run the workflow - should succeed - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!(result.is_ok()); } @@ -334,7 +336,7 @@ mod tests { let mock_prompts = MockPrompts::new().with_error(Error::Cancelled); let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!(result.is_err()); assert!(matches!(result.unwrap_err(), Error::Cancelled)); @@ -353,7 +355,7 @@ mod tests { .with_confirm(false); // User cancels at confirmation let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!(result.is_err()); assert!(matches!(result.unwrap_err(), Error::Cancelled)); @@ -384,7 +386,7 @@ mod tests { // Clone before moving into workflow so we can inspect emitted messages after let mock_prompts_handle = mock_prompts.clone(); let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; // Should succeed after the retry assert!( @@ -421,7 +423,7 @@ mod tests { )); let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!(result.is_err()); assert!(matches!(result.unwrap_err(), Error::InvalidScope(_))); @@ -440,7 +442,7 @@ mod tests { )); let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!(result.is_err()); assert!(matches!(result.unwrap_err(), Error::InvalidDescription(_))); @@ -479,7 +481,7 @@ mod tests { MockJjExecutor::new().with_is_repo_response(Ok(true)), mock_prompts, ); - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!(result.is_ok(), "Failed for commit type: {:?}", commit_type); } } @@ -503,7 +505,7 @@ mod tests { mock_prompts, ); { - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!(result.is_ok()); } @@ -521,7 +523,7 @@ mod tests { mock_prompts, ); { - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!(result.is_ok()); } } @@ -621,7 +623,7 @@ mod tests { .with_confirm(true); let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!( result.is_ok(), @@ -716,7 +718,7 @@ mod tests { .with_confirm(true); let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!( result.is_ok(), @@ -748,7 +750,7 @@ mod tests { .with_confirm(true); let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); - let result: Result<(), Error> = workflow.run().await; + let result: Result<(), Error> = workflow.run_for_revset("@").await; assert!( result.is_ok(), diff --git a/tests/cli.rs b/tests/cli.rs index 8ab6e19..d27468d 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1,28 +1,13 @@ use assert_fs::TempDir; #[cfg(feature = "test-utils")] -use jj_cz::{Body, BreakingChange, CommitType, Description, MockPrompts, Scope}; +use jj_cz::{Body, BreakingChange, CommitType, Description, MockJjExecutor, MockPrompts, Scope}; use jj_cz::{CommitWorkflow, Error, JjLib}; -#[cfg(feature = "test-utils")] -use jj_lib::{config::StackedConfig, settings::UserSettings, workspace::Workspace}; - -/// Helper to initialize a temporary jj repository using jj-lib directly (no CLI required) -#[cfg(feature = "test-utils")] -async fn init_jj_repo(temp_dir: &TempDir) { - let config = StackedConfig::with_defaults(); - let settings = UserSettings::from_config(config).expect("Failed to create settings"); - Workspace::init_internal_git(&settings, temp_dir.path()) - .await - .expect("Failed to initialize jj repository"); -} #[cfg(feature = "test-utils")] #[tokio::test] async fn test_happy_path_integration() { // T037: Happy path integration test - let temp_dir = TempDir::new().unwrap(); - init_jj_repo(&temp_dir).await; - - // Create mock prompts that simulate a successful workflow + let mock_executor = MockJjExecutor::new(); let mock_prompts = MockPrompts::new() .with_commit_type(CommitType::Feat) .with_scope(Scope::empty()) @@ -31,13 +16,9 @@ async fn test_happy_path_integration() { .with_body(Body::default()) .with_confirm(true); - // Create a mock executor that tracks calls - let executor = JjLib::with_working_dir(temp_dir.path()); - let workflow = CommitWorkflow::with_prompts(executor, mock_prompts); + let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); + let result = workflow.run_for_revset("@").await; - let result = workflow.run().await; - - // The workflow should complete successfully assert!( result.is_ok(), "Workflow should complete successfully: {:?}", @@ -47,17 +28,11 @@ async fn test_happy_path_integration() { #[tokio::test] async fn test_not_in_repo() { - // T038: Not-in-repo integration test + // T038: Not-in-repo integration test - with_working_dir itself returns the error let temp_dir = TempDir::new().unwrap(); - // Don't initialize jj repo - // Create executor with the temp directory (which is not a jj repo) - let executor = JjLib::with_working_dir(temp_dir.path()); - let workflow = CommitWorkflow::new(executor); + let result = JjLib::with_working_dir(temp_dir.path()).await; - let result = workflow.run().await; - - // Should fail with NotARepository error assert!(matches!(result, Err(Error::NotARepository))); } @@ -77,9 +52,13 @@ async fn test_cancellation() { Ok(true) } - async fn describe(&self, _message: &str) -> Result<(), Error> { + async fn describe(&self, _revset: &str, _message: &str) -> Result<(), Error> { Err(Error::Cancelled) } + + async fn get_description(&self, _revset: &str) -> Result { + Ok(String::new()) + } } let executor = CancelMock; @@ -92,7 +71,7 @@ async fn test_cancellation() { .with_confirm(true); let workflow = CommitWorkflow::with_prompts(executor, mock_prompts); - let result = workflow.run().await; + let result = workflow.run_for_revset("@").await; // Should fail with Cancelled error assert!(matches!(result, Err(Error::Cancelled)));