fix(gix-config-value): expand ~user with no path after it - #2986
fix(gix-config-value): expand ~user with no path after it#2986Roshan Ramani (rawsun007) wants to merge 1 commit into
Conversation
`git` takes everything past the `~` as the user name when there is no `/`, so `~root` is that user's home. This required a `/` and returned `~root` unchanged. `~` on its own still does not interpolate, which `tilde_alone_does_not_interpolate` pins for the no-context case. Assisted-by: Claude Opus 5 (Claude Code)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 438e2bee06
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
| // directory on git 2.50.1: everything past the `~` is the user name when there | ||
| // is no `/`, and no trailing slash is needed. | ||
| let home = std::env::current_dir()?; | ||
| assert_eq!(interpolate_without_context("~user")?, home.join("user")); |
There was a problem hiding this comment.
The new assert_eq! omits the required final description, so a regression reports only the compared paths rather than explaining that a bare ~user must resolve directly to that user's home directory. Add an assertion message stating that invariant.
AGENTS.md reference: AGENTS.md:L118-L121
Useful? React with 👍 / 👎.
| })?; | ||
| Ok(home_path.join(val)) | ||
| } else if self.starts_with(b"~") && self.contains(&b'/') { | ||
| } else if self.starts_with(b"~") && self.len() > 1 { |
There was a problem hiding this comment.
~user form
When callers rely on this public API's rustdoc, they are still told that only ~user/ triggers named-user interpolation and that other nonempty values remain unchanged; Context::home_for_user likewise documents only ~name/. With this condition, bare ~user instead invokes the callback and can return Missing, so update both public descriptions to include the slashless form.
AGENTS.md reference: AGENTS.md:L147-L152
Useful? React with 👍 / 👎.
|
You must be running the same workflow: Duplicate of #2984 |
|
my fault, i should have searched for an open pr before starting. #2984 predates mine by hours and is the better change anyway, it does the bare codex was right on both counts too, for the record: the new assertion had no description and i did not update the rustdoc, and i had already been pulled up on the assertion rule in #2973. Assisted-by: Claude Opus 5 (Claude Code). |
2 participants
written by Claude Opus 5 in Claude Code, through Roshan's account, per the identification rule in CONTRIBUTING.md. he has not read the diff line by line yet.
third type in this crate after #2967 and #2980, same method: run the inputs through
git config --type=pathand diff.on git 2.50.1, everything matched except the tilde with no
/after it:~root~root, unchanged~root/~root/xx%(prefix)%(prefix), unchanged~nosuchuser/xgittakes everything past the~as the user name when there is no/, so the fix is to stop requiring one.interpolate_usernow takes the whole remainder as the name in that case and returns the home directory as-is.one thing i deliberately did not change, because it looks like your decision rather than an oversight.
gitalso expands a bare~to the home directory, and this crate returns it unchanged, whichtilde_alone_does_not_interpolateasserts. that test goes throughinterpolate_without_context, so what it really pins is~with nohome_dirin theContext, where returning the literal is a reasonable lenience and erroring would be the alternative. note~/xdoes error there, so the two are inconsistent today. happy to make~expand wheneverhome_diris set and keep the literal when it is not, which would close the gap without touching that test, if that is the behaviour you want.cargo test -p gix-config-valueis 59 passing. reverting onlysrc/path.rsfails exactly the new test.cargo fmtapplied; clippy reports only the pre-existing removed-lint warning the workspace emits.