chore: Optimize PowerShell profile modification tool#18187
chore: Optimize PowerShell profile modification tool#18187AkariiinMKII wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
bucket/scoop-completion.json (1)
18-46: 📐 Maintainability & Code Quality | 🔵 TrivialPlease run the standard Scoop manifest checks before merge.
As per path instructions, test this manifest locally with the usual Scoop flow and include an uninstall pass, since this PR changes both
post_installandpre_uninstall.scoop config debug true scoop config gh_token <your-github-token> # optional; needed only for GitHub release API access .\bin\checkver.ps1 -App scoop-completion -f .\bin\formatjson.ps1 -App scoop-completion scoop install bucket\scoop-completion.json -a <architecture> scoop uninstall scoop-completion🤖 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 `@bucket/scoop-completion.json` around lines 18 - 46, The manifest change touches both post_install and pre_uninstall in scoop-completion, so it needs the standard Scoop validation flow before merge. Verify the manifest locally using the usual Scoop checks via checkver.ps1 and formatjson.ps1, then test a real install and uninstall cycle for scoop-completion to confirm the new profile-modification cleanup works as expected.Source: Path instructions
🤖 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 `@scripts/ModifyPSProfile.psm1`:
- Around line 140-144: The profile cleanup logic in ModifyPSProfile should only
remove the exact generated line, not any matching text anywhere in the file or
surrounding whitespace. Update the matching/removal in the script’s
profile-editing flow to anchor the regex to the generated line from the Content
value, and avoid using a global trim on the full profile so unrelated user
formatting is preserved. Use the existing $RawProfile, $escapedContent, and
$modifiedProfile flow to make the deletion precise and limited to the generated
import line.
- Around line 46-54: The generated Import-Module command in ModifyPSProfile is
emitting $UtilsPath unquoted, which breaks when the Scoop path contains spaces.
Update the command construction in the ModifyPSProfile logic so the path is
wrapped as a single quoted argument when building $ImportUtilsCommand, and keep
the add/remove script generation consistent by verifying the resulting
Import-Module string is safe for paths with spaces.
- Around line 58-73: The file I/O operations in the profile modification helpers
are not terminating, so failures can slip past the try/catch and still report
success. Update the file read/write calls in New-ProfileModifier,
Add-ProfileContent, and Remove-ProfileContent—especially the Out-File,
Add-Content, and Get-Content usages—to use -ErrorAction Stop so any I/O error is
caught by the existing catch blocks.
- Around line 96-100: The profile write path in ModifyPSProfile should ensure
the parent directory exists before writing, since Out-File -Force still fails
when $PROFILE points to a missing folder. Update the logic around Test-Path,
Add-Content, and Out-File to create the $PROFILE parent directory first, then
proceed with writing the profile content so first-run users do not hit the catch
block.
---
Nitpick comments:
In `@bucket/scoop-completion.json`:
- Around line 18-46: The manifest change touches both post_install and
pre_uninstall in scoop-completion, so it needs the standard Scoop validation
flow before merge. Verify the manifest locally using the usual Scoop checks via
checkver.ps1 and formatjson.ps1, then test a real install and uninstall cycle
for scoop-completion to confirm the new profile-modification cleanup works as
expected.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06611a41-34ad-4589-9f1f-f9668a957203
📒 Files selected for processing (2)
bucket/scoop-completion.jsonscripts/ModifyPSProfile.psm1
|
/verify |
|
All changes look good. Wait for review from human collaborators. scoop-completion
|
1 participant
Considering that the widely used built-in PowerShell version in Windows 10 is 5.1, and I'm not able to test on version 5.0, the required version has been upgraded to 5.1.
<manifest-name[@version]|chore>: <general summary of the pull request>