-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Clarify checkpointing git phrasing #8392
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
jfcantu
wants to merge
6
commits into
RooCodeInc:main
from
jfcantu:clarify-checkpointing-git-phrasing
Closed
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
e072c22
Eliminate term "nested" Git repositories to reduce confusion, clarifi…
jfcantu 97e324a
Merge branch 'main' of https://github.com/jfcantu/Roo-Code into clari…
jfcantu ba30ca7
fix regex match for modified error message
jfcantu 439a0a2
Eliminate term "nested" Git repositories to reduce confusion, clarifi…
jfcantu 37bedd0
fix regex match for modified error message
jfcantu 3e1b9fc
Merge branch 'clarify-checkpointing-git-phrasing' of https://github.c…
jfcantu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,17 +70,17 @@ export abstract class ShadowCheckpointService extends EventEmitter { | |
| throw new Error("Shadow git repo already initialized") | ||
| } | ||
|
|
||
| const nestedGitPath = await this.getNestedGitRepository() | ||
| const childGitPath = await this.getChildGitRepository() | ||
|
|
||
| if (nestedGitPath) { | ||
| if (childGitPath) { | ||
| // Show persistent error message with the offending path | ||
| const relativePath = path.relative(this.workspaceDir, nestedGitPath) | ||
| const message = t("common:errors.nested_git_repos_warning", { path: relativePath }) | ||
| const relativePath = path.relative(this.workspaceDir, childGitPath) | ||
| const message = t("common:errors.child_git_repos_warning", { path: relativePath }) | ||
|
||
| vscode.window.showErrorMessage(message) | ||
|
|
||
| throw new Error( | ||
| `Checkpoints are disabled because a nested git repository was detected at: ${relativePath}. ` + | ||
| "Please remove or relocate nested git repositories to use the checkpoints feature.", | ||
| `Checkpoints are disabled because a Git repository was detected below the workspace root at: ${relativePath}. ` + | ||
| `To use checkpoints, please remove or relocate this git repository, or open a Git repository as the workspace root.`, | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -160,17 +160,17 @@ export abstract class ShadowCheckpointService extends EventEmitter { | |
| } | ||
| } | ||
|
|
||
| private async getNestedGitRepository(): Promise<string | null> { | ||
| private async getChildGitRepository(): Promise<string | null> { | ||
| try { | ||
| // Find all .git/HEAD files that are not at the root level. | ||
| const args = ["--files", "--hidden", "--follow", "-g", "**/.git/HEAD", this.workspaceDir] | ||
|
|
||
| const gitPaths = await executeRipgrep({ args, workspacePath: this.workspaceDir }) | ||
|
|
||
| // Filter to only include nested git directories (not the root .git). | ||
| // Filter to only include child git directories (not the root .git). | ||
| // Since we're searching for HEAD files, we expect type to be "file" | ||
| const nestedGitPaths = gitPaths.filter(({ type, path: filePath }) => { | ||
| // Check if it's a file and is a nested .git/HEAD (not at root) | ||
| const childGitPaths = gitPaths.filter(({ type, path: filePath }) => { | ||
| // Check if it's a file and is a child .git/HEAD (not at root) | ||
| if (type !== "file") return false | ||
|
|
||
| // Ensure it's a .git/HEAD file and not the root one | ||
|
|
@@ -182,10 +182,10 @@ export abstract class ShadowCheckpointService extends EventEmitter { | |
| ) | ||
| }) | ||
|
|
||
| if (nestedGitPaths.length > 0) { | ||
| // Get the first nested git repository path | ||
| if (childGitPaths.length > 0) { | ||
| // Get the first child git repository path | ||
| // Remove .git/HEAD from the path to get the repository directory | ||
| const headPath = nestedGitPaths[0].path | ||
| const headPath = childGitPaths[0].path | ||
|
|
||
| // Use path module to properly extract the repository directory | ||
| // The HEAD file is at .git/HEAD, so we need to go up two directories | ||
|
|
@@ -195,18 +195,18 @@ export abstract class ShadowCheckpointService extends EventEmitter { | |
| const absolutePath = path.join(this.workspaceDir, repoDir) | ||
|
|
||
| this.log( | ||
| `[${this.constructor.name}#getNestedGitRepository] found ${nestedGitPaths.length} nested git repositories, first at: ${repoDir}`, | ||
| `[${this.constructor.name}#getChildGitRepository] found ${childGitPaths.length} child git repositories, first at: ${repoDir}`, | ||
| ) | ||
| return absolutePath | ||
| } | ||
|
|
||
| return null | ||
| } catch (error) { | ||
| this.log( | ||
| `[${this.constructor.name}#getNestedGitRepository] failed to check for nested git repos: ${error instanceof Error ? error.message : String(error)}`, | ||
| `[${this.constructor.name}#getChildGitRepository] failed to check for child git repos: ${error instanceof Error ? error.message : String(error)}`, | ||
| ) | ||
|
|
||
| // If we can't check, assume there are no nested repos to avoid blocking the feature. | ||
| // If we can't check, assume there are no child repos to avoid blocking the feature. | ||
| return null | ||
| } | ||
| } | ||
|
|
||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typographical consistency: The string uses "Git repository" earlier but refers to it as "git repository" later on this line. Consider using a consistent capitalization (e.g., "Git repository") throughout.