fix: support worktree commands in bare workspace root#37
Conversation
Signed-off-by: samzong <samzong.lu@gmail.com>
Summary of ChangesHello @samzong, 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 addresses a functional limitation where Git worktree commands, specifically 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. 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
|
There was a problem hiding this comment.
Code Review
This pull request enhances worktree command support for bare workspace roots by detecting the .bare directory. The implementation in the List function correctly addresses this, but it introduces some code duplication. I've provided a suggestion to refactor this for better maintainability and conciseness. Overall, this is a valuable fix that improves the tool's usability with bare repository setups.
| // Find the bare root to support running from any directory | ||
| root, err := FindBareRoot("") | ||
| if err != nil { | ||
| // Fallback to current directory if not in a bare repo structure | ||
| result, err := c.runner.RunLogged("worktree", "list", "--porcelain") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to list worktrees: %w", err) | ||
| } | ||
| return parseWorktreeList(string(result.Stdout)) | ||
| } | ||
|
|
||
| // Run git command from the .bare directory | ||
| bareDir := filepath.Join(root, ".bare") | ||
| result, err := c.runner.RunLogged("-C", bareDir, "worktree", "list", "--porcelain") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to list worktrees: %w", err) | ||
| } |
There was a problem hiding this comment.
There's some code duplication in the List function. Both the fallback logic (when FindBareRoot fails) and the main logic for bare repositories run a git worktree list command, check for an error, and then parse the result. The if block has its own return statement, which is similar to the logic at the end of the function.
You can refactor this to reduce duplication by first determining the arguments for the git command, and then executing the command and processing its output only once at the end of the function.
| // Find the bare root to support running from any directory | |
| root, err := FindBareRoot("") | |
| if err != nil { | |
| // Fallback to current directory if not in a bare repo structure | |
| result, err := c.runner.RunLogged("worktree", "list", "--porcelain") | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to list worktrees: %w", err) | |
| } | |
| return parseWorktreeList(string(result.Stdout)) | |
| } | |
| // Run git command from the .bare directory | |
| bareDir := filepath.Join(root, ".bare") | |
| result, err := c.runner.RunLogged("-C", bareDir, "worktree", "list", "--porcelain") | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to list worktrees: %w", err) | |
| } | |
| var result gitcmd.Result | |
| var err error | |
| // Find the bare root to support running from any directory | |
| root, findErr := FindBareRoot("") | |
| if findErr != nil { | |
| // Fallback to current directory if not in a bare repo structure | |
| result, err = c.runner.RunLogged("worktree", "list", "--porcelain") | |
| } else { | |
| // Run git command from the .bare directory | |
| bareDir := filepath.Join(root, ".bare") | |
| result, err = c.runner.RunLogged("-C", bareDir, "worktree", "list", "--porcelain") | |
| } | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to list worktrees: %w", err) | |
| } |
No description provided.