Compare commits

..

4 Commits

Author SHA1 Message Date
e28f1fdbe1 feat(errors): preserve jj-emitted errors when loading config
Some checks failed
Publish Docker Images / coverage-and-sonar (push) Failing after 16m33s
2026-04-05 15:56:24 +02:00
10d52d3e72 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 15:56:24 +02:00
7149337353 refactor(workflow): remove unnecessary async declarations 2026-04-05 15:56:24 +02:00
1c983f3a8d refactor(nix): simplify package declaration 2026-04-05 15:56:24 +02:00
9 changed files with 115 additions and 123 deletions

View File

@@ -41,36 +41,22 @@
}; };
in { in {
formatter = alejandra.defaultPackage.${system}; formatter = alejandra.defaultPackage.${system};
packages = packages = let
(import ./nix/package.nix {inherit pkgs rustPlatform;}) nativeRustVersion = pkgs.rust-bin.stable.latest.default;
// { nativeRustPlatform = pkgs.makeRustPlatform {
windows = let cargo = nativeRustVersion;
rustc = nativeRustVersion;
};
mingwPkgs = pkgs.pkgsCross.mingwW64; mingwPkgs = pkgs.pkgsCross.mingwW64;
rustWindows = pkgs.rust-bin.stable.latest.default.override { windowsRustVersion = pkgs.rust-bin.stable.latest.default.override {
targets = ["x86_64-pc-windows-gnu"]; targets = ["x86_64-pc-windows-gnu"];
}; };
rustPlatformWindows = mingwPkgs.makeRustPlatform { windowsRustPlatform = mingwPkgs.makeRustPlatform {
cargo = rustWindows; cargo = windowsRustVersion;
rustc = rustWindows; rustc = windowsRustVersion;
}; };
cargoToml = builtins.fromTOML (builtins.readFile ./Cargo.toml);
in in
rustPlatformWindows.buildRustPackage { import ./nix/package.nix {inherit pkgs nativeRustPlatform windowsRustPlatform;};
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
'';
};
};
devShell = import ./nix/shell.nix { devShell = import ./nix/shell.nix {
inherit inputs pkgs rustVersion; inherit inputs pkgs rustVersion;
}; };

View File

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

View File

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

View File

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

View File

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

View File

@@ -41,16 +41,22 @@ impl JjLib {
let mut config = StackedConfig::with_defaults(); let mut config = StackedConfig::with_defaults();
for path in Self::user_config_paths() { for path in Self::user_config_paths() {
if path.is_dir() { if path.is_dir() {
config config.load_dir(ConfigSource::User, &path).map_err(|e| {
.load_dir(ConfigSource::User, &path) Error::FailedReadingConfig {
.map_err(|_| Error::FailedReadingConfig)?; context: e.to_string(),
}
})?;
} else if path.exists() { } else if path.exists() {
config config.load_file(ConfigSource::User, path).map_err(|e| {
.load_file(ConfigSource::User, path) Error::FailedReadingConfig {
.map_err(|_| 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: /// 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::InvalidCommitMessage(_) => EXIT_ERROR,
Error::NonInteractive => EXIT_ERROR, Error::NonInteractive => EXIT_ERROR,
Error::FailedGettingCurrentDir => EXIT_ERROR, Error::FailedGettingCurrentDir => EXIT_ERROR,
Error::FailedReadingConfig => EXIT_ERROR, Error::FailedReadingConfig { .. } => EXIT_ERROR,
} }
} }

View File

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

View File

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