Repository navigation
Conversation
The public `join_path` helper drops the slash in `join_path("parent", "/child")`, yielding `parentchild`. Strip leading slashes from later tokens before the normal joining logic so they remain relative to the accumulated prefix and receive a separator. Document this retained relative-token contract and cover string/PathLike inputs, chained tokens, roots, empty tokens and repeated leading separators.
Five of eleven regression cases fail before this change. Afterward the complete utility module passes 128 tests with 39 skips on each CLI/Gix backend. Tree/reference/remote caller modules pass 230 tests with seven stalled-remote timeout failures per backend; the exact same seven failures reproduce on unchanged main. Repository pre-commit hooks pass. Tested on Linux with Python 3.12.14, Git 2.52.0 and GixPython 0.3.0; no other platform execution is claimed.
Assisted-by: OpenAI Codex (model: unknown, autonomous)
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the join_path() prefix-preservation change.
No open issues 🎉
What changed in this PR
Fixes join_path() so slash-prefixed later tokens retain the accumulated prefix and are joined with the correct separator.
Changes:
- Strip leading slashes from later tokens before applying the existing join logic.
- Document that later tokens are treated as relative.
- Add 11 regression cases covering strings, PathLike input, chained tokens, roots, empty tokens, and repeated leading slashes.
| File | Description |
|---|---|
test/test_util.py |
Adds parameterized coverage for relative-token joining. |
git/util.py |
Fixes separator handling and documents the joining contract. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The public
join_path("parent", "/child")returnsparentchild, and chaining slash-prefixed tokens loses each separator. Strip leading slashes from later tokens before the existing join logic. Later tokens still extend the accumulated prefix; they do not replace it. Document this contract and cover strings, PathLike input, chained tokens, roots, empty tokens and repeated leading slashes.This preserves the relative-token intention explained in the earlier #119 discussion. That proposal replaced the accumulated prefix with an absolute token; this patch retains the prefix and fixes the missing seam.
Motivation: This work was pursued to build @gavenren's requested portfolio of tested open-source contributions. Inspecting GitPython's public path helpers exposed this reproducible case.
Validation (Linux, Python 3.12.14, Git 2.52.0, GixPython 0.3.0):
main.test/test_util.py, viatest/run-local.py, with each ofGIT_PYTHON_BACKEND=cliandgix: 128 passed, 39 skipped per backend.test_tree.py test_refs.py test_remote.py, through the same runner with both backends: 230 passed, seven failed per backend. The identical seventest_timeout_stalled_remotecases fail on an unchanged-base worktree with both backends (seven failed, two passed in the isolated timeout selection). These failures concern the test's three-second timeout-cleanup bound; their root cause has not been established here.pre-commit run --files git/util.py test/test_util.py: passed all applicable hooks, including Ruff check/format.git diff --check: passed.Agent identification: This PR, implementation, tests, commit and description were prepared autonomously by OpenAI Codex in ChatGPT Work Mode, communicating through @gavenren's account. The exact underlying model and reasoning setting are unavailable. No human review, approval or manual testing is claimed.