Skip to content

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

Open
sidfishus wants to merge 1 commit into
gitbutlerapp:masterfrom
sidfishus:13318_fix-commit-p-staged-path
Open

fix(but): resolve bare paths for staged files in commit -p (#13318)#14579
sidfishus wants to merge 1 commit into
gitbutlerapp:masterfrom
sidfishus:13318_fix-commit-p-staged-path

Conversation

@sidfishus

Copy link
Copy Markdown
Contributor

🧢 Changes

Fix bare-path resolution in IdMap so but commit -p foo.txt works for staged files.
Add unit + integration tests for bare path (positive) and zz: prefix (negative).

☕️ Reasoning

non_colon_uncommitted_filename: I thought creating a separate test for cases where there's no colon / prefix makes more sense instead of adding them to the existing colon_uncommitted_filename. That's the reproducer for the bare-path case (#13318).

colon_uncommitted_filename: I only added zz:assigned → empty here, a guard so zz: stays scoped to the unassigned bucket and doesn't regress.

commit.rs: commit_with_bare_path_on_staged_file is the reproducer for that style of test. I also added commit_with_zz_prefix_on_staged_file_fails as the matching negative case. I didn't add comments because the tests are self explanatory and this is consistent with the other tests.

Issue discussion: Per #13318, fix the reported case where a single staged foo.txt wasn't found. Qualified IDs (oo, i0:foo.txt) still work. Duplicate paths across areas still hit the existing ambiguity error; no new disambiguation logic in this PR.

This seems like a suitable issue to pick up for my first contribution.

AI Notice: I have peer programmed this with an AI agent. I've spent a lot of time reproducing and understanding the problem, and considering how best to name the functions (I got confused between uncommitted files and uncommitted/unassigned (zz)). This is what I've finalised on, but happy to be led by somebody with more experience of the codebase.

🎫 Affected issues

Fixes: #13318

@sidfishus sidfishus force-pushed the 13318_fix-commit-p-staged-path branch from 35554b5 to caea74f Compare July 2, 2026 13:08
@sidfishus sidfishus force-pushed the 13318_fix-commit-p-staged-path branch from caea74f to de583ee Compare July 2, 2026 13:08
@Byron

Byron commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Thanks a lot for giving it a shot!

I think this is an area where @jonathantanmy2 would have the most insight.

@Byron Byron requested a review from jonathantanmy2 July 2, 2026 22:37
@sidfishus

Copy link
Copy Markdown
Contributor Author

To help with review and give proof here's the 2 new automated tests failing without the changes:

test-fail-without-fix-compact

And the end to end run failing without the changes:

e2e-without-fix

And then the 2 new tests succeeding with fix:

test-pass-with-fix-compact

And the end to end run succeeding with the fix:

e2e-with-fix

@Byron

Byron commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Thanks a lot @sidfishus, this helps for sure!

Let me also reel in @davidpdrsn who has been working no a new implementation of but commit, to see if this fix is already included there as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

but commit -p doesn't accept file path when file is staged

2 participants