Windows 终端体验优化：友好名称、WSL 检测、shell 重构#294
Open
nothingness0db wants to merge 10 commits into
Open
Conversation
|
Warning Review limit reached
More reviews will be available in 59 minutes and 59 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (31)
📝 WalkthroughWalkthroughImplements cross-platform shell detection and per-shell init script generation, refactors PTY shell-injection and cleanup, updates SQLite pragmas, adds platform-aware Git polling and scoped file-watcher invalidation, and memoizes sidebar components. ChangesShell Infrastructure & Initialization
Frontend Terminal UI & Performance Optimization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
nothingness0db
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/layout/sidebar/ProjectMenuItem.tsx (1)
36-329:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
prefer-arrow-callbacklint inProjectMenuItem
memo(function ProjectMenuItem ...)is flagged; switch to arrow callback form (optionally adddisplayName).Proposed fix
-export const ProjectMenuItem = memo(function ProjectMenuItem({ +export const ProjectMenuItem = memo(({ project, projectGroups, }: { project: ProjectWithProfiles; projectGroups: ProjectGroup[]; -}) { +}) => { // ... -}); +}); + +ProjectMenuItem.displayName = "ProjectMenuItem";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/layout/sidebar/ProjectMenuItem.tsx` around lines 36 - 329, Replace the named function passed to memo with an arrow function to satisfy prefer-arrow-callback: change memo(function ProjectMenuItem({ project, projectGroups }: { project: ProjectWithProfiles; projectGroups: ProjectGroup[] }) { ... }) to memo(({ project, projectGroups }: { project: ProjectWithProfiles; projectGroups: ProjectGroup[] }) => { ... }); after the memo call assign ProjectMenuItem.displayName = "ProjectMenuItem" so the component keeps a readable name in devtools; keep the exported identifier export const ProjectMenuItem as-is and preserve all existing inner logic and hooks.
🧹 Nitpick comments (1)
src/features/watcher/fileWatcher.test.ts (1)
32-33: ⚡ Quick winAdd a test for the new scoped invalidation branch.
Lines 32-33 force
getQueryDatatoundefined, so the suite currently only validates fallback-wide invalidation. Please add one case where cached projects exist and an event includesproject_id, then assert invalidation is scoped to affected profile/folder keys.Proposed test addition
+import { queryKeys } from "`@/shared/lib/queryKeys`"; + +it("invalidates only affected project/profile queries when projects cache is present", async () => { + const channel = await loadWatcher(); + getQueryDataMock.mockReturnValue([ + { id: "p1", folder: "/repo/a", profiles: [{ id: "prof-a" }] }, + { id: "p2", folder: "/repo/b", profiles: [{ id: "prof-b" }] }, + ]); + + channel.onmessage?.({ project_id: "p2" } as never); + vi.advanceTimersByTime(1000); + + expect(invalidateQueriesMock).toHaveBeenCalledWith({ + queryKey: queryKeys.git.diff("prof-b"), + }); + expect(invalidateQueriesMock).toHaveBeenCalledWith({ + queryKey: queryKeys.git.diffStats("prof-b"), + }); + expect(invalidateQueriesMock).toHaveBeenCalledWith({ + queryKey: queryKeys.git.status("prof-b"), + }); + expect(invalidateQueriesMock).toHaveBeenCalledWith({ + queryKey: queryKeys.git.log("prof-b"), + }); + expect(invalidateQueriesMock).toHaveBeenCalledWith({ + queryKey: queryKeys.fs.tree("/repo/b"), + }); +});Also applies to: 48-89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/watcher/fileWatcher.test.ts` around lines 32 - 33, Add a test case in fileWatcher.test.ts that covers the scoped invalidation branch: instead of forcing getQueryDataMock to return undefined, have getQueryDataMock return a cached projects array (include entries with project_id values) and craft an event object that includes project_id; then call the same handler used elsewhere in the suite (the test uses getQueryDataMock and invalidateQueriesMock) and assert invalidateQueriesMock is invoked with the scoped keys for the affected profile/folder (not the fallback-wide key). Reference getQueryDataMock and invalidateQueriesMock (and the file watcher handler used in the suite) to locate where to add the new case and mirror existing test structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/crates/infra/scripts/shellIntegration-bash.sh`:
- Around line 104-109: The current negated if (! builtin eval
"$VSCODE_PYTHON_BASH_ACTIVATE") records $__vsc_activation_status=$? after the
negation, so it captures the exit status of the if-test instead of the actual
builtin eval failure; change the control flow to run builtin eval non-negated
and capture its exit code in the else path (for example: if builtin eval
"$VSCODE_PYTHON_BASH_ACTIVATE"; then :; else __vsc_activation_status=$?; builtin
printf ... "$__vsc_activation_status"; fi), ensuring you reference
VSCODE_PYTHON_BASH_ACTIVATE, builtin eval and $__vsc_activation_status when
making the change.
In `@src-tauri/crates/infra/src/pty.rs`:
- Around line 131-174: parse_shell_command currently round-trips the input
through split_whitespace(), which breaks quoted args and spaces (e.g., quoted
program paths or distro names like "Ubuntu 24.04"); stop flattening and use a
real tokenizer or keep program+args structural. Replace the split_whitespace
logic in parse_shell_command with a proper shell-aware split (for example using
a crate like shell_words or shlex) to produce Vec<String> tokens that preserve
quotes/escapes, then select the first token as program and the rest as args (or
change callers to accept the Vec<String> directly); remove the manual
path-reconstruction logic that joins tokens and instead rely on the shell-aware
tokens so quoted paths such as "C:\Program Files\PowerShell\7\pwsh.exe" and
multi-word args survive intact.
- Around line 183-195: The Bash and Zsh branches currently drop the parsed
existing_args, causing user flags like `--login`/`-l` to be lost; update the
ShellInjection::Bash and ShellInjection::Zsh handling to include the
existing_args when constructing the CommandBuilder (created via
CommandBuilder::new(program)) instead of discarding them — e.g., append or
extend the CommandBuilder with existing_args before/after adding the
integration-specific flag (`--init-file` for Bash or `-i` for Zsh) so
user-supplied flags are preserved.
In `@src-tauri/crates/infra/src/shell_init.rs`:
- Around line 126-129: The current HOME lookup returns early if HOME is missing,
causing Windows runs to skip wrapper creation; update the logic that sets the
home PathBuf (the match on std::env::var("HOME") that produces `home` and
`PathBuf::from(h)`) to try Windows fallbacks: first try
std::env::var("USERPROFILE"), then if that is missing try concatenating
std::env::var("HOMEDRIVE") and std::env::var("HOMEPATH") (only use the parts
that exist), and only return early if none of these produce a non-empty path;
ensure the final value remains a PathBuf and preserve the existing early-return
behavior when all options are absent.
- Around line 155-162: The current code interpolates notify.to_string_lossy()
into a raw JSON string which breaks on Windows due to backslashes; instead build
the settings object and serialize it with serde_json (or serde_json::json!) so
the path is properly escaped: collect notify.to_string_lossy().to_string() into
a variable and use it as the "command" value inside a Rust data structure or
serde_json::Value for the hooks (refer to the local variables settings and
notify and the JSON structure being written), then call
serde_json::to_string_pretty (or to_string) and write that output to settings;
add the necessary serde_json import and handle serialization errors similarly to
the current write handling.
In `@src-tauri/src/handler/shell.rs`:
- Around line 5-9: The current call to
tauri::async_runtime::spawn_blocking(infra::shell_detect::load_available_shells).await.unwrap_or_default()
swallows join errors and returns an ambiguous empty list; change it to
explicitly handle the Result returned by await, log the join/error (use the
existing logger or e.g. processLogger) when the task fails or panics, and only
then return a sensible default; update the code around the spawn_blocking call
(the invocation of infra::shell_detect::load_available_shells and the
unwrap_or_default usage) to match this explicit error handling and logging so
failures are visible while preserving the fallback behavior.
In `@src/features/terminal/Terminal.tsx`:
- Around line 42-49: The friendlyShellName function currently maps any string
containing "wsl" to "WSL" and only matches "cmd.exe" for Command Prompt; update
friendlyShellName to (1) broaden cmd detection to match "cmd" with or without
".exe" (e.g., check lower.includes("cmd") or use /\bcmd(?:\.exe)?\b/) and (2)
preserve WSL distro labels by extracting a distro token when present (e.g.,
parse patterns like "wsl -d DistroName", "wsl.exe -d DistroName", or common
distro names appearing alongside "wsl") and return `WSL (DistroName)` instead of
collapsing to "WSL"; keep the existing PowerShell/PowerShell Core and Git Bash
checks and fall back to null if no friendly name can be determined.
---
Outside diff comments:
In `@src/layout/sidebar/ProjectMenuItem.tsx`:
- Around line 36-329: Replace the named function passed to memo with an arrow
function to satisfy prefer-arrow-callback: change memo(function
ProjectMenuItem({ project, projectGroups }: { project: ProjectWithProfiles;
projectGroups: ProjectGroup[] }) { ... }) to memo(({ project, projectGroups }: {
project: ProjectWithProfiles; projectGroups: ProjectGroup[] }) => { ... });
after the memo call assign ProjectMenuItem.displayName = "ProjectMenuItem" so
the component keeps a readable name in devtools; keep the exported identifier
export const ProjectMenuItem as-is and preserve all existing inner logic and
hooks.
---
Nitpick comments:
In `@src/features/watcher/fileWatcher.test.ts`:
- Around line 32-33: Add a test case in fileWatcher.test.ts that covers the
scoped invalidation branch: instead of forcing getQueryDataMock to return
undefined, have getQueryDataMock return a cached projects array (include entries
with project_id values) and craft an event object that includes project_id; then
call the same handler used elsewhere in the suite (the test uses
getQueryDataMock and invalidateQueriesMock) and assert invalidateQueriesMock is
invoked with the scoped keys for the affected profile/folder (not the
fallback-wide key). Reference getQueryDataMock and invalidateQueriesMock (and
the file watcher handler used in the suite) to locate where to add the new case
and mirror existing test structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2808f7ad-eef8-467e-a70c-cf8ef2db1e73
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.locksrc-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
package.jsonsrc-tauri/.gitignoresrc-tauri/Cargo.tomlsrc-tauri/capabilities/default.jsonsrc-tauri/crates/infra/scripts/default_init_common.shsrc-tauri/crates/infra/scripts/default_init_fish.fishsrc-tauri/crates/infra/scripts/default_init_pwsh.ps1src-tauri/crates/infra/scripts/default_init_zsh.shsrc-tauri/crates/infra/scripts/shellIntegration-bash.shsrc-tauri/crates/infra/scripts/shellIntegration-env.zshsrc-tauri/crates/infra/scripts/shellIntegration-login.zshsrc-tauri/crates/infra/scripts/shellIntegration-profile.zshsrc-tauri/crates/infra/scripts/shellIntegration-rc.zshsrc-tauri/crates/infra/scripts/shellIntegration.fishsrc-tauri/crates/infra/scripts/shellIntegration.ps1src-tauri/crates/infra/src/config.rssrc-tauri/crates/infra/src/db.rssrc-tauri/crates/infra/src/git.rssrc-tauri/crates/infra/src/lib.rssrc-tauri/crates/infra/src/no_window.rssrc-tauri/crates/infra/src/pty.rssrc-tauri/crates/infra/src/shell_detect.rssrc-tauri/crates/infra/src/shell_init.rssrc-tauri/crates/service/src/pty.rssrc-tauri/src/handler/shell.rssrc-tauri/src/handler/topbar.rssrc-tauri/src/handler/updater.rssrc-tauri/src/lib.rssrc/features/git/hooks.tssrc/features/projects/components/ProjectTemplatesEditor.tsxsrc/features/projects/hooks.tssrc/features/settings/GlobalTerminalTemplatesSettings.tsxsrc/features/terminal/Terminal.tsxsrc/features/terminal/TerminalTabs.tsxsrc/features/terminal/keybindings.test.tssrc/features/terminal/keybindings.tssrc/features/watcher/fileWatcher.test.tssrc/features/watcher/fileWatcher.tssrc/layout/sidebar/ProfileItem.tsxsrc/layout/sidebar/ProjectGroupSection.tsxsrc/layout/sidebar/ProjectMenuItem.tsx
💤 Files with no reviewable changes (1)
- src-tauri/crates/infra/scripts/default_init_common.sh
nothingness0db
force-pushed
the
perf/windows-optimization
branch
from
a95f30c to
ce7d71e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/terminal/Terminal.tsx (1)
392-447:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
shelltoterminalRefdependencies (or otherwise ensureonTitleChangesees the latestshell)
terminalRefis created viauseCallbackwith dependencies[decreaseFontSize, handleTerminalLinkOpen, increaseFontSize, profileId, sessionId, syncTerminalLayout], but it readsshellto computeshellFriendlyNameinsideterm.onTitleChange(...). Withshellomitted, the title handler can keep using a stale friendly-name mapping ifshellchanges for an existing session.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/terminal/Terminal.tsx` around lines 392 - 447, The title-change handler registered on term via term.onTitleChange uses the outer-scope variable shell (and computes shellFriendlyName), but terminalRef's useCallback dependencies do not include shell, causing stale values; update the useCallback that defines terminalRef to include shell in its dependency array (or alternatively read shell from a ref like shellRef.current inside the handler), ensuring term.onTitleChange sees the latest shell value and calling useTerminalStore.getState().updateTabTitle(profileId, sessionId, shellFriendlyName) uses the current friendly name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/crates/infra/scripts/default_init_pwsh.ps1`:
- Around line 10-11: Replace the substring check against $env:PATH with an
exact-path-element check: split $env:PATH using [System.IO.Path]::PathSeparator
into components, perform a case-insensitive equality comparison for each element
against $env:_2CODE_BIN, and only prepend
"$env:_2CODE_BIN$([System.IO.Path]::PathSeparator)$env:PATH" when no exact match
is found; update the conditional that currently uses -notlike
"*$env:_2CODE_BIN*" to use this element membership test so partial matches
inside other PATH entries won't block insertion.
In `@src/layout/sidebar/ProfileItem.tsx`:
- Line 13: The component is wrapped with memo using a function expression which
triggers the ESLint prefer-arrow-callback rule; change the memo call to use an
arrow function instead. Replace memo(function ProfileItem({ ... }) { ... }) with
memo((props) => { /* keep same body but use props or destructure inside */ })
and ensure the exported symbol remains ProfileItem (e.g., const ProfileItem =
memo((props) => { ... }); export { ProfileItem };), preserving all existing
props, types, and displayName if set.
In `@src/layout/sidebar/ProjectGroupSection.tsx`:
- Line 21: ESLint flags use of function expressions inside React.memo; replace
the anonymous function passed to memo with an arrow function to satisfy
prefer-arrow-callback. Find the memo(...) calls for ProjectGroupSection,
ProfileItem, and ProjectMenuItem and change memo(function Name({...}) { ... })
to memo((props) => { ... }) or memo(({ ... }) => { ... }) while preserving the
component name (export const ProjectGroupSection = memo((props) => { ... })
etc.), keeping the same props, return value, and any memoization logic.
In `@src/layout/sidebar/ProjectMenuItem.tsx`:
- Line 36: The component is currently wrapped as memo(function
ProjectMenuItem(...)) which is fine but the review points out an attempted
change to an arrow form was written with incorrect syntax; to fix, either keep
the existing named function style export const ProjectMenuItem = memo(function
ProjectMenuItem(props) { ... }) or convert to a correct arrow form by wrapping
the props destructure in an arrow: export const ProjectMenuItem = memo(({ ... })
=> { ... }); update the ProjectMenuItem declaration (and any other memo(...)
usages in this file) to use one of these valid syntaxes so the code compiles and
memo is applied correctly.
---
Outside diff comments:
In `@src/features/terminal/Terminal.tsx`:
- Around line 392-447: The title-change handler registered on term via
term.onTitleChange uses the outer-scope variable shell (and computes
shellFriendlyName), but terminalRef's useCallback dependencies do not include
shell, causing stale values; update the useCallback that defines terminalRef to
include shell in its dependency array (or alternatively read shell from a ref
like shellRef.current inside the handler), ensuring term.onTitleChange sees the
latest shell value and calling
useTerminalStore.getState().updateTabTitle(profileId, sessionId,
shellFriendlyName) uses the current friendly name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99010373-3f45-4751-907e-8d224e7312b6
⛔ Files ignored due to path filters (5)
src/generated/.typecacheis excluded by!**/generated/**src/generated/commands.tsis excluded by!**/generated/**src/generated/events.tsis excluded by!**/generated/**src/generated/index.tsis excluded by!**/generated/**src/generated/types.tsis excluded by!**/generated/**
📒 Files selected for processing (18)
src-tauri/crates/infra/scripts/default_init_fish.fishsrc-tauri/crates/infra/scripts/default_init_pwsh.ps1src-tauri/crates/infra/src/db.rssrc-tauri/crates/infra/src/lib.rssrc-tauri/crates/infra/src/no_window.rssrc-tauri/crates/infra/src/shell_detect.rssrc-tauri/crates/infra/src/shell_init.rssrc-tauri/crates/service/src/pty.rssrc-tauri/src/handler/shell.rssrc/features/git/hooks.tssrc/features/projects/hooks.tssrc/features/terminal/Terminal.tsxsrc/features/terminal/TerminalTabs.tsxsrc/features/watcher/fileWatcher.test.tssrc/features/watcher/fileWatcher.tssrc/layout/sidebar/ProfileItem.tsxsrc/layout/sidebar/ProjectGroupSection.tsxsrc/layout/sidebar/ProjectMenuItem.tsx
✅ Files skipped from review due to trivial changes (1)
- src-tauri/crates/infra/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (10)
- src/features/terminal/TerminalTabs.tsx
- src/features/watcher/fileWatcher.test.ts
- src-tauri/crates/infra/scripts/default_init_fish.fish
- src/features/watcher/fileWatcher.ts
- src-tauri/crates/infra/src/db.rs
- src-tauri/crates/infra/src/no_window.rs
- src/features/projects/hooks.ts
- src-tauri/crates/infra/src/shell_detect.rs
- src/features/git/hooks.ts
- src-tauri/crates/infra/src/shell_init.rs
nothingness0db
force-pushed
the
perf/windows-optimization
branch
from
ce7d71e to
3e4a0ce
Compare
- shell 检测逻辑从 handler/shell.rs 搬到 infra/shell_detect.rs - extract_exe() 处理 Windows 带空格路径 - setup_2code_home() 为 fish/pwsh 创建 notify hook - shell picker 和终端标签页显示友好名称 - WSL 发行版检测 (wsl -l -q) - git 轮询间隔 Windows 上从 1s 调到 4s - SQLite PRAGMA 优化 - sidebar 组件 React.memo
nothingness0db
force-pushed
the
perf/windows-optimization
branch
from
3e4a0ce to
2889114
Compare
Contributor
Author
CodeRabbit Review 回复已修复
不修
|
❌ Deploy Preview for grand-selkie-225f87 failed. Why did it fail? →
|
added 6 commits
May 29, 2026 15:51
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/features/terminal/Terminal.tsx (1)
45-49: ⚡ Quick winFriendly names are user-facing tab titles but not localized.
These strings render as tab titles, yet they bypass Paraglide. Brand tokens like "Git Bash"/"WSL" may stay verbatim, but route them through
m.*so they participate in the i18n pipeline.As per coding guidelines: "Use i18n via Paraglide.js with
import * as m from '@/paraglide/messages.js'pattern for all user-facing strings."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/terminal/Terminal.tsx` around lines 45 - 49, The user-facing hardcoded tab title strings in Terminal.tsx (the returns in the function that maps shell names: the lines returning "PowerShell 7", "Windows PowerShell", "Command Prompt", "WSL", "Git Bash") must be routed through Paraglide i18n; import the messages module (import * as m from '`@/paraglide/messages.js`') and replace those literal return values with message lookups (e.g., m.terminal.powerShell7, m.terminal.windowsPowerShell, m.terminal.commandPrompt, m.terminal.wsl, m.terminal.gitBash or similarly named keys), adding the corresponding keys to messages.js so these tab titles participate in the i18n pipeline. Ensure you update any consumers/tests that expect the literal strings if needed.src-tauri/crates/infra/src/shell_init.rs (1)
493-534: ⚡ Quick winFish/Pwsh tests write into the developer's real
$HOME/.2code.
prepare_fish/prepare_pwshcallsetup_2code_home(), so these tests create~/.2code/hooks/notify.sh,~/.2code/hooks/claude-settings.json, and~/.2code/bin/claudeunder the actual home directory of whoever runs the suite (dev or CI). The tests don't assert on those files, so the writes are a pure side effect that pollutes the environment. Consider extracting asetup_2code_home_in(base: &Path)seam so the public function resolves the home dir and the tests can point it at a temp dir.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/crates/infra/src/shell_init.rs` around lines 493 - 534, The tests call prepare_shell_injection (via prepare_fish/prepare_pwsh) which uses setup_2code_home() and so mutates the real $HOME/.2code; add a seam by extracting a new helper setup_2code_home_in(base: &Path) that performs the actual directory/file creation, update setup_2code_home() to resolve the real home and forward to setup_2code_home_in, and update the tests to create a tempdir (e.g., via tempfile::tempdir()), call the new setup_2code_home_in or pass that base into prepare_shell_injection (or provide an overload that accepts a base path) so the test writes go into the tempdir instead of the real home, then clean up the tempdir at test end.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/crates/infra/src/shell_init.rs`:
- Around line 54-72: The loop in extract_exe incorrectly treats Path::exists()
as a signal the candidate is an executable, which returns true for directories
and truncates paths like "C:\Program Files (x86)\Git\bin\bash.exe"; update the
candidate existence check inside the while loop (the block that currently
branches on lower.ends_with(...) || Path::new(&candidate).exists()) to require a
regular file (use Path::new(&candidate).is_file()) instead of exists(), leaving
the other extension checks intact so the full executable path (e.g., bash.exe)
is still matched on the next iterations; ensure this change is applied where
extract_exe builds candidates so detect_shell_type downstream keeps correct
values.
---
Nitpick comments:
In `@src-tauri/crates/infra/src/shell_init.rs`:
- Around line 493-534: The tests call prepare_shell_injection (via
prepare_fish/prepare_pwsh) which uses setup_2code_home() and so mutates the real
$HOME/.2code; add a seam by extracting a new helper setup_2code_home_in(base:
&Path) that performs the actual directory/file creation, update
setup_2code_home() to resolve the real home and forward to setup_2code_home_in,
and update the tests to create a tempdir (e.g., via tempfile::tempdir()), call
the new setup_2code_home_in or pass that base into prepare_shell_injection (or
provide an overload that accepts a base path) so the test writes go into the
tempdir instead of the real home, then clean up the tempdir at test end.
In `@src/features/terminal/Terminal.tsx`:
- Around line 45-49: The user-facing hardcoded tab title strings in Terminal.tsx
(the returns in the function that maps shell names: the lines returning
"PowerShell 7", "Windows PowerShell", "Command Prompt", "WSL", "Git Bash") must
be routed through Paraglide i18n; import the messages module (import * as m from
'`@/paraglide/messages.js`') and replace those literal return values with message
lookups (e.g., m.terminal.powerShell7, m.terminal.windowsPowerShell,
m.terminal.commandPrompt, m.terminal.wsl, m.terminal.gitBash or similarly named
keys), adding the corresponding keys to messages.js so these tab titles
participate in the i18n pipeline. Ensure you update any consumers/tests that
expect the literal strings if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 402886bc-b052-487c-98df-10bf323c48f7
⛔ Files ignored due to path filters (5)
src/generated/.typecacheis excluded by!**/generated/**src/generated/commands.tsis excluded by!**/generated/**src/generated/events.tsis excluded by!**/generated/**src/generated/index.tsis excluded by!**/generated/**src/generated/types.tsis excluded by!**/generated/**
📒 Files selected for processing (19)
src-tauri/crates/infra/scripts/default_init_fish.fishsrc-tauri/crates/infra/scripts/default_init_pwsh.ps1src-tauri/crates/infra/src/db.rssrc-tauri/crates/infra/src/lib.rssrc-tauri/crates/infra/src/no_window.rssrc-tauri/crates/infra/src/shell_detect.rssrc-tauri/crates/infra/src/shell_init.rssrc-tauri/crates/service/src/pty.rssrc-tauri/src/handler/shell.rssrc/features/git/hooks.tssrc/features/projects/hooks.tssrc/features/terminal/Terminal.tsxsrc/features/terminal/TerminalTabs.tsxsrc/features/watcher/fileWatcher.test.tssrc/features/watcher/fileWatcher.tssrc/layout/sidebar/ProfileItem.tsxsrc/layout/sidebar/ProjectGroupSection.tsxsrc/layout/sidebar/ProjectMenuItem.tsxsrc/shared/lib/queryKeys.test.ts
✅ Files skipped from review due to trivial changes (1)
- src-tauri/crates/infra/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (16)
- src/shared/lib/queryKeys.test.ts
- src-tauri/crates/infra/src/db.rs
- src/features/watcher/fileWatcher.test.ts
- src/layout/sidebar/ProjectGroupSection.tsx
- src/features/projects/hooks.ts
- src/features/terminal/TerminalTabs.tsx
- src/layout/sidebar/ProfileItem.tsx
- src-tauri/crates/infra/scripts/default_init_pwsh.ps1
- src/layout/sidebar/ProjectMenuItem.tsx
- src-tauri/crates/service/src/pty.rs
- src-tauri/crates/infra/src/no_window.rs
- src-tauri/crates/infra/scripts/default_init_fish.fish
- src/features/watcher/fileWatcher.ts
- src-tauri/src/handler/shell.rs
- src/features/git/hooks.ts
- src-tauri/crates/infra/src/shell_detect.rs
Contributor
Author
|
@coderabbitai 这两个 nitpick 有点虚：
Actionable 那个 |
|
Tip For best results, initiate chat on the files or code changes.
🐇✨ ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
改了什么
shell 检测重构
把
handler/shell.rs里的 shell 枚举逻辑搬到infra/shell_detect.rs，handler 只剩 10 行委托代码。新增supports_integration字段，cmd.exe 和 WSL 标记为不支持集成，其余标记为支持。友好名称
shell picker 和终端标签页显示「PowerShell 7」「Command Prompt」「Git Bash」，不再显示完整 exe 路径。标签页的
onTitleChange回调改为从 shell 配置生成友好名称，忽略 shell 设的工作目录路径。WSL 发行版检测
跑
wsl -l -q拿已安装的发行版名，每个发行版单独一条，显示为「Ubuntu (WSL)」。输出是 UTF-16LE，做了 BOM 检测和解码。Windows 终端修复
extract_exe()处理 Windows 带空格路径（C:\Program Files\PowerShell\7\pwsh.exe）setup_2code_home()给 fish 和 pwsh 创建 notify hookdefault_init_fish.fish和default_init_pwsh.ps1，设置 2code 环境变量和 PATH\x1b[1;1RCPR 前缀，保留 1s sleep 做 PowerShell 同步和上游的冲突
上游做了 tab 格式化、加了 codex 支持和 notes editor，和我们的改动在不同区域，没有实质冲突。rebase 可以直接合并。
测试
Summary by CodeRabbit
New Features
Bug Fixes
Performance
UI
Tests