feat: implicitly use current repo and branch context for pr commands#2
Conversation
- Add util helpers CurrentRepo and CurrentBranch to parse local git context - Remove required --repo flag from all pr subcommands, falling back to CurrentRepo - Modify pr view to accept 0 args, inferring target PR via CurrentBranch and head matching - Update api client ListPRs to support head filtering
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the CLI by allowing repository and branch context to be inferred from the local environment, making the --repo flag optional and enabling the view command to default to the current branch's pull request. The review feedback highlights several areas for improvement: refactoring duplicated repository inference logic, making the remote name configurable instead of hardcoding origin, increasing the robustness of the git URL parsing logic, and handling potential ambiguities when searching for pull requests in cross-repository setups.
|
|
||
| // CurrentRepo returns the owner/name of the repository for the current directory. | ||
| func CurrentRepo() (string, error) { | ||
| out, err := exec.Command("git", "config", "--get", "remote.origin.url").Output() |
There was a problem hiding this comment.
The remote name is hardcoded to origin. While this is a common default, users often use other names like upstream for the main repository (especially when working with forks). Consider allowing the remote name to be configurable or detecting the remote associated with the current branch's upstream.
| if strings.HasPrefix(urlStr, "git@") { | ||
| parts := strings.Split(urlStr, ":") | ||
| if len(parts) == 2 { | ||
| return parts[1], nil | ||
| } | ||
| } | ||
|
|
||
| // https://gitee.com/owner/repo | ||
| if strings.HasPrefix(urlStr, "http://") || strings.HasPrefix(urlStr, "https://") { | ||
| parts := strings.Split(urlStr, "/") | ||
| if len(parts) >= 2 { | ||
| return parts[len(parts)-2] + "/" + parts[len(parts)-1], nil | ||
| } | ||
| } | ||
|
|
||
| return "", fmt.Errorf("could not parse remote URL: %s", urlStr) |
There was a problem hiding this comment.
The current URL parsing logic is fragile and does not handle ssh:// URLs or URLs with non-standard ports. A more robust approach is to strip the protocol and user information, then extract the last two segments of the path as the owner/repo.
if i := strings.Index(urlStr, "@"); i != -1 {
urlStr = urlStr[i+1:]
} else {
urlStr = strings.TrimPrefix(urlStr, "http://")
urlStr = strings.TrimPrefix(urlStr, "https://")
urlStr = strings.TrimPrefix(urlStr, "ssh://")
}
// The path is usually the last two segments (owner/repo)
parts := strings.FieldsFunc(urlStr, func(r rune) bool {
return r == '/' || r == ':'
})
if len(parts) >= 2 {
return parts[len(parts)-2] + "/" + parts[len(parts)-1], nil
}
return "", fmt.Errorf("could not parse remote URL: %s", urlStr)| if repo == "" { | ||
| repo, _ = util.CurrentRepo() | ||
| } | ||
| if repo == "" { | ||
| return fmt.Errorf("--repo is required or must be run inside a git repository") | ||
| } |
There was a problem hiding this comment.
This logic for inferring the repository context is duplicated across all PR subcommands (list, view, create, merge, close, comment). To improve maintainability and reduce boilerplate, consider refactoring this into a helper function.
| prs, err := app.Client.ListPRs(app.Ctx, owner, name, "open", owner+":"+branch, 1, 30) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if len(prs) == 0 { | ||
| prs, err = app.Client.ListPRs(app.Ctx, owner, name, "open", branch, 1, 30) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if len(prs) == 0 { | ||
| return fmt.Errorf("no open pull requests found for branch %s", branch) | ||
| } | ||
| } |
There was a problem hiding this comment.
Finding a PR by branch name using owner + ":" + branch as a filter only works for pull requests where the head branch is in the same repository as the base. For cross-repository PRs (e.g., from a fork), the head is typically fork_owner:branch. The fallback at line 87 might not be supported by the Gitee API or could be ambiguous. Consider if a more comprehensive search or local filtering of open PRs is needed here.
Address all four review comments from gemini-code-assist:
- Refactor duplicated repo inference logic into resolveRepo() helper
instead of repeating it in every subcommand handler.
- Detect upstream remote via git rev-parse @{upstream} instead of
hardcoding 'origin', with 'origin' as fallback.
- Improve git remote URL parsing to handle all schemes (git@, ssh://,
http://, https://) by stripping protocol/user prefix then extracting
the last two path segments.
- Improve cross-repo PR branch lookup: first search via head API param,
then fall back to local filtering of open PRs by Head.Ref and
Head.Label.
Refs #2
1 participant
This PR updates the
gt prcommands to implicitly infer the repository context from the local git remote if the--repoflag is not explicitly provided. This bringsgtup to parity with standardghbehavior. It also modifiesgt pr viewto implicitly view the PR associated with the current checked-out branch when zero arguments are provided.