Skip to content

fix(pwsh): self-build scripts set UTF-8 console so CLI emojis render#98

Merged
PrzemyslawKlys merged 1 commit intomainfrom
fix/powershell-utf8-selfbuild
Feb 10, 2026
Merged

fix(pwsh): self-build scripts set UTF-8 console so CLI emojis render#98
PrzemyslawKlys merged 1 commit intomainfrom
fix/powershell-utf8-selfbuild

Conversation

@PrzemyslawKlys
Copy link
Member

Fixes garbled Summary emojis when running Module\Build\Build-ModuleSelf.ps1 from Windows PowerShell by switching the console codepage to UTF-8 for the duration of the self-build run and restoring it afterward.\n\nAlso removes raw emoji literals from build scripts (uses [char] codes) so PS5.1 doesn't misread UTF-8-no-BOM scripts.

@claude
Copy link

claude bot commented Feb 10, 2026

Pull Request Review: Fix UTF-8 console encoding for emoji rendering

Summary

This PR addresses garbled emoji rendering when running build scripts from Windows PowerShell by temporarily switching the console codepage to UTF-8 and replacing raw emoji literals with character codes.

Code Quality & Best Practices ✅

Strengths:

  • Well-structured approach: The solution correctly addresses the root cause (console encoding) rather than just removing emojis
  • Proper cleanup: Uses try-finally to ensure console settings are restored even if errors occur
  • Defensive programming: Checks for redirected output before modifying console settings
  • Consistent implementation: All three files apply the same pattern appropriately
  • Good comments: Clear explanation of why the change is needed (lines 27-28 in Build-ModuleSelf.ps1)

Minor suggestions:

  1. Variable initialization clarity (Build-ModuleSelf.ps1:19-21): While setting to null is fine, consider documenting why these need to be script-scope
  2. Error suppression (line 42): The empty catch block with best effort only comment is reasonable for console encoding, but consider logging to a debug stream if available

Potential Issues 🟡

1. Silent failure on encoding setup

If the encoding setup fails, the script continues but emojis may still be garbled. Consider adding Write-Debug in the catch block.

2. Codepage restoration (Build-ModuleSelf.ps1:137)

The check if ($oldConsoleCodePage) will be false if the original codepage was 0 (unlikely but theoretically possible). Use explicit null check: if ($null -ne $oldConsoleCodePage)

3. Character code compatibility (Build-PowerForge.ps1:23)

The comment no VS16 suggests this is intentionally the base emoji without variation selector. This is correct for PS5.1 compatibility.

Performance Considerations ✅

  • The codepage switching occurs once per script execution
  • Character code lookups are negligible
  • No performance concerns identified

Security Concerns ✅

  • Using cmd /c chcp with numeric formatting (no injection risk)
  • No external input processed through encoding changes
  • No security concerns identified

Test Coverage 🟡

No automated tests for this functionality. Consider adding manual test cases to verify emoji rendering in PS5.1 on Windows and that console encoding is restored after script execution.

Additional Observations

Positive patterns:

  1. Consistent style: All emoji replacements follow the same pattern with inline comments
  2. Scope appropriateness: UTF-8 encoding is only set in Build-ModuleSelf.ps1, which is correct since only the self-build invokes PowerForge.Cli.exe
  3. BOM-free UTF-8: Using New-Object System.Text.UTF8Encoding($false) correctly creates UTF-8 without BOM

Questions for author:

  1. Have you tested this on Windows PowerShell 5.1, PowerShell Core 7+, and different Windows console hosts?
  2. Does the chcp 65001 command output suppression work correctly in all environments?

Overall Assessment

This is a well-implemented fix for a legitimate issue. The approach is sound, the code is clean, and the implementation is consistent. The minor suggestions above are refinements rather than blockers.

Recommendation: Approve with minor suggestions

The PR successfully addresses the stated problem and follows repository conventions. The suggestions around error handling and null checking are defensive improvements that would make the code more robust but are not critical for merging.


Review generated with assistance from Claude Code

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 459403a135

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

function Write-Ok($t) { Write-Host "✅ $t" -ForegroundColor Green }
function Write-Step($t) { Write-Host "🛠️ $t" -ForegroundColor Yellow }
function Write-Ok($t) { Write-Host ("{0} {1}" -f ([char]0x2705), $t) -ForegroundColor Green } # ✅
function Write-Step($t) { Write-Host ("{0} {1}" -f ([char]0x1F6E0), $t) -ForegroundColor Yellow } # 🛠 (no VS16)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use valid Unicode representation for wrench icon

[char] can only represent a single UTF-16 code unit, so [char]0x1F6E0 overflows and raises a conversion error at runtime. Because Write-Step is called immediately in this script, Build/Build-PowerForge.ps1 will fail before publishing starts in environments that execute this path. Please represent this symbol as a surrogate-pair string (or another safe string form) instead of a single [char] cast.

Useful? React with 👍 / 👎.

@PrzemyslawKlys PrzemyslawKlys merged commit 3b22eda into main Feb 10, 2026
3 checks passed
@PrzemyslawKlys PrzemyslawKlys deleted the fix/powershell-utf8-selfbuild branch February 10, 2026 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant