-
Notifications
You must be signed in to change notification settings - Fork 245
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
fix: unquote zdotdir for starting zsh in wsl #1574
Conversation
Due to the way this command is run, the quotes are not being handled by the shell. Removing them allows them to be interpreted correctly in most cases.
WalkthroughThe pull request introduces a modification to the The primary change involves removing the quotation marks around the This modification affects how the Zsh shell integration directory path is constructed and potentially how the shell interprets the environment variable. The change is applied consistently across both functions that set the The modification does not introduce changes to the overall logic, error handling, or control flow of the functions. It is a subtle adjustment in how the path for the Zsh integration directory is specified when launching a shell environment. Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)pkg/shellexec/shellexec.go (2)
The ZDOTDIR path is now unquoted in WSL but remains quoted in remote shells. While this might be intentional due to different shell behaviors, we should verify this is the desired approach. Let's analyze the ZDOTDIR handling across different environments: ✅ Verification successfulZDOTDIR quoting differences are intentional and correct The different handling of ZDOTDIR between environments is intentional and properly implemented:
Each approach is appropriate for its specific context:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check how ZDOTDIR is handled in different contexts
echo "WSL ZDOTDIR handling:"
rg -B 5 -A 5 'ZDOTDIR.*waveterm' --glob 'pkg/**/*wsl*.{go,rs,js,ts}'
echo "Remote ZDOTDIR handling:"
rg -B 5 -A 5 'ZDOTDIR.*waveterm' --glob 'pkg/**/*ssh*.{go,rs,js,ts}'
echo "Related issues or discussions:"
rg -i 'zdotdir.*(wsl|remote|ssh)' --type md
Length of output: 370 Script: #!/bin/bash
# Let's try a broader search for ZDOTDIR usage across the codebase
echo "All ZDOTDIR occurrences:"
rg -B 5 -A 5 'ZDOTDIR' pkg/
# Also check for shell-specific configurations
echo -e "\nShell configuration related code:"
rg -B 5 -A 5 'shellOpts.*=' pkg/
Length of output: 9294
While removing quotes from ZDOTDIR may fix the immediate WSL issue, we should verify the behavior with paths containing special characters. Let's check how other environment variables are handled in WSL and if there are any existing issues with special characters in paths: Consider adding a comment explaining why ZDOTDIR is handled differently in WSL compared to remote shells (where it remains quoted) to prevent future regressions. ✅ Verification successfulUnquoted ZDOTDIR in WSL is safe and consistent with codebase patterns After analyzing the codebase, I can confirm that the unquoted ZDOTDIR in WSL is the correct approach:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check for other environment variable handling patterns in the codebase
rg -A 2 'shellOpts = append\(shellOpts, fmt\.Sprintf\(`.*=.*`.*\)\)'
# Look for any existing issues or discussions about path quoting in WSL
rg -i 'wsl.*path.*quot(e|ing)'
rg -i 'zdotdir.*path.*quot(e|ing)'
Length of output: 376 Script: #!/bin/bash
# Let's check how ZDOTDIR is handled in other shell execution contexts
ast-grep --pattern 'ZDOTDIR=$_'
# Check for any WSL-specific path handling patterns
rg -B 2 -A 2 'wsl.*path'
# Look for other environment variable settings in shell execution
rg -B 2 -A 2 'shellOpts.*=.*append.*env'
# Check for any comments or documentation about WSL path handling
rg -l -i 'wsl.*path'
Length of output: 1138 Script: #!/bin/bash
# Let's examine the WSL utility file for path handling patterns
cat pkg/wsl/wsl-util.go
# Let's also check the full context of shell execution in shellexec.go
cat pkg/shellexec/shellexec.go
Length of output: 23993 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Due to the way this command is run, the quotes are not being handled by the shell. Removing them allows them to be interpreted correctly in most cases. This resolves #1569