Phase 1: PS7 correctness fixes - #1
Merged
Merged
Conversation
…ette#98, KevinMarquette#97, KevinMarquette#104, KevinMarquette#66, KevinMarquette#75/KevinMarquette#88/KevinMarquette#85) - Export-PSGraph: replace Invoke-Expression with Invoke-Item for -ShowGraph so destination paths containing spaces work on PS7 (KevinMarquette#110/PR-102). - Export-PSGraph: pin $OutputEncoding to UTF8-no-BOM around the pipe to dot's stdin so an ambient BOM-emitting encoding (e.g. in CI) can't break dot's parser; verified non-ASCII labels already round-trip correctly once the BOM is gone (KevinMarquette#97, KevinMarquette#104). - Export-PSGraph: prefer a cross-platform `Get-Command dot` PATH lookup over the hardcoded path glob when -GraphVizPath isn't explicitly given; an explicit -GraphVizPath still wins outright (KevinMarquette#75, KevinMarquette#88, KevinMarquette#85). - Graph: only default the compound attribute to 'true' when the caller hasn't already supplied one, so compound=$false survives (KevinMarquette#98). - docs/Command-SubGraph.md: fix the documented SubGraph example to pass -ScriptBlock by name, matching how PowerShell's parameter-set binding actually resolves an unnamed subgraph with -Attributes (KevinMarquette#66). Added regression tests for all of the above. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AgrjdhkmUCswsFGbn3EwCg
3 tasks
4 tasks
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.
Summary
Phase 1 of
plans/modernization-roadmap.md: low-risk PS7 correctness fixes, each reproduced and verified against a real GraphViz install before/after the fix.-ShowGraphfailed on a destination path with spaces (Invoke-Expressionsplits on spaces). Swapped inInvoke-Item. Confirmed thedot.exeargument-array invocation itself already handles spaced paths correctly on PS7 via splatting, so no--%stop-parsing token is needed there.dot's parser: reproduced by setting$OutputEncodingto UTF8-with-BOM (as some CI/profile environments do), which injected a BOM into the stdin pipe todotand causedsyntax error near 'digraph'. Fixed by pinning$OutputEncodingto UTF8-no-BOM insideExport-PSGraph, independent of caller/ambient state. Verified non-ASCII labels already round-trip correctly through the pipe once the BOM is gone.Get-Command dotPATH lookup when-GraphVizPathisn't explicitly supplied, falling back to the glob; an explicit-GraphVizPathstill wins outright rather than silently falling back to PATH.Graph's begin block unconditionally setcompound = 'true', silently overriding a user-suppliedcompound=$false. Now only defaults it when the caller hasn't already supplied a value.SubGraph -Attributes @{...} { }example doesn't bind ScriptBlock (a real PowerShell positional-binding limit across the 4 parameter sets — restructuring positions to support it breaks basic positional binding elsewhere). Fixed the doc to the working call shape (-Attributes @{...} -ScriptBlock { }) instead, per the roadmap's fallback option.Also removed the now-dead
PSAvoidUsingInvokeExpressionsuppression onExport-PSGraph, since noInvoke-Expressioncall remains in the module.Test plan
Tests/Regression.Tests.ps1(compound, SubGraph binding) andTests/Export-PSGraph.Tests.ps1(spaced path, ShowGraph, BOM, non-ASCII, path detection).psm1not present in this checkout — Phase 2 CI work; 2 are pre-existing ScriptAnalyzer findings inEntity.ps1/Node.ps1untouched by this PR)dot - graphviz version 15.1.1) before and after the changeClaude-Session: https://claude.ai/code/session_01AgrjdhkmUCswsFGbn3EwCg