7 Commits

Author SHA1 Message Date
3da214ae4c refactor(prompter): simplify commit type selection
Some checks failed
Publish Docker Images / coverage-and-sonar (push) Failing after 14m51s
2026-04-05 16:41:11 +02:00
64652fc81d fix(scope): no new string allocation to count characters 2026-04-05 16:41:11 +02:00
0ef1f61613 feat(errors): preserve jj-emitted errors when loading config 2026-04-05 16:41:11 +02:00
52f0667777 refactor(BreakingChange): rename method ignore to is_absent
Method `ignore` did not carry its meaning well by the way it is named.
This commit renames it to `is_absent` to clearly state this method
returns whether we have a breaking change.
2026-04-05 16:41:11 +02:00
61288e8f49 refactor(workflow): remove unnecessary async declarations 2026-04-05 16:41:11 +02:00
1c983f3a8d refactor(nix): simplify package declaration 2026-04-05 15:56:24 +02:00
CI Bot
a5b2bc41aa chore(release): bump version to 1.0.1-dev [skip ci] 2026-03-25 15:06:26 +00:00
13 changed files with 133 additions and 181 deletions

2
Cargo.lock generated
View File

@@ -1650,7 +1650,7 @@ dependencies = [
[[package]]
name = "jj-cz"
version = "1.0.0"
version = "1.0.1-dev"
dependencies = [
"assert_cmd",
"assert_fs",

View File

@@ -1,6 +1,6 @@
[package]
name = "jj-cz"
version = "1.0.0"
version = "1.0.1-dev"
description = "Conventional commits for Jujutsu"
edition = "2024"
publish = true

View File

@@ -41,36 +41,22 @@
};
in {
formatter = alejandra.defaultPackage.${system};
packages =
(import ./nix/package.nix {inherit pkgs rustPlatform;})
// {
windows = let
mingwPkgs = pkgs.pkgsCross.mingwW64;
rustWindows = pkgs.rust-bin.stable.latest.default.override {
targets = ["x86_64-pc-windows-gnu"];
};
rustPlatformWindows = mingwPkgs.makeRustPlatform {
cargo = rustWindows;
rustc = rustWindows;
};
cargoToml = builtins.fromTOML (builtins.readFile ./Cargo.toml);
in
rustPlatformWindows.buildRustPackage {
pname = cargoToml.package.name;
version = cargoToml.package.version;
src = pkgs.lib.cleanSource ./.;
cargoLock.lockFile = ./Cargo.lock;
nativeBuildInputs = [pkgs.upx];
doCheck = false;
meta = {
description = "Conventional commits for Jujutsu";
homepage = "https://labs.phundrak.com/phundrak/jj-cz";
};
postBuild = ''
${pkgs.upx}/bin/upx target/*/release/jj-cz.exe
'';
};
packages = let
nativeRustVersion = pkgs.rust-bin.stable.latest.default;
nativeRustPlatform = pkgs.makeRustPlatform {
cargo = nativeRustVersion;
rustc = nativeRustVersion;
};
mingwPkgs = pkgs.pkgsCross.mingwW64;
windowsRustVersion = pkgs.rust-bin.stable.latest.default.override {
targets = ["x86_64-pc-windows-gnu"];
};
windowsRustPlatform = mingwPkgs.makeRustPlatform {
cargo = windowsRustVersion;
rustc = windowsRustVersion;
};
in
import ./nix/package.nix {inherit pkgs nativeRustPlatform windowsRustPlatform;};
devShell = import ./nix/shell.nix {
inherit inputs pkgs rustVersion;
};

View File

@@ -1,26 +1,36 @@
{
pkgs,
rustPlatform,
nativeRustPlatform,
windowsRustPlatform,
...
}: let
cargoToml = fromTOML (builtins.readFile ../Cargo.toml);
name = cargoToml.package.name;
version = cargoToml.package.version;
rustBuild = rustPlatform.buildRustPackage {
buildArgs = {
pname = name;
inherit version;
src = pkgs.lib.cleanSource ../.;
cargoLock.lockFile = ../Cargo.lock;
nativeBuildInputs = [pkgs.upx];
useNextest = true;
meta = {
description = "Conventional commits for Jujutsu";
homepage = "https://labs.phundrak.com/phundrak/jj-cz";
inherit (cargoToml.package) description homepage;
};
postBuild = ''
${pkgs.upx}/bin/upx target/*/release/${name}
'';
};
nativeBuild =
nativeRustPlatform.buildRustPackage buildArgs
// {
postBuild = "${pkgs.upx}/bin/upx target/*/release/${name}";
};
windowsBuild =
windowsRustPlatform.buildRustPackage buildArgs
// {
postBuild = "${pkgs.upx}/bin/upx target/*/release/${name}.exe";
};
in {
default = rustBuild;
default = nativeBuild;
windows = windowsBuild;
}

View File

@@ -31,7 +31,7 @@ pub enum BreakingChange {
}
impl BreakingChange {
pub fn ignore(&self) -> bool {
pub fn is_absent(&self) -> bool {
matches!(self, BreakingChange::No)
}

View File

@@ -76,7 +76,7 @@ impl ConventionalCommit {
pub fn first_line_len(&self) -> usize {
self.commit_type.len()
+ self.scope.header_segment_len()
+ if self.breaking_change.ignore() { 0 } else { 1 }
+ if self.breaking_change.is_absent() { 0 } else { 1 }
+ 2 // ": "
+ self.description.len()
}

View File

@@ -61,7 +61,11 @@ impl Scope {
/// Returns the visible length of the header segment
pub fn header_segment_len(&self) -> usize {
self.header_segment().chars().count()
if self.is_empty() {
0
} else {
self.0.chars().count() + 2
}
}
}

View File

@@ -18,8 +18,8 @@ pub enum Error {
RepositoryLocked,
#[error("Could not get current directory")]
FailedGettingCurrentDir,
#[error("Could not load Jujutsu configuration")]
FailedReadingConfig,
#[error("Could not load Jujutsu configuration: {context}")]
FailedReadingConfig { context: String },
// Application errors
#[error("Operation cancelled by user")]
Cancelled,

View File

@@ -41,16 +41,22 @@ impl JjLib {
let mut config = StackedConfig::with_defaults();
for path in Self::user_config_paths() {
if path.is_dir() {
config
.load_dir(ConfigSource::User, &path)
.map_err(|_| Error::FailedReadingConfig)?;
config.load_dir(ConfigSource::User, &path).map_err(|e| {
Error::FailedReadingConfig {
context: e.to_string(),
}
})?;
} else if path.exists() {
config
.load_file(ConfigSource::User, path)
.map_err(|_| Error::FailedReadingConfig)?;
config.load_file(ConfigSource::User, path).map_err(|e| {
Error::FailedReadingConfig {
context: e.to_string(),
}
})?;
}
}
UserSettings::from_config(config).map_err(|_| Error::FailedReadingConfig)
UserSettings::from_config(config).map_err(|e| Error::FailedReadingConfig {
context: e.to_string(),
})
}
/// Resolves user config file paths following the same logic as the jj CLI:

View File

@@ -21,7 +21,7 @@ fn error_to_exit_code(error: &Error) -> i32 {
Error::InvalidCommitMessage(_) => EXIT_ERROR,
Error::NonInteractive => EXIT_ERROR,
Error::FailedGettingCurrentDir => EXIT_ERROR,
Error::FailedReadingConfig => EXIT_ERROR,
Error::FailedReadingConfig { .. } => EXIT_ERROR,
}
}

View File

@@ -17,7 +17,7 @@ use crate::{
///
/// Implement this trait to supply a custom front-end (interactive TUI, mock,
/// headless, etc.) to [`CommitWorkflow`](super::CommitWorkflow).
pub trait Prompter: Send + Sync {
pub trait Prompter {
/// Prompt the user to select a commit type
fn select_commit_type(&self) -> Result<CommitType, Error>;
@@ -66,67 +66,32 @@ pub struct RealPrompts;
impl Prompter for RealPrompts {
fn select_commit_type(&self) -> Result<CommitType, Error> {
use inquire::Select;
let options: Vec<_> = CommitType::all()
.iter()
.map(|ct| format!("{}: {}", ct, ct.description()))
.collect();
let answer = Select::new("Select commit type:", options)
inquire::Select::new("Select commit type:", CommitType::all().to_vec())
.with_page_size(11)
.with_help_message(
"Use arrow keys to navigate, Enter to select. See https://www.conventionalcommits.org/ for details.",
)
.with_formatter(&|option| format!("{}: {}", option.value.as_str(), option.value.description()))
.prompt()
.map_err(|_| Error::Cancelled)?;
// Extract the commit type from the selected option
let selected_type = answer
.split(':')
.next()
.ok_or_else(|| Error::JjOperation {
context: "Failed to parse selected commit type".to_string(),
})?
.trim();
CommitType::all()
.iter()
.find(|ct| ct.as_str() == selected_type)
.copied()
.ok_or_else(|| Error::JjOperation {
context: format!("Unknown commit type: {}", selected_type),
})
.map_err(|_| Error::Cancelled)
}
fn input_scope(&self) -> Result<Scope, Error> {
use inquire::Text;
let answer = Text::new("Enter scope (optional):")
let answer = inquire::Text::new("Enter scope (optional):")
.with_help_message(
"Scope is optional. If provided, it should be a noun representing the section of code affected (e.g., 'api', 'ui', 'config'). Max 30 characters.",
)
.with_placeholder("Leave empty if no scope")
.prompt_skippable()
.map_err(|_| Error::Cancelled)?;
// Empty input is valid (no scope)
let answer_str = match answer {
Some(s) => s,
None => return Ok(Scope::empty()),
};
if answer_str.trim().is_empty() {
return Ok(Scope::empty());
match answer {
Some(s) if s.trim().is_empty() => Ok(Scope::empty()),
Some(s) => Scope::parse(s.trim()).map_err(|e| Error::InvalidScope(e.to_string())),
None => Ok(Scope::empty()),
}
// Parse and validate the scope
Scope::parse(answer_str.trim()).map_err(|e| Error::InvalidScope(e.to_string()))
}
fn input_description(&self) -> Result<Description, Error> {
use inquire::Text;
loop {
let answer = Text::new("Enter description (required):")
.with_help_message(
@@ -180,13 +145,10 @@ impl Prompter for RealPrompts {
}
fn input_body(&self) -> Result<Body, Error> {
use inquire::Editor;
let wants_body = Confirm::new("Add a body?")
.with_default(false)
.prompt()
.map_err(|_| Error::Cancelled)?;
if !wants_body {
return Ok(Body::default());
}
@@ -196,12 +158,11 @@ JJ: Body (optional). Markdown is supported.\n\
JJ: Wrap prose lines at 72 characters where possible.\n\
JJ: Lines starting with \"JJ:\" will be removed.\n";
let raw = Editor::new("Body:")
let raw = inquire::Editor::new("Body:")
.with_predefined_text(template)
.with_file_extension(".md")
.prompt()
.map_err(|_| Error::Cancelled)?;
let stripped: String = raw
.lines()
.filter(|line| !line.starts_with("JJ:"))
@@ -212,16 +173,11 @@ JJ: Lines starting with \"JJ:\" will be removed.\n";
}
fn confirm_apply(&self, message: &str) -> Result<bool, Error> {
use inquire::Confirm;
// Show preview
println!(
"\n📝 Commit Message Preview:\n{}\n",
format_message_box(message)
);
// Get confirmation
Confirm::new("Apply this commit message?")
inquire::Confirm::new("Apply this commit message?")
.with_default(true)
.with_help_message("Select 'No' to cancel and start over")
.prompt()

View File

@@ -58,16 +58,13 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
if !self.executor.is_repository().await? {
return Err(Error::NotARepository);
}
let commit_type = self.type_selection().await?;
let commit_type = self.type_selection()?;
loop {
let scope = self.scope_input().await?;
let description = self.description_input().await?;
let breaking_change = self.breaking_change_input().await?;
let body = self.body_input().await?;
match self
.preview_and_confirm(commit_type, scope, description, breaking_change, body)
.await
{
let scope = self.scope_input()?;
let description = self.description_input()?;
let breaking_change = self.breaking_change_input()?;
let body = self.body_input()?;
match self.preview_and_confirm(commit_type, scope, description, breaking_change, body) {
Ok(conventional_commit) => {
self.executor
.describe(&conventional_commit.to_string())
@@ -86,7 +83,7 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
}
/// Prompt user to select a commit type from the 11 available options
async fn type_selection(&self) -> Result<CommitType, Error> {
fn type_selection(&self) -> Result<CommitType, Error> {
self.prompts.select_commit_type()
}
@@ -94,7 +91,7 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
///
/// Returns Ok(Scope) with the validated scope, or
/// Error::Cancelled if user cancels
async fn scope_input(&self) -> Result<Scope, Error> {
fn scope_input(&self) -> Result<Scope, Error> {
self.prompts.input_scope()
}
@@ -102,7 +99,7 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
///
/// Returns Ok(Description) with the validated description, or
/// Error::Cancelled if user cancels
async fn description_input(&self) -> Result<Description, Error> {
fn description_input(&self) -> Result<Description, Error> {
self.prompts.input_description()
}
@@ -110,12 +107,12 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
///
/// Returns Ok(BreakingChange) with the validated breaking change,
/// or Error::Cancel if user cancels
async fn breaking_change_input(&self) -> Result<BreakingChange, Error> {
fn breaking_change_input(&self) -> Result<BreakingChange, Error> {
self.prompts.input_breaking_change()
}
/// Prompt user to optionally add a free-form body via an external editor
async fn body_input(&self) -> Result<Body, Error> {
fn body_input(&self) -> Result<Body, Error> {
self.prompts.input_body()
}
@@ -123,7 +120,7 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
///
/// This method also validates that the complete first line
/// doesn't exceed 72 characters
async fn preview_and_confirm(
fn preview_and_confirm(
&self,
commit_type: CommitType,
scope: Scope,
@@ -224,46 +221,46 @@ mod tests {
}
/// Test that type_selection returns a valid CommitType
#[tokio::test]
async fn type_selection_returns_valid_type() {
#[test]
fn type_selection_returns_valid_type() {
// Updated to use mock prompts to avoid TUI hanging
let mock_executor = MockJjExecutor::new();
let mock_prompts = MockPrompts::new().with_commit_type(CommitType::Feat);
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
// Now we can actually test the method with mock prompts
let result = workflow.type_selection().await;
let result = workflow.type_selection();
assert!(result.is_ok());
assert_eq!(result.unwrap(), CommitType::Feat);
}
/// Test that scope_input returns a valid Scope
#[tokio::test]
async fn scope_input_returns_valid_scope() {
#[test]
fn scope_input_returns_valid_scope() {
let mock_executor = MockJjExecutor::new();
let mock_prompts = MockPrompts::new().with_scope(Scope::parse("test").unwrap());
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
let result = workflow.scope_input().await;
let result = workflow.scope_input();
assert!(result.is_ok());
assert_eq!(result.unwrap(), Scope::parse("test").unwrap());
}
/// Test that description_input returns a valid Description
#[tokio::test]
async fn description_input_returns_valid_description() {
#[test]
fn description_input_returns_valid_description() {
let mock_executor = MockJjExecutor::new();
let mock_prompts = MockPrompts::new().with_description(Description::parse("test").unwrap());
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
let result = workflow.description_input().await;
let result = workflow.description_input();
assert!(result.is_ok());
assert_eq!(result.unwrap(), Description::parse("test").unwrap());
}
/// Test that preview_and_confirm returns a ConventionalCommit
#[tokio::test]
async fn preview_and_confirm_returns_conventional_commit() {
#[test]
fn preview_and_confirm_returns_conventional_commit() {
let mock_executor = MockJjExecutor::new();
let mock_prompts = MockPrompts::new().with_confirm(true);
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
@@ -273,9 +270,8 @@ mod tests {
let description = Description::parse("test description").unwrap();
let breaking_change = BreakingChange::No;
let body = Body::default();
let result = workflow
.preview_and_confirm(commit_type, scope, description, breaking_change, body)
.await;
let result =
workflow.preview_and_confirm(commit_type, scope, description, breaking_change, body);
assert!(result.is_ok());
}
@@ -451,8 +447,8 @@ mod tests {
}
/// Test that mock prompts track method calls correctly
#[tokio::test]
async fn test_mock_prompts_track_calls() {
#[test]
fn test_mock_prompts_track_calls() {
let mock_executor = MockJjExecutor::new().with_is_repo_response(Ok(true));
let mock_prompts = MockPrompts::new()
.with_commit_type(CommitType::Feat)
@@ -552,21 +548,19 @@ mod tests {
/// BreakingChange::No was hard-coded, so a confirmed
/// breaking-change commit was silently applied without the '!'
/// marker.
#[tokio::test]
async fn preview_and_confirm_forwards_breaking_change_yes() {
#[test]
fn preview_and_confirm_forwards_breaking_change_yes() {
let mock_executor = MockJjExecutor::new();
let mock_prompts = MockPrompts::new().with_confirm(true);
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
let result = workflow
.preview_and_confirm(
CommitType::Feat,
Scope::empty(),
Description::parse("remove old API").unwrap(),
BreakingChange::Yes,
Body::default(),
)
.await;
let result = workflow.preview_and_confirm(
CommitType::Feat,
Scope::empty(),
Description::parse("remove old API").unwrap(),
BreakingChange::Yes,
Body::default(),
);
assert!(result.is_ok(), "expected Ok, got: {:?}", result);
let message = result.unwrap().to_string();
@@ -580,22 +574,20 @@ mod tests {
/// Preview_and_confirm must forward BreakingChange::WithNote,
/// producing a commit with both the '!' header marker and the
/// BREAKING CHANGE footer.
#[tokio::test]
async fn preview_and_confirm_forwards_breaking_change_with_note() {
#[test]
fn preview_and_confirm_forwards_breaking_change_with_note() {
let mock_executor = MockJjExecutor::new();
let mock_prompts = MockPrompts::new().with_confirm(true);
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
let breaking_change: BreakingChange = "removes legacy endpoint".into();
let result = workflow
.preview_and_confirm(
CommitType::Feat,
Scope::empty(),
Description::parse("drop legacy API").unwrap(),
breaking_change,
Body::default(),
)
.await;
let result = workflow.preview_and_confirm(
CommitType::Feat,
Scope::empty(),
Description::parse("drop legacy API").unwrap(),
breaking_change,
Body::default(),
);
assert!(result.is_ok(), "expected Ok, got: {:?}", result);
let message = result.unwrap().to_string();
@@ -655,21 +647,19 @@ mod tests {
///
/// Currently the implementation passes Body::default() instead of the
/// received body, so this test will fail until that is fixed.
#[tokio::test]
async fn preview_and_confirm_forwards_body() {
#[test]
fn preview_and_confirm_forwards_body() {
let mock_executor = MockJjExecutor::new();
let mock_prompts = MockPrompts::new().with_confirm(true);
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
let result = workflow
.preview_and_confirm(
CommitType::Feat,
Scope::empty(),
Description::parse("add feature").unwrap(),
BreakingChange::No,
Body::from("This explains the change."),
)
.await;
let result = workflow.preview_and_confirm(
CommitType::Feat,
Scope::empty(),
Description::parse("add feature").unwrap(),
BreakingChange::No,
Body::from("This explains the change."),
);
assert!(result.is_ok(), "expected Ok, got: {:?}", result);
assert!(
@@ -684,21 +674,19 @@ mod tests {
/// preview_and_confirm must forward the body even when a breaking change is present
///
/// Expected format: "type!: desc\n\nbody\n\nBREAKING CHANGE: note"
#[tokio::test]
async fn preview_and_confirm_forwards_body_with_breaking_change() {
#[test]
fn preview_and_confirm_forwards_body_with_breaking_change() {
let mock_executor = MockJjExecutor::new();
let mock_prompts = MockPrompts::new().with_confirm(true);
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
let result = workflow
.preview_and_confirm(
CommitType::Feat,
Scope::empty(),
Description::parse("drop legacy API").unwrap(),
"removes legacy endpoint".into(),
Body::from("The endpoint was deprecated in v2."),
)
.await;
let result = workflow.preview_and_confirm(
CommitType::Feat,
Scope::empty(),
Description::parse("drop legacy API").unwrap(),
"removes legacy endpoint".into(),
Body::from("The endpoint was deprecated in v2."),
);
assert!(result.is_ok(), "expected Ok, got: {:?}", result);
let message = result.unwrap().to_string();

View File

@@ -20,7 +20,9 @@ fn test_all_error_variants() {
};
let _repo_locked = Error::RepositoryLocked;
let _failed_dir = Error::FailedGettingCurrentDir;
let _failed_config = Error::FailedReadingConfig;
let _failed_config = Error::FailedReadingConfig {
context: "test".to_string(),
};
// Application errors
let cancelled = Error::Cancelled;