Conversation
spacewander
left a comment
There was a problem hiding this comment.
The commit hash is only embedded in the URL when a filename is given, but the new check runs on every invocation — so git browse with no filename (whose URL never contains the commit) gets blocked unnecessarily.
Signed-off-by: vjymisal0 <misalvijay153@gmail.com>
|
Thanks for catching this @spacewander! I’ve limited the remote-commit validation to invocations that include a filename, so still opens the repository even when HEAD is unpushed. I also added a regression test for that case. Verified with bash syntax checks and manual pushed/unpushed filename scenarios; the local environment doesn’t have Bats installed. |
Signed-off-by: vjymisal0 <misalvijay153@gmail.com>
|
Thanks for the CI report. The new regression test was still invoking the no-filename form while expecting rejection. I updated it to pass |
…nt prefix/dot mismatches
| commit_hash=$(git rev-parse HEAD 2>/dev/null) | ||
| commit_or_branch=${commit_hash:-${branch}} | ||
|
|
||
| if [[ -n ${filename} ]] && ! git branch --remotes --contains "$commit_or_branch" 2>/dev/null | |
There was a problem hiding this comment.
Why add the filename guard?
There was a problem hiding this comment.
When no filename is provided (e.g. git browse or git browse ), the command opens the repository root URL (e.g. https://github.com/owner/repo), which does not include any commit hash and always succeeds even with unpushed local commits.
When a filename is specified, the URL embeds $commit_or_branch (e.g. .../blob//), which returns a 404 on GitHub/GitLab if the commit has not been pushed to that remote. The filename guard allows general repo browsing to work freely while protecting file permalinks against 404s.
|
Thanks for catching this @spacewander! I updated the remote-commit check to run only when a filename is provided, so continues to open the repository URL even with an unpushed local commit. I also added a regression test and tightened the remote matching. Verified with bash syntax checking and manual pushed/unpushed cases; the full Bats suite could not run here because is not installed. |
|
Thanks @spacewander — the remote commit check is now limited to URL forms that include a filename, while remote-only browsing remains unaffected. Added regression coverage for remote branch matching and unpushed file commits; the branch is clean and pushed. |
hyperupcall
left a comment
There was a problem hiding this comment.
I'm not convinced this is necessary. If the commit doesn't exist on remote, the web page will open, and it will 404. This gives information to the user, who can then push and refresh the page.
With this new code, the user will have to run this command again in that case. Additionally, the user will be shown a non-standard error, and if the network is slow, then this command will take even longer to complete.
|
Thanks for the thoughtful feedback @hyperupcall! That makes complete sense — avoiding the additional network round-trip keeps git-browse snappy, and landing on the remote 404 page naturally prompts the user to push if needed without requiring a re-invocation. |
|
Great glad that we agree |
Summary
git-browsecan construct a repository URL for a local commit that the selected remote does not contain. Such links fail when opened. Check the remote-tracking refs before constructing the URL and report a clear error when the commit has not been pushed.Changes
HEADis contained by a ref under the selected remote.Test plan
bash -n bin/git-browsegit diff --checkFixes #1153