Uninstall: remove the previous release's Oh My Posh block and validate the profile only in pwsh - #123
Open
Laurent Zogaj (26zl) wants to merge 1 commit into
Open
Laurent Zogaj (26zl) wants to merge 1 commit into
Laurent Zogaj (26zl) wants to merge 1 commit into
Conversation
…e the profile only in pwsh Remove-DevConfigOhMyPoshProfile compared the profile text as read, but the block the previous signed release wrote has mixed line endings: CRLF inside the here-string (the release file is CRLF) and LF after it. Neither the LF nor the CRLF candidate matched, so nothing was removed and the untouched profile was re-parsed by the cleanup host, Windows PowerShell, which rejects the block's leading pipe; the step was flagged with "An empty pipe element is not allowed". Setup already normalizes line endings before comparing; cleanup now does the same. The post-removal parse also failed for any PowerShell 7 syntax elsewhere in the pwsh profile (for example ??), because the cleanup always runs in Windows PowerShell. Only pwsh can validate its own profile, so the check is skipped under the Desktop edition; removal is an exact match at a top-level statement start either way.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused change correctly addresses both documented cleanup failures without introducing unresolved issues.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes Oh My Posh profile cleanup compatibility when Uninstall runs under Windows PowerShell.
Changes:
- Normalizes mixed line endings before matching legacy blocks.
- Restricts profile syntax validation to PowerShell 7.
| File | Description |
|---|---|
src/windows-dev-config/steps/powershell-profile.ps1 |
Makes profile cleanup handle legacy formatting and pwsh-only syntax. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
One of my machines was set up with the flow as it was before #108, so I checked how the new Uninstall handles the Oh My Posh block that version wrote. It does not remove it, for two reasons; cleanup always runs in Windows PowerShell, which matters for both.
The old release wrote the block with mixed line endings (CRLF inside the here-string, LF after it).
Remove-DevConfigOhMyPoshProfilecompares the profile as read against a pure-LF and a pure-CRLF candidate, so nothing matches, and the untouched profile is then re-parsed by Windows PowerShell, which rejects the block's leading| Invoke-Expression. The step is flagged withAn empty pipe element is not allowed.and the block stays. Setup already normalizes line endings before comparing; cleanup does not.The post-removal validation uses the host's parser. The file is pwsh's own
$PROFILE, so anything PowerShell 7 accepts there (??,?., ternaries) is a parse error in Windows PowerShell, and the step is flagged withUnexpected token '??'while the block is left in place.The fix normalizes line endings before matching, as setup does, and runs the post-removal parse only under PowerShell 7. Removal is an exact text match at a top-level statement start, so it does not depend on that parse.
Checked on Windows 11 Pro 25H2 (build 26200.9550) with the phase file dot-sourced and the profile path stubbed, setup in pwsh 7.6.6 and cleanup in Windows PowerShell 5.1.26100: the old block (from the signed
bf74ae3file, CRLF kept), a profile with a??line plus the new block, user lines around the old block, a CRLF profile without any block, and the new block on both hosts. Before: flagged in the first three cases. After: block removed, the user lines and the??line kept, the no-block profile untouched.If you would rather keep a check under Windows PowerShell, the parse could be delegated to
pwsh -NoProfile, which is still installed at that point; happy to do that instead. Onlysrc/is touched.Thanks for having a look.