Bump Spectre.Console, refactor Format-SpectreTable, fix bugs, and add new features and tests #21
Conversation
I'm not really fussed on private function names being a bit verbose, it seems to describe what it does quite well the way it is. Do whatever you want with it though, I appreciate the work you've done on this 🙏 |
|
Most of the logic is in the column and row wrangling, it might be better to handle them independently i.e. set up the columns fully first then set up the row data. It could be broken out into a couple of functions like below to make the 👇 this is pseudo-code as heck, I might have missed some things that make this not work the way I think it will... End block: end {
if("nothing is in the collector") {
# bail out
}
$standardMembers = Get-DefaultDisplayMembers # add the check here for .PSTypeNames[0] -notmatch 'PSCustomObject'
if($Property) {
$collector = $collector | Select-Object -Property $Property
} elseif($standardMembers) {
$collector = $collector | Select-Object $standardMembers.Format
}
Add-TableColumns -StandardMembers $StandardMembers -Table $table -Collector $Collector
foreach ($item in $collector) {
$row = New-TableRow -Item $item -AllowMarkup $AllowMarkup
[TableExtensions]::AddRow($table, $row) # this works without casting the array of rows so no AllowMarkup check required
}
Write-AnsiConsole $table
}Functions supporting the above: # This is the bulk of the existing code in the end block
function Add-TableColumns {
param (
$standardMembers
$table,
$collector
)
if("$standardMembers are available / not null") {
# add the columns to the $table with available formatting
} elseif(Test-IsScalar $collector[0].PSObject) {
# simple/scalar types show up wonky, we can detect them and just use a dummy header for the table
$table.AddColumn("Value")
} else {
# no formatting found and no properties selected, enumerating psobject.properties.name
}
}# Check if the object type is scalar/simple
function Test-IsScalar {
param (
$Value
)
$matchedScalarTypes = $Value.PSObject.TypeNames | Where-Object {
@("System.String", "System.ValueType") -contains $_
}
return $null -ne $matchedScalarTypes
}# I should have done this when I smushed the AllowMarkup stuff into your branch, handling the allowmarkup inline makes the "end {}" block huge
function New-TableRow {
param (
[object] $Item,
[boolean] $AllowMarkup
)
$columns = @()
if(Test-IsScalar $Item) {
$columns += [string]$Item
} else {
foreach($cell in $item.PSObject.Properties) {
$Value = $cell.Value
if([string]::IsNullOrEmpty($Value)) {
$Value = " "
}
$columns += [string]$Value
}
}
return $columns | Foreach-Object {
if($AllowMarkup) {
[Markup]::new($_)
} else {
[Text]::new($_)
}
}
} |
|
The default behaviour for dumping an array of hashtables to a spectre table could also be cast to a pscustom object in the process block. I'm not sure of many situations where I would want to dump the properties of the hashtable vs it trying to output the actual hashtable entries foreach ($entry in $data) {
if($entry -is [hashtable]) {
$collector.add([pscustomobject]$entry)
} else {
$collector.add($entry)
}
}
|
|
Really like the proposed changes! |
|
Proposed changes look great IMO as well! Looking forward to trying it all out when implemented. |
|
this test here works for me, im not sure if it covers all scenarios.. (and ofcourse the pester tests) did i miss any object type we should render? I need to do some cleanup before merge, theres alot of Write-Debug stuff in the code at the moment. [System.Collections.Generic.List[object]]$GenericList = @()
1..5 | ForEach-Object { $GenericList.Add([PSCustomObject]@{ Name = (Get-Random) }) }
$sample = @{
StringArray = @("one", "two", "three")
GenericList = $GenericList
hashtable = @{
PwshSpectreConsole = "nice"
pwsh = $PSVersionTable.PSVersion
third = 'something'
}
Filesystem = Get-Childitem (Split-Path -Parent (Get-Module PwshSpectreConsole).Path)
PSCustomObject = [PSCustomObject]@{
Name = 'PwshSpectre'
Version = '0.0.1'
Description = 'A PowerShell module to display data in a table'
dummy = 'dummy'
dummy2 = 'dummy2'
dummy3 = 'dummy3'
}
markdown = @(
[PSCustomObject]@{
Name = "[green]PwshSpectre[/]"
Version = "[white]Terminated[/]"
Description = "[red]PowerShell module[/]"
}
)
}
foreach ($test in $sample.GetEnumerator()) {
Write-SpectreHost -Message "Testing $($test.Name)"
if ($Test.Name -eq 'PSCustomObject') {
Format-SpectreTable -Data $test.Value -Title 'My PSCustomObject' -Property Name, Version, Description
continue
}
if ($Test.Name -eq 'markdown') {
Format-SpectreTable -Data $test.Value -Title 'My markdown' -AllowMarkup
continue
}
Format-SpectreTable -Data $test.Value
}
|
|
There are some psstyle options I think will need to be mapped/stripped
based on it failing in one of the tests I just pushed.
I think I might have a couple of your scenarios in those tests, if I missed
some just copy paste the basic test and pipe in different input types.
For validation the best you can do is check the row counts are correct on
the returned table object because not much else is exposed publicly in
spectre console.
If you’re running the tests locally you need to run build.ps1 first to
download the spectre libs.
You can leave write debug in there. I find it convenient, I used to cmdlet
bind everything and pass $verbose preference the whole way through 🤦♂️
write-debug is much better.
…On Mon, 8 Jan 2024 at 12:48 PM, trackd ***@***.***> wrote:
this test here works for me, im not sure if it covers all scenarios..
(and ofcourse the pester tests)
did i miss any object type we should render?
I need to do some cleanup before merge, theres alot of Write-Debug stuff
in the code at the moment.
but prefer to keep it in while working on it.
[System.Collections.Generic.List[object]]$GenericList = @()1..5 | ForEach-Object { $GenericList.Add([PSCustomObject]@{ Name = (Get-Random) }) }
[System.Collections.Generic.List[System.IO.DirectoryInfo]]$Files = @()$path = $env:PSModulePath -split ([System.IO.Path]::PathSeparator) | Select-Object -First 11..5 | ForEach-Object { $Files.Add([System.IO.DirectoryInfo]"$(Join-Path $path ([string](Get-Random) + '.tmp'))") }$sample = @{
StringArray = @("one", "two", "three")
GenericList = $GenericList
hashtable = @{
PwshSpectreConsole = "nice"
pwsh = $PSVersionTable.PSVersion
third = 'something'
}
Filesystem = $Files
PSCustomObject = [PSCustomObject]@{
Name = 'PwshSpectre'
Version = '0.0.1'
Description = 'A PowerShell module to display data in a table'
dummy = 'dummy'
dummy2 = 'dummy2'
dummy3 = 'dummy3'
}
}
foreach ($test in $sample.GetEnumerator()) {
Write-SpectreHost -Message "Testing $($test.Name)"
if ($test.Name -eq 'hashtable') {
Format-SpectreTable -Data $test.Value -Title 'My hashtable'
continue
}
if ($Test.Name -eq 'PSCustomObject') {
Format-SpectreTable -Data $test.Value -Title 'My PSCustomObject' -Property Name, Version, Description
continue
}
Format-SpectreTable -Data $test.Value
}
—
Reply to this email directly, view it on GitHub
<#21 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADEMYIQPWLWR4L47BMH36KLYNMX27AVCNFSM6AAAAABBKXRFVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBQGIZDEOBSGI>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
I’ve had to take off to an appointment sorry to push broken tests and bail
😂
On Mon, 8 Jan 2024 at 1:10 PM, Shaun Lawrie ***@***.***>
wrote:
… There are some psstyle options I think will need to be mapped/stripped
based on it failing in one of the tests I just pushed.
I think I might have a couple of your scenarios in those tests, if I
missed some just copy paste the basic test and pipe in different input
types.
For validation the best you can do is check the row counts are correct on
the returned table object because not much else is exposed publicly in
spectre console.
If you’re running the tests locally you need to run build.ps1 first to
download the spectre libs.
You can leave write debug in there. I find it convenient, I used to cmdlet
bind everything and pass $verbose preference the whole way through 🤦♂️
write-debug is much better.
On Mon, 8 Jan 2024 at 12:48 PM, trackd ***@***.***> wrote:
> this test here works for me, im not sure if it covers all scenarios..
>
> (and ofcourse the pester tests)
>
> did i miss any object type we should render?
>
> I need to do some cleanup before merge, theres alot of Write-Debug stuff
> in the code at the moment.
> but prefer to keep it in while working on it.
>
> [System.Collections.Generic.List[object]]$GenericList = @()1..5 | ForEach-Object { $GenericList.Add([PSCustomObject]@{ Name = (Get-Random) }) }
> [System.Collections.Generic.List[System.IO.DirectoryInfo]]$Files = @()$path = $env:PSModulePath -split ([System.IO.Path]::PathSeparator) | Select-Object -First 11..5 | ForEach-Object { $Files.Add([System.IO.DirectoryInfo]"$(Join-Path $path ([string](Get-Random) + '.tmp'))") }$sample = @{
> StringArray = @("one", "two", "three")
> GenericList = $GenericList
> hashtable = @{
> PwshSpectreConsole = "nice"
> pwsh = $PSVersionTable.PSVersion
> third = 'something'
> }
> Filesystem = $Files
> PSCustomObject = [PSCustomObject]@{
> Name = 'PwshSpectre'
> Version = '0.0.1'
> Description = 'A PowerShell module to display data in a table'
> dummy = 'dummy'
> dummy2 = 'dummy2'
> dummy3 = 'dummy3'
> }
> }
> foreach ($test in $sample.GetEnumerator()) {
> Write-SpectreHost -Message "Testing $($test.Name)"
> if ($test.Name -eq 'hashtable') {
> Format-SpectreTable -Data $test.Value -Title 'My hashtable'
> continue
> }
> if ($Test.Name -eq 'PSCustomObject') {
> Format-SpectreTable -Data $test.Value -Title 'My PSCustomObject' -Property Name, Version, Description
> continue
> }
> Format-SpectreTable -Data $test.Value
> }
>
> —
> Reply to this email directly, view it on GitHub
> <#21 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ADEMYIQPWLWR4L47BMH36KLYNMX27AVCNFSM6AAAAABBKXRFVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBQGIZDEOBSGI>
> .
> You are receiving this because you commented.Message ID:
> ***@***.***>
>
|
I think i just copied values from wikipedia/ANSI_escape_code I'm sure we could tweak it a bit. those codes can be a bit spotty on the implementation side not all terminals use the same code for the same thing or implement all the codes. hmm, looking at the test a bit i think we need to figure out a different way to test decorations.
I think it would be better to just compare the ansi output if we could capture that and compare function Get-AnsiEscapeSequence {
param(
[Parameter(Mandatory, ValueFromPipeline)]
$String
)
process {
$decoded = $String.EnumerateRunes() | ForEach-Object {
if ($_.Value -le 0x1f) {
[Text.Rune]::new($_.Value + 0x2400)
} else {
$_
}
} | Join-String
[PSCustomObject]@{
Decoded = $decoded
Original = $String
}
}
}
"$($PSStyle.blink) testblink $($PSStyle.Reset)" | Get-AnsiEscapeSequence
it is quite alot at the moment though, but ill leave it in. |
|
i looked over the TestHelpers module and added a new function, Describe "Get-SpectreRenderable" {
It "returns the correct output for a given renderable" {
$renderable = [Spectre.Console.Text]::new("Hello, world!")
$output = Get-SpectreRenderable $renderable
$output | Should -Be "Hello, world!"
}
}
Describe "Get-SpectreRenderable" {
It "returns the correct output for a given renderable" {
$renderable = [Spectre.Console.Markup]::new("[Yellow]Hello[/], [Green]world![/]")
$output = Get-SpectreRenderable $renderable
$output | Should -Be "Hello, world!"
}
}
gives this, |
|
also this package exists Spectre.Console.Testing was looking at this as well that's how spectre tests, should be possible to use |
Now i remember why i thought about changing the name, it's from my first attempt to find "default" properties, see -> (ls -file)[0].PSStandardMembers.DefaultDisplayPropertySet^ is an optional setting, and not everything implements it. it's also possible to do something like this, $w = [PSCustomObject]@{
PSTypeName = 'custom.Sample'
Name = 'MyName'
Stuff = 'MyStuff'
123 = 'My123'
sample = 'sample'
test = 'test'
}
Update-TypeData -TypeName 'custom.Sample' -DefaultDisplayPropertySet 'name','stuff','123' -Force
# the custom typedata does not work for this (yet atleast)
$w | Format-SpectreTable
# as opposed to
$w | Format-TableI just seem to have forgot to change it, might be more correct to swap to I also created a little debug function, not sure if we should include something like this in the module. basically it lets you "mock" at console so we can capture output. with should be able to do something like this, 1,2,3 | fst -Title 'int' -Border Markdown | ConvertFrom-Markdown | fl *but unfortunately theres some bugs with I'm not so well versed in pester tests in github runners, but it appears my example test is failing, I'm unable to replicate this locally though, using a fresh session and -noprofile. PS> Invoke-Pester -Path .\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\formatting\new_Format-SpectreTable.tests.ps1 -Output Detailed
Pester v5.5.0
Starting discovery in 1 files.
Discovery found 1 tests in 102ms.
Running tests.
Running tests from 'D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\formatting\new_Format-SpectreTable.tests.ps1'
Describing Format-SpectreTable
[+] Should create a table when default display members for a command are required 47ms (34ms|14ms)
Tests completed in 197ms
Tests Passed: 1, Failed: 0, Skipped: 0 NotRun: 0
Even with diagnostic all looks good.
I symlink ~/PwshSpectreConsole/PwshSpectreConsole/packages to a libfolder outside the repo $path = Get-Childitem -Path $env:USERPROFILE\Documents\PowerShell\Modules\PwshSpectreConsole\ | Select-Object -Last 1
New-Item -ItemType SymbolicLink -Path packages -Target "$path\packages"I havn't looked that closely at the build script, do i still need to run it or is it just the libs?
btw, Write-Debug and Write-Verbose works pretty much the same way, if your using an advanced function ( I'm gonna take another look at the PSStyle and the C# dict tonight. end wall of text 😆 |
It just downloads the libs atm |
…mething is wonky. need to debug
|
ok i figured out whats wrong with a few of the tests.. added a small function Seems the github runners are only running EightBit. i tried setting $env:COLORTERM = 'truecolor' on a local linux machine but it doesn't seem to affect this, this means that spectre console will automatically downgrade any 24bit colors to a color that is supported, which is pretty cool but makes this test a bit hard. but the tests work locally, and did find a big bug in my converter code that im gonna try and fix. Enrichers : GitHub
ColorSystem : EightBit
Unicode : True
Ansi : True
Links : False
Legacy : False
Interactive : False
Terminal : False
Writer : System.IO.TextWriter+SyncTextWriter
Width : 80
Height : 80
Encoding : Unicode (UTF-8)
PSStyle : Host
ConsoleOutputEncoding : System.Text.ConsoleEncoding
ConsoleInputEncoding : System.Text.ConsoleEncoding |
|
we should probably disable a few of the tests for CI as it requires truecolor? but all tests work locally for me atleast Tests completed in 13.12s
Tests Passed: 72, Failed: 0, Skipped: 0 NotRun: 0
Processing code coverage result.
Covered 43,46% / 75%. 1 560 analyzed Commands in 57 Files.and all the examples from #20 work. |
|
now the function maps console colors correctly, but Spectre dont use the same escape sequence so its gonnna be a bit harder to verify in tests. Test-Colortable it'll adapt to the colors you have chosen in your color profile in your terminal. |
trackd
changed the title
|
@ShaunLawrie this is ready for prerelease. I ran through CI tests failed but that's because they can only run 8bit so 24bit rgb is downgraded. TestlogStarting pwsh -noprofile and Running Pester tests...Starting discovery in 21 files. Completing a single stage process ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% job 1 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% [+] D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\progress\Invoke-SpectreCommandWithProgress.tests.ps1 2.3s (2.14s|41ms) |
|
The publish won’t happen if the CI test fail, you could add a tag and
pester config to not run them in CI
…On Fri, 19 Jan 2024 at 7:14 AM, trackd ***@***.***> wrote:
@ShaunLawrie <https://github.com/ShaunLawrie> this is ready for
prerelease.
I ran through Start-SpectreDemo and the Pester tests, all looks good.
CI tests failed but that's because they can only run 8bit so 24bit rgb is
downgraded.
Testlog Starting pwsh -noprofile and Running Pester tests...
Starting discovery in 21 files.
Discovery found 72 tests in 2.84s.
Starting code coverage.
Running tests.
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\formatting\ConvertTo-SpectreDecoration.tests.ps1
1.94s (1.33s|296ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\formatting\Format-SpectreBarChart.tests.ps1
653ms (443ms|86ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\formatting\Format-SpectreBreakdownChart.tests.ps1
369ms (167ms|63ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\formatting\Format-SpectreJson.tests.ps1
302ms (127ms|46ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\formatting\Format-SpectrePanel.tests.ps1
215ms (57ms|40ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\formatting\Format-SpectreTable.tests.ps1
504ms (307ms|69ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\formatting\Format-SpectreTree.tests.ps1
234ms (76ms|41ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\formatting\new_Format-SpectreTable.tests.ps1
249ms (86ms|46ms)
Completing a single stage process ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
100%
job 1 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100%
job 2 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100%
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\progress\Invoke-SpectreCommandWithProgress.tests.ps1
2.3s (2.14s|41ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\progress\Invoke-SpectreCommandWithStatus.tests.ps1
1.25s (1.09s|41ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\prompts\Read-SpectreConfirm.tests.ps1
284ms (116ms|40ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\prompts\Read-SpectreMultiSelection.tests.ps1
286ms (112ms|48ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\prompts\Read-SpectreMultiSelectionGrouped.tests.ps1
364ms (204ms|42ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\prompts\Read-SpectrePause.tests.ps1
265ms (106ms|44ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\prompts\Read-SpectreSelection.tests.ps1
216ms (54ms|42ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\prompts\Read-SpectreText.tests.ps1
214ms (51ms|38ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\writing\Get-SpectreEscapedText.tests.ps1
171ms (8ms|42ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\writing\Write-SpectreCalender.tests.ps1
344ms (148ms|45ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\writing\Write-SpectreFigletText.tests.ps1
212ms (43ms|40ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\writing\Write-SpectreHost.tests.ps1
224ms (57ms|43ms)
[+]
D:\Code\PwshSpectreConsole\PwshSpectreConsole.Tests\writing\Write-SpectreRule.tests.ps1
175ms (18ms|40ms)
Tests completed in 10.83s
Tests Passed: 72, Failed: 0, Skipped: 0 NotRun: 0
Processing code coverage result.
Covered 43,21% / 75%. 1 560 analyzed Commands in 57 Files.
—
Reply to this email directly, view it on GitHub
<#21 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADEMYIRCW23TTO7RZQH2P4TYPG3F3AVCNFSM6AAAAABBKXRFVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJZGQZDEMJZGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I've added a filter called 'ExcludeCI'to the 24bit tests. also modified the detailsThe `Format-SpectreTable` property test fails because of console width in CI is 80. Seems the width is large enough for normal `Get-ChildItem` but `Format-SpectreTable` is wider so it truncates LastWriteTime I'll look into changing that test to something with less columns that should be unaffected with 80 width.
With normal console width the test work fine in Linux as well. |
I think maybe i was a bit unclear earlier. All CI tests pass now So it should be able to merge now. (sorry if you knew and just havnt had time, its no rush from me) just thought that maybe my response might look weird on email as I had an expanding section. |
Summary
Write-SpectreCalendarFormat-SpectreTableAdd-TableColumnNew-TableCellNew-TableRowTest-IsScalarConvertFrom-ConsoleColorConvertTo-SpectreDecoration, better cleaning of strings.Get-DefaultDisplayMembersGet-SpectreRenderableGet-AnsiEscapeSequenceGet-PSStyleRandomGet-SpectreProfileGet-SpectreColorSample