show worktree locations in av tree output#675
Conversation
Current Aviator status
This PR was merged using Aviator.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
✅ FlexReview StatusCommon Owner:
Review SLO: |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 adds a useful feature to display worktree locations in the av tree output. The implementation is mostly correct, but I have a couple of suggestions. First, an error returned from listing worktrees is currently ignored, which should be handled to provide feedback to the user in case of failure. Second, the parsing logic for the git worktree list output in the new internal/git/worktree.go file uses a Go 1.22 feature and can be refactored for better compatibility and readability. I've provided suggestions for both points.
| return err | ||
| } | ||
|
|
||
| worktrees, _ := repo.WorktreeList(ctx) |
There was a problem hiding this comment.
The error returned by repo.WorktreeList(ctx) is ignored. If the command fails (e.g., due to a Git error), this will fail silently. The error should be handled by printing a warning to the user so they are aware that the worktree information might be missing.
worktrees, err := repo.WorktreeList(ctx)
if err != nil {
// Not a fatal error, just print a warning and continue.
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "warning: could not read worktree locations: %v\n", err)
}
| var worktrees []WorktreeInfo | ||
| var current WorktreeInfo | ||
| for line := range strings.SplitSeq(out, "\n") { | ||
| if line == "" { | ||
| if current.Path != "" { | ||
| worktrees = append(worktrees, current) | ||
| } | ||
| current = WorktreeInfo{} | ||
| continue | ||
| } | ||
| if rest, ok := strings.CutPrefix(line, "worktree "); ok { | ||
| current.Path = rest | ||
| } else if rest, ok := strings.CutPrefix(line, "HEAD "); ok { | ||
| current.HEAD = rest | ||
| } else if rest, ok := strings.CutPrefix(line, "branch "); ok { | ||
| current.Branch = strings.TrimPrefix(rest, "refs/heads/") | ||
| } | ||
| } | ||
| if current.Path != "" { | ||
| worktrees = append(worktrees, current) | ||
| } | ||
| return worktrees, nil |
There was a problem hiding this comment.
The use of for line := range strings.SplitSeq(out, "\n") requires Go 1.22. For better compatibility with older Go versions, it's safer to use strings.Split. Additionally, the parsing logic can be simplified by splitting the output into records first, which also removes the duplicated code for appending the last worktree. This improves readability and maintainability.
| var worktrees []WorktreeInfo | |
| var current WorktreeInfo | |
| for line := range strings.SplitSeq(out, "\n") { | |
| if line == "" { | |
| if current.Path != "" { | |
| worktrees = append(worktrees, current) | |
| } | |
| current = WorktreeInfo{} | |
| continue | |
| } | |
| if rest, ok := strings.CutPrefix(line, "worktree "); ok { | |
| current.Path = rest | |
| } else if rest, ok := strings.CutPrefix(line, "HEAD "); ok { | |
| current.HEAD = rest | |
| } else if rest, ok := strings.CutPrefix(line, "branch "); ok { | |
| current.Branch = strings.TrimPrefix(rest, "refs/heads/") | |
| } | |
| } | |
| if current.Path != "" { | |
| worktrees = append(worktrees, current) | |
| } | |
| return worktrees, nil | |
| var worktrees []WorktreeInfo | |
| // Each worktree record is separated by a blank line. | |
| for _, record := range strings.Split(out, "\n\n") { | |
| var current WorktreeInfo | |
| for _, line := range strings.Split(record, "\n") { | |
| if rest, ok := strings.CutPrefix(line, "worktree "); ok { | |
| current.Path = rest | |
| } else if rest, ok := strings.CutPrefix(line, "HEAD "); ok { | |
| current.HEAD = rest | |
| } else if rest, ok := strings.CutPrefix(line, "branch "); ok { | |
| current.Branch = strings.TrimPrefix(rest, "refs/heads/") | |
| } | |
| } | |
| if current.Path != "" { | |
| worktrees = append(worktrees, current) | |
| } | |
| } | |
| return worktrees, nil |
davi-maciel
requested review from
jainankit
and removed request for
davi-maciel
2 participants
branches checked out in worktrees now show the worktree directory name next to them