feat: implement --new flag

The new `--new` or `-n` flag allows to create a new revision after the
single revision being described. Running `jj-cz --new` is the
equivalent of running `jj-cz @ && jj new`. Running `jj-cz --new xs` is
the equivalent of running `jj-cz xs && jj new xs`.

Passing several revisions to `jj-cz` with the `--new` flag will result
in an error.

Refs: #6
This commit is contained in:
2026-06-07 12:37:18 +02:00
parent 0e6b559d00
commit e45130d31b
9 changed files with 446 additions and 19 deletions
+95
View File
@@ -18,6 +18,10 @@ pub struct Cli {
/// The revision(s) whose description to edit (default: @) /// The revision(s) whose description to edit (default: @)
#[arg(value_name = "REVSETS")] #[arg(value_name = "REVSETS")]
revsets: Vec<String>, revsets: Vec<String>,
/// Create a new child revision after editing the description
#[arg(short, long)]
new: bool,
} }
impl Cli { impl Cli {
@@ -29,4 +33,95 @@ impl Cli {
self.revsets.iter().map(|s| s.as_str()).collect() self.revsets.iter().map(|s| s.as_str()).collect()
} }
} }
pub fn create_new(&self) -> bool {
self.new
}
pub fn validate(&self) -> Result<(), jj_cz::Error> {
if self.new && self.revsets().len() > 1 {
Err(jj_cz::Error::NewFlagWithMultipleRevisions)
} else {
Ok(())
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use clap::Parser;
#[test]
fn revsets_defaults_to_at() {
let cli = Cli::parse_from(["jj-cz"]);
assert_eq!(cli.revsets(), vec!["@"]);
}
#[test]
fn revsets_returns_provided_values() {
let cli = Cli::parse_from(["jj-cz", "abc", "def"]);
let revsets = cli.revsets();
assert_eq!(revsets, vec!["abc", "def"]);
}
#[test]
fn revsets_single_revset() {
let cli = Cli::parse_from(["jj-cz", "xyz"]);
assert_eq!(cli.revsets(), vec!["xyz"]);
}
#[test]
fn create_new_returns_false_by_default() {
let cli = Cli::parse_from(["jj-cz"]);
assert!(!cli.create_new());
}
#[test]
fn create_new_returns_true_with_flag() {
let cli = Cli::parse_from(["jj-cz", "--new"]);
assert!(cli.create_new());
}
#[test]
fn create_new_returns_true_with_short_flag() {
let cli = Cli::parse_from(["jj-cz", "-n"]);
assert!(cli.create_new());
}
#[test]
fn validate_ok_with_no_args() {
let cli = Cli::parse_from(["jj-cz"]);
assert!(cli.validate().is_ok());
}
#[test]
fn validate_ok_with_new_and_single_revset() {
let cli = Cli::parse_from(["jj-cz", "--new", "@"]);
assert!(cli.validate().is_ok());
}
#[test]
fn validate_ok_with_multiple_revsets_no_new() {
let cli = Cli::parse_from(["jj-cz", "abc", "def"]);
assert!(cli.validate().is_ok());
}
#[test]
fn validate_err_with_new_and_multiple_revsets() {
let cli = Cli::parse_from(["jj-cz", "--new", "abc", "def"]);
let result = cli.validate();
assert!(result.is_err());
assert!(matches!(
result.unwrap_err(),
jj_cz::Error::NewFlagWithMultipleRevisions
));
}
#[test]
fn cli_derives_debug() {
let cli = Cli::parse_from(["jj-cz", "--new", "@"]);
let debug = format!("{:?}", cli);
assert!(debug.contains("Cli"));
}
} }
+2
View File
@@ -31,6 +31,8 @@ pub enum Error {
RevsetResolutionError { revset: String, context: String }, RevsetResolutionError { revset: String, context: String },
#[error("Revision set '{revset}' resolves to multiple commits; specify a single revision")] #[error("Revision set '{revset}' resolves to multiple commits; specify a single revision")]
MultipleRevisions { revset: String }, MultipleRevisions { revset: String },
#[error("--new cannot be used with multiple revisions")]
NewFlagWithMultipleRevisions,
} }
impl From<ScopeError> for Error { impl From<ScopeError> for Error {
+27
View File
@@ -263,6 +263,33 @@ impl JjExecutor for JjLib {
})?; })?;
Ok(commit.description().trim_end().to_string()) Ok(commit.description().trim_end().to_string())
} }
async fn new_revision(&self, revset: &str) -> Result<(), Error> {
let commit_id = self.get_commit_id(revset).await?;
let repo = self.repo.lock()?.clone();
let mut tx = repo.start_transaction();
let parent_commit =
tx.repo()
.store()
.get_commit(&commit_id)
.map_err(|e| Error::JjOperation {
context: e.to_string(),
})?;
tx.repo_mut()
.check_out(self.workspace_name.clone(), &parent_commit)
.await
.map_err(|e| Error::JjOperation {
context: e.to_string(),
})?;
let new_repo =
tx.commit("jj-cz: create new revision")
.await
.map_err(|e| Error::JjOperation {
context: e.to_string(),
})?;
*self.repo.lock()? = new_repo;
Ok(())
}
} }
#[cfg(test)] #[cfg(test)]
+88 -5
View File
@@ -11,18 +11,22 @@ use std::sync::{Mutex, atomic::AtomicBool};
/// Mock implementation of JjExecutor for testing /// Mock implementation of JjExecutor for testing
#[derive(Debug)] #[derive(Debug)]
pub struct MockJjExecutor { pub struct MockJjExecutor {
/// Response to return from is_repository() /// Response to return from `is_repository()`
is_repo_response: Result<bool, Error>, is_repo_response: Result<bool, Error>,
/// Response to return from describe() /// Response to return from `describe()`
describe_response: Result<(), Error>, describe_response: Result<(), Error>,
/// Track described revsets /// Track described revsets
described_revsets: Mutex<Vec<String>>, described_revsets: Mutex<Vec<String>>,
/// Track response to return from get_description() /// Track response to return from `get_description()`
get_description_response: Result<String, Error>, get_description_response: Result<String, Error>,
/// Track calls to is_repository() /// Track calls to `is_repository()`
is_repo_called: AtomicBool, is_repo_called: AtomicBool,
/// Track calls to describe() with the message passed /// Track calls to `describe()` with the message passed
describe_calls: Mutex<Vec<String>>, describe_calls: Mutex<Vec<String>>,
/// Track response to return from `new_revision()`
new_revision_response: Result<(), Error>,
/// Track calls to `new_revision()`
new_revision_calls: Mutex<Vec<String>>,
} }
impl Default for MockJjExecutor { impl Default for MockJjExecutor {
@@ -34,6 +38,8 @@ impl Default for MockJjExecutor {
get_description_response: Ok(String::new()), get_description_response: Ok(String::new()),
is_repo_called: AtomicBool::new(false), is_repo_called: AtomicBool::new(false),
describe_calls: Mutex::new(Vec::new()), describe_calls: Mutex::new(Vec::new()),
new_revision_response: Ok(()),
new_revision_calls: Mutex::new(Vec::new()),
} }
} }
} }
@@ -66,6 +72,15 @@ impl MockJjExecutor {
pub fn describe_messages(&self) -> Vec<String> { pub fn describe_messages(&self) -> Vec<String> {
self.describe_calls.lock().unwrap().clone() self.describe_calls.lock().unwrap().clone()
} }
pub fn with_new_revision_response(mut self, response: Result<(), Error>) -> Self {
self.new_revision_response = response;
self
}
pub fn new_revision_calls(&self) -> Vec<String> {
self.new_revision_calls.lock().unwrap().clone()
}
} }
#[async_trait(?Send)] #[async_trait(?Send)]
@@ -97,6 +112,17 @@ impl JjExecutor for MockJjExecutor {
async fn get_description(&self, _revset: &str) -> Result<String, Error> { async fn get_description(&self, _revset: &str) -> Result<String, Error> {
self.get_description_response.clone() self.get_description_response.clone()
} }
async fn new_revision(&self, revset: &str) -> Result<(), Error> {
self.new_revision_calls
.lock()
.unwrap()
.push(revset.to_string());
match &self.new_revision_response {
Ok(()) => Ok(()),
Err(e) => Err(e.clone()),
}
}
} }
#[cfg(test)] #[cfg(test)]
@@ -245,4 +271,61 @@ mod tests {
mock.is_repository().await.unwrap(); mock.is_repository().await.unwrap();
assert!(mock.was_is_repo_called()); assert!(mock.was_is_repo_called());
} }
/// Test mock new_revision() records the revset
#[tokio::test]
async fn mock_new_revision_records_revset() {
let mock = MockJjExecutor::new();
let result = mock.new_revision("@").await;
assert!(result.is_ok());
let calls = mock.new_revision_calls();
assert_eq!(calls.len(), 1);
assert_eq!(calls[0], "@");
}
/// Test mock new_revision() records multiple calls
#[tokio::test]
async fn mock_new_revision_records_multiple_calls() {
let mock = MockJjExecutor::new();
mock.new_revision("@").await.unwrap();
mock.new_revision("abc").await.unwrap();
mock.new_revision("xyz").await.unwrap();
let calls = mock.new_revision_calls();
assert_eq!(calls.len(), 3);
assert_eq!(calls[0], "@");
assert_eq!(calls[1], "abc");
assert_eq!(calls[2], "xyz");
}
/// Test mock new_revision() returns configured error
#[tokio::test]
async fn mock_new_revision_returns_error() {
let mock = MockJjExecutor::new().with_new_revision_response(Err(Error::RepositoryLocked));
let result = mock.new_revision("@").await;
assert!(result.is_err());
assert!(matches!(result.unwrap_err(), Error::RepositoryLocked));
}
/// Test mock new_revision() records revset even on error
#[tokio::test]
async fn mock_new_revision_records_revset_on_error() {
let mock = MockJjExecutor::new().with_new_revision_response(Err(Error::JjOperation {
context: "failed".to_string(),
}));
let result = mock.new_revision("abc").await;
assert!(result.is_err());
let calls = mock.new_revision_calls();
assert_eq!(calls.len(), 1);
assert_eq!(calls[0], "abc");
}
/// Test mock new_revision() can be inspected after success
#[tokio::test]
async fn mock_new_revision_returns_ok_and_tracks_revset() {
let mock = MockJjExecutor::new();
let result = mock.new_revision("my-feature").await;
assert!(result.is_ok());
let calls = mock.new_revision_calls();
assert_eq!(calls, vec!["my-feature"]);
}
} }
+5
View File
@@ -21,6 +21,11 @@ pub trait JjExecutor: Send + Sync {
/// Get the current description of a specific revision /// Get the current description of a specific revision
async fn get_description(&self, revset: &str) -> Result<String, Error>; async fn get_description(&self, revset: &str) -> Result<String, Error>;
/// Create a new empty child revision parented on `revset`.
///
/// Equivalent to `jj new <revset>`
async fn new_revision(&self, revset: &str) -> Result<(), Error>;
} }
#[cfg(test)] #[cfg(test)]
+1 -1
View File
@@ -1,5 +1,5 @@
mod commit; mod commit;
mod error; pub mod error;
mod jj; mod jj;
mod prompts; mod prompts;
+17 -13
View File
@@ -10,7 +10,7 @@ const EXIT_CANCELLED: i32 = 130; // Same as SIGINT (Ctrl+C)
const EXIT_ERROR: i32 = 1; const EXIT_ERROR: i32 = 1;
/// Map application errors to appropriate exit codes /// Map application errors to appropriate exit codes
fn error_to_exit_code(error: &Error) -> i32 { fn error_to_exit_code(error: Error) -> i32 {
match error { match error {
Error::Cancelled => EXIT_CANCELLED, Error::Cancelled => EXIT_CANCELLED,
Error::NotARepository => EXIT_ERROR, Error::NotARepository => EXIT_ERROR,
@@ -24,6 +24,7 @@ fn error_to_exit_code(error: &Error) -> i32 {
Error::FailedReadingConfig { .. } => EXIT_ERROR, Error::FailedReadingConfig { .. } => EXIT_ERROR,
Error::RevsetResolutionError { .. } => EXIT_ERROR, Error::RevsetResolutionError { .. } => EXIT_ERROR,
Error::MultipleRevisions { .. } => EXIT_ERROR, Error::MultipleRevisions { .. } => EXIT_ERROR,
Error::NewFlagWithMultipleRevisions => EXIT_ERROR,
} }
} }
@@ -34,25 +35,26 @@ fn is_interactive_terminal() -> bool {
} }
#[tokio::main] #[tokio::main]
async fn main() { async fn main() -> Result<(), ()> {
let cli = cli::Cli::parse(); let cli = cli::Cli::parse();
cli.validate().map_err(exit_on_error)?;
if !is_interactive_terminal() { if !is_interactive_terminal() {
eprintln!("❌ Error: jj-cz requires an interactive terminal (TTY)"); eprintln!("❌ Error: jj-cz requires an interactive terminal (TTY)");
eprintln!(" This tool cannot be used in non-interactive mode or when piping input."); eprintln!(" This tool cannot be used in non-interactive mode or when piping input.");
eprintln!(" Use --help for usage information."); eprintln!(" Use --help for usage information.");
process::exit(EXIT_ERROR); process::exit(EXIT_ERROR);
} }
let executor = match JjLib::new().await { let executor = JjLib::new().await.map_err(exit_on_error)?;
Ok(e) => e,
Err(e) => {
eprintln!("❌ Error: {}", e);
process::exit(EXIT_ERROR);
}
};
let workflow = CommitWorkflow::new(executor); let workflow = CommitWorkflow::new(executor);
for revset in cli.revsets() { for revset in cli.revsets() {
let result = workflow.run_for_revset(revset).await; let result = workflow.run_for_revset(revset).await;
handle_result(result); handle_result(result);
if cli.create_new() {
println!("Creating a new revision after {revset}");
workflow.new_revision(revset).await.map_err(exit_on_error)?;
}
} }
fn handle_result(result: Result<(), Error>) { fn handle_result(result: Result<(), Error>) {
@@ -62,11 +64,13 @@ async fn main() {
println!("🟡 Operation cancelled by user."); println!("🟡 Operation cancelled by user.");
process::exit(EXIT_CANCELLED); process::exit(EXIT_CANCELLED);
} }
Err(e) => { Err(e) => exit_on_error(e),
eprintln!("❌ Error: {}", e);
process::exit(error_to_exit_code(&e));
}
} }
} }
process::exit(EXIT_SUCCESS); process::exit(EXIT_SUCCESS);
} }
fn exit_on_error(e: Error) {
eprintln!("❌ Error: {}", e);
process::exit(error_to_exit_code(e));
}
+59
View File
@@ -184,6 +184,10 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
Err(Error::Cancelled) Err(Error::Cancelled)
} }
} }
pub async fn new_revision(&self, revset: &str) -> Result<(), Error> {
self.executor.new_revision(revset).await
}
} }
#[cfg(test)] #[cfg(test)]
@@ -762,4 +766,59 @@ mod tests {
assert_eq!(messages.len(), 1); assert_eq!(messages.len(), 1);
assert_eq!(messages[0], "fix: fix crash"); assert_eq!(messages[0], "fix: fix crash");
} }
/// Test workflow new_revision() records the revset
#[tokio::test]
async fn workflow_new_revision_records_revset() {
let mock_executor = MockJjExecutor::new();
let workflow = CommitWorkflow::new(mock_executor);
let result = workflow.new_revision("@").await;
assert!(result.is_ok());
let calls = workflow.executor.new_revision_calls();
assert_eq!(calls, vec!["@"]);
}
/// Test workflow new_revision() propagates executor errors
#[tokio::test]
async fn workflow_new_revision_propagates_error() {
let mock_executor =
MockJjExecutor::new().with_new_revision_response(Err(Error::RepositoryLocked));
let workflow = CommitWorkflow::new(mock_executor);
let result = workflow.new_revision("@").await;
assert!(result.is_err());
assert!(matches!(result.unwrap_err(), Error::RepositoryLocked));
}
/// Test workflow run_for_revset() followed by new_revision() records both
///
/// This mirrors the actual usage pattern in main.rs.
#[tokio::test]
async fn workflow_describe_then_new_revision() {
let mock_executor = MockJjExecutor::new().with_is_repo_response(Ok(true));
let mock_prompts = MockPrompts::new()
.with_commit_type(CommitType::Feat)
.with_scope(Scope::empty())
.with_description(Description::parse("add feature").unwrap())
.with_breaking_change(BreakingChange::No)
.with_body(Body::default())
.with_confirm(true);
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
workflow.run_for_revset("@").await.expect("describe failed");
workflow
.new_revision("@")
.await
.expect("new_revision failed");
let messages = workflow.executor.describe_messages();
assert_eq!(messages.len(), 1);
assert!(messages[0].contains("feat:"));
let calls = workflow.executor.new_revision_calls();
assert_eq!(calls, vec!["@"]);
}
} }
+152
View File
@@ -135,3 +135,155 @@ fn test_jj_operation_context() {
panic!("Expected JjOperation variant"); panic!("Expected JjOperation variant");
} }
} }
/// Test conversion from std::io::Error
#[test]
fn test_from_io_error() {
let io_err = std::io::Error::new(std::io::ErrorKind::NotFound, "file not found");
let error: Error = io_err.into();
assert!(matches!(error, Error::FailedGettingCurrentDir));
}
/// Test conversion from std::sync::PoisonError
#[test]
fn test_from_poison_error() {
let mutex = std::sync::Mutex::new(());
// Poison the mutex by panicking while holding the lock
let poison_err = std::panic::catch_unwind(|| {
let _guard = mutex.lock().unwrap();
panic!("deliberate panic");
});
assert!(poison_err.is_err());
// Now lock should fail with PoisonError
let result = mutex.lock();
assert!(result.is_err());
let error: Error = result.unwrap_err().into();
assert!(matches!(error, Error::JjOperation { .. }));
assert_eq!(
format!("{}", error),
"Repository operation failed: internal lock poisoned"
);
}
/// Test from_revset_evaluation_error constructs RevsetResolutionError
#[test]
fn test_from_revset_evaluation_error() {
let underlying = std::io::Error::new(std::io::ErrorKind::Other, "store failure");
let eval_err = jj_lib::revset::RevsetEvaluationError::Other(Box::new(underlying));
let error = Error::from_revset_evaluation_error("@", eval_err);
assert!(matches!(error, Error::RevsetResolutionError { .. }));
let description = format!("{}", error);
assert!(description.contains("@"));
assert!(description.contains("store failure"));
}
/// Test from_revset_resolution_error constructs RevsetResolutionError
#[test]
fn test_from_revset_resolution_error() {
let resolution_err = jj_lib::revset::RevsetResolutionError::NoSuchRevision {
name: "nonexistent".to_string(),
candidates: Vec::new(),
};
let error = Error::from_revset_resolution_error("@", resolution_err);
assert!(matches!(error, Error::RevsetResolutionError { .. }));
let description = format!("{}", error);
assert!(description.contains("@"));
assert!(description.contains("nonexistent"));
}
/// Test NewFlagWithMultipleRevisions error display
#[test]
fn test_new_flag_with_multiple_revisions() {
let error = Error::NewFlagWithMultipleRevisions;
assert_eq!(
format!("{}", error),
"--new cannot be used with multiple revisions"
);
}
/// Test NonInteractive error display
#[test]
fn test_non_interactive() {
let error = Error::NonInteractive;
assert_eq!(format!("{}", error), "Non-interactive terminal detected");
}
/// Test FailedReadingConfig error display
#[test]
fn test_failed_reading_config() {
let error = Error::FailedReadingConfig {
context: "config parse error".to_string(),
};
let description = format!("{}", error);
assert!(description.contains("config parse error"));
}
/// Test MultipleRevisions error display
#[test]
fn test_multiple_revisions() {
let error = Error::MultipleRevisions {
revset: "abc | def".to_string(),
};
let description = format!("{}", error);
assert!(description.contains("abc | def"));
assert!(description.contains("multiple commits"));
}
/// Test RepositoryLocked error display
#[test]
fn test_repository_locked() {
let error = Error::RepositoryLocked;
assert_eq!(
format!("{}", error),
"Repository is locked by another process"
);
}
/// Test FailedGettingCurrentDir error display
#[test]
fn test_failed_getting_current_dir() {
let error = Error::FailedGettingCurrentDir;
assert_eq!(format!("{}", error), "Could not get current directory");
}
/// Test error matching on all variants
#[test]
fn test_error_matching_all_variants() {
let variants: Vec<Error> = vec![
Error::InvalidScope("s".into()),
Error::InvalidDescription("d".into()),
Error::InvalidCommitMessage("m".into()),
Error::NotARepository,
Error::JjOperation {
context: "c".into(),
},
Error::RepositoryLocked,
Error::FailedGettingCurrentDir,
Error::FailedReadingConfig {
context: "c".into(),
},
Error::Cancelled,
Error::NonInteractive,
Error::RevsetResolutionError {
revset: "@".into(),
context: "c".into(),
},
Error::MultipleRevisions { revset: "@".into() },
Error::NewFlagWithMultipleRevisions,
];
// All variants should be displayable without panicking
for variant in &variants {
let _ = format!("{}", variant);
let _ = format!("{:?}", variant);
}
// Verify all variants can be cloned
let cloned: Vec<Error> = variants.iter().map(|e| e.clone()).collect();
assert_eq!(variants.len(), cloned.len());
for (original, clone) in variants.iter().zip(cloned.iter()) {
assert_eq!(original, clone);
}
}