From de583ee6154451692304f018f532df574f9c9ddc Mon Sep 17 00:00:00 2001 From: sidfishus Date: Thu, 2 Jul 2026 14:08:33 +0100 Subject: [PATCH] fix(but): resolve bare paths for staged files in `commit -p` (#13318) --- crates/but/src/id/mod.rs | 34 +++++++++++++----- crates/but/src/id/tests.rs | 48 ++++++++++++++++++++++++++ crates/but/tests/but/command/commit.rs | 40 +++++++++++++++++++++ 3 files changed, 114 insertions(+), 8 deletions(-) diff --git a/crates/but/src/id/mod.rs b/crates/but/src/id/mod.rs index 366e4fb4e99..45ad7532252 100644 --- a/crates/but/src/id/mod.rs +++ b/crates/but/src/id/mod.rs @@ -729,11 +729,31 @@ impl IdMap { /// Private methods to individually parse what can appear on both side of a /// colon. (Some of them can also appear alone.) impl IdMap { - fn parse_uncommitted_filename<'a>( + /// Bare path token (e.g. `foo.txt`): any entry in [`Self::uncommitted_files`] with this path, + /// whether assigned to a stack or in the uncommitted area (`zz`). + /// + /// For stack-scoped resolution (e.g. `branch@{stack}:file.txt`), use + /// [`StackWithId::parse`] instead. + fn parse_uncommitted_filename<'a>(&'a self, element: &str) -> Vec + 'a>> { + self.parse_uncommitted_filename_where(element, |_| true) + } + + /// After [`UNCOMMITTED`] (`zz:`): entries in [`Self::uncommitted_files`] that are in the + /// uncommitted area (not assigned to a stack). + fn parse_uncommitted_area_filename<'a>(&'a self, element: &str) -> Vec + 'a>> { + self.parse_uncommitted_filename_where(element, |hunk_assignment| { + hunk_assignment.stack_id.is_none() + }) + } + + fn parse_uncommitted_filename_where<'a, F>( &'a self, - stack_id: Option, element: &str, - ) -> Vec + 'a>> { + include: F, + ) -> Vec + 'a>> + where + F: Fn(&HunkAssignment) -> bool, + { let mut matches = Vec:: + 'a>>::new(); for uncommitted_file in self.uncommitted_files.values() { let hunk_assignments = uncommitted_file.hunk_assignments(); @@ -741,9 +761,7 @@ impl IdMap { // TODO once the set of allowed CLI IDs is determined and the // access patterns of `uncommitted_files` are known, change its data // structure to be more efficient than the current linear search. - if hunk_assignment.stack_id == stack_id - && hunk_assignment.path_bytes == element.as_bytes() - { + if hunk_assignment.path_bytes == element.as_bytes() && include(hunk_assignment) { matches.push(Box::new(uncommitted_file)); } } @@ -810,7 +828,7 @@ impl IdMap { } } } - matches.extend(self.parse_uncommitted_filename(None, element)); + matches.extend(self.parse_uncommitted_filename(element)); // The following match only if there have been no matches so far. if !matches.is_empty() { @@ -896,7 +914,7 @@ impl IdMap { id_map: &'a IdMap, _changes_in_commit_fn: &mut ChangesInCommitFn<'a>, ) -> anyhow::Result + 'a>>> { - Ok(id_map.parse_uncommitted_filename(None, element)) + Ok(id_map.parse_uncommitted_area_filename(element)) } fn to_cli_id( diff --git a/crates/but/src/id/tests.rs b/crates/but/src/id/tests.rs index 567ee3c7956..fad32581f79 100644 --- a/crates/but/src/id/tests.rs +++ b/crates/but/src/id/tests.rs @@ -1045,6 +1045,54 @@ fn colon_uncommitted_filename() -> anyhow::Result<()> { ] "#); + // `zz:` is scoped to the uncommitted area only; staged files are not visible here. + insta::assert_debug_snapshot!(id_map.parse("zz:assigned", Box::new(changed_paths_fn))?, @"[]"); + + Ok(()) +} + +#[test] +fn non_colon_uncommitted_filename() -> anyhow::Result<()> { + let stacks = vec![stack([segment("unused", [id(1)], None, [])])]; + let hunk_assignments = vec![hunk_assignment( + "assigned", + Some(StackId::from_number_for_testing(1)), + )]; + let id_map = IdMap::new(stacks, hunk_assignments)?; + let changed_paths_fn = |commit_id: gix::ObjectId, + parent_id: Option| + -> anyhow::Result> { + bail!("unexpected IDs {commit_id} {parent_id:?}"); + }; + + // Bare path resolves staged files too. + insta::assert_debug_snapshot!(id_map.parse("assigned", Box::new(changed_paths_fn))?, @r#" + [ + UncommittedHunkOrFile( + UncommittedHunkOrFile { + id: "mv", + hunk_assignments: NonEmpty { + head: HunkAssignment { + id: None, + hunk_header: None, + path: "", + path_bytes: "assigned", + stack_id: Some( + 00000000-0000-0000-0000-000000000001, + ), + branch_ref_bytes: None, + line_nums_added: None, + line_nums_removed: None, + diff: None, + }, + tail: [], + }, + is_entire_file: true, + }, + ), + ] + "#); + Ok(()) } diff --git a/crates/but/tests/but/command/commit.rs b/crates/but/tests/but/command/commit.rs index 506c2c72ebd..0f84f4b8a9b 100644 --- a/crates/but/tests/but/command/commit.rs +++ b/crates/but/tests/but/command/commit.rs @@ -1302,6 +1302,46 @@ Error: Invalid file ID(s): "#]]); } +#[test] +fn commit_with_bare_path_on_staged_file() -> anyhow::Result<()> { + let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack"); + env.setup_metadata(&["A"]); + + env.file("foo.txt", "hello"); + env.but("stage foo.txt A").assert().success(); + + env.but("commit -p foo.txt -m 'test commit'") + .assert() + .success() + .stdout_eq(str![[r#" +✓ Created commit [..] on branch A + +"#]]); + + let log = env.git_log(); + assert!(log.contains("test commit")); + + Ok(()) +} + +#[test] +fn commit_with_zz_prefix_on_staged_file_fails() { + let env = Sandbox::init_scenario_with_target_and_default_settings("one-stack"); + env.setup_metadata(&["A"]); + + env.file("foo.txt", "hello"); + env.but("stage foo.txt A").assert().success(); + + env.but("commit -p zz:foo.txt -m 'test commit'") + .assert() + .failure() + .stderr_eq(str![[r#" +Error: Invalid file ID(s): + 'zz:foo.txt' not found. Run 'but status' to see available file IDs. + +"#]]); +} + #[test] fn commit_with_file_assigned_to_different_stack_fails() -> anyhow::Result<()> { let env = Sandbox::init_scenario_with_target_and_default_settings("two-stacks");