fix(but): resolve bare paths for staged files in commit -p (#13318)

crates/but/src/id/mod.rs

@@ -729,21 +729,39 @@ 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<Box<dyn Node<'a> + '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<Box<dyn Node<'a> + '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<StackId>,
         element: &str,
-    ) -> Vec<Box<dyn Node<'a> + 'a>> {
+        include: F,
+    ) -> Vec<Box<dyn Node<'a> + 'a>>
+    where
+        F: Fn(&HunkAssignment) -> bool,
+    {
         let mut matches = Vec::<Box<dyn Node<'a> + 'a>>::new();
         for uncommitted_file in self.uncommitted_files.values() {
             let hunk_assignments = uncommitted_file.hunk_assignments();
             let hunk_assignment = hunk_assignments.first();
             // 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<Vec<Box<dyn Node<'a> + 'a>>> {
-                    Ok(id_map.parse_uncommitted_filename(None, element))
+                    Ok(id_map.parse_uncommitted_area_filename(element))
                 }
 
                 fn to_cli_id(

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<gix::ObjectId>|
+     -> anyhow::Result<Vec<but_core::TreeChange>> {
+        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(())
 }
 

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");