New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix formatting of tables where headers span multiple rows #6504
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
<Description>PowerShell Core's System.Management.Automation project</Description> | ||
<NoWarn>$(NoWarn);CS1570;CS1734</NoWarn> | ||
<AssemblyName>System.Management.Automation</AssemblyName> | ||
<LangVersion>7.2</LangVersion> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need this? Currently we use default and seems the default is 7.2. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Span was not found without this and from what I could find, currently we need to opt into 7.2 features. Perhaps it will be default once dotnetcore2.1 GAs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @SteveL-MSFT I found in docs that we could use /cc @daxian-dbw There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened the tracking Issue #6547. |
||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
|
@@ -13,6 +14,7 @@ | |
<!-- the following package(s) are from https://github.com/dotnet/corefx --> | ||
<PackageReference Include="Microsoft.Win32.Registry.AccessControl" Version="4.4.0" /> | ||
<PackageReference Include="System.IO.FileSystem.AccessControl" Version="4.4.0" /> | ||
<PackageReference Include="System.Memory" Version="4.5.0-*"/> | ||
<PackageReference Include="System.Security.AccessControl" Version="4.4.1" /> | ||
<PackageReference Include="System.Security.Cryptography.Pkcs" Version="4.4.0" /> | ||
<PackageReference Include="System.Security.Permissions" Version="4.4.1" /> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -339,4 +339,141 @@ Left Center Right | |
$output = $ps.Invoke() | ||
$output.Replace("`n","").Replace("`r","") | Should -BeExactly $expected | ||
} | ||
|
||
It "Format-Table should correctly render headers that span multiple rows: <variation>" -TestCases @( | ||
@{ view = "Default"; widths = 4,7,4; variation = "4 row, 1 row, 2 row"; expectedTable = @" | ||
|
||
Long Header2 Head | ||
Long er3 | ||
Head | ||
er | ||
---- ------- ---- | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we should have a test for edge case - one column. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm also going to turn the ps1xml into a template since most of it is the same |
||
1 2 3 | ||
|
||
|
||
|
||
"@ }, | ||
@{ view = "Default"; widths = 4,4,7; variation = "4 row, 2 row, 1 row"; expectedTable = @" | ||
|
||
Long Head Header3 | ||
Long er2 | ||
Head | ||
er | ||
---- ---- ------- | ||
1 2 3 | ||
|
||
|
||
|
||
"@ }, | ||
@{ view = "Default"; widths = 14,7,3; variation = "1 row, 1 row, 3 row"; expectedTable = @" | ||
|
||
LongLongHeader Header2 Hea | ||
der | ||
3 | ||
-------------- ------- --- | ||
1 2 3 | ||
|
||
|
||
|
||
"@ }, | ||
@{ view = "One"; widths = 4,1,1; variation = "1 column"; expectedTable = @" | ||
|
||
Long | ||
Long | ||
Head | ||
er | ||
---- | ||
1 | ||
|
||
|
||
|
||
"@ } | ||
) { | ||
param($view, $widths, $expectedTable) | ||
$ps1xml = @" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: This is rather hard to read with large here strings in the middle of the code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In VSCode, the coloring makes it much easier to read. Collapsing the XML will make it harder to read the XML. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree too that it is more editors and formatting plugins issue and it strongly depends on personal preferences. We could be more happy with the support of YAML. |
||
<Configuration> | ||
<ViewDefinitions> | ||
<View> | ||
<Name>Default</Name> | ||
<ViewSelectedBy> | ||
<TypeName>Test.Format</TypeName> | ||
</ViewSelectedBy> | ||
<TableControl> | ||
<TableHeaders> | ||
<TableColumnHeader> | ||
<Label>LongLongHeader</Label> | ||
<Width>{0}</Width> | ||
</TableColumnHeader> | ||
<TableColumnHeader> | ||
<Label>Header2</Label> | ||
<Width>{1}</Width> | ||
</TableColumnHeader> | ||
<TableColumnHeader> | ||
<Label>Header3</Label> | ||
<Width>{2}</Width> | ||
</TableColumnHeader> | ||
</TableHeaders> | ||
<TableRowEntries> | ||
<TableRowEntry> | ||
<TableColumnItems> | ||
<TableColumnItem> | ||
<PropertyName>First</PropertyName> | ||
</TableColumnItem> | ||
<TableColumnItem> | ||
<PropertyName>Second</PropertyName> | ||
</TableColumnItem> | ||
<TableColumnItem> | ||
<PropertyName>Third</PropertyName> | ||
</TableColumnItem> | ||
</TableColumnItems> | ||
</TableRowEntry> | ||
</TableRowEntries> | ||
</TableControl> | ||
</View> | ||
<View> | ||
<Name>One</Name> | ||
<ViewSelectedBy> | ||
<TypeName>Test.Format</TypeName> | ||
</ViewSelectedBy> | ||
<TableControl> | ||
<TableHeaders> | ||
<TableColumnHeader> | ||
<Label>LongLongHeader</Label> | ||
<Width>{0}</Width> | ||
</TableColumnHeader> | ||
</TableHeaders> | ||
<TableRowEntries> | ||
<TableRowEntry> | ||
<TableColumnItems> | ||
<TableColumnItem> | ||
<PropertyName>First</PropertyName> | ||
</TableColumnItem> | ||
</TableColumnItems> | ||
</TableRowEntry> | ||
</TableRowEntries> | ||
</TableControl> | ||
</View> | ||
</ViewDefinitions> | ||
</Configuration> | ||
"@ | ||
$ps1xml = $ps1xml.Replace("{0}", $widths[0]).Replace("{1}", $widths[1]).Replace("{2}", $widths[2]) | ||
$ps1xmlPath = Join-Path -Path $TestDrive -ChildPath "test.format.ps1xml" | ||
Set-Content -Path $ps1xmlPath -Value $ps1xml | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't needed here, |
||
# run in own runspace so not affect global sessionstate | ||
$ps = [powershell]::Create() | ||
$ps.AddScript( { | ||
param($ps1xmlPath, $view) | ||
Update-FormatData -AppendPath $ps1xmlPath | ||
$a = [PSCustomObject]@{First=1;Second=2;Third=3} | ||
$a.PSObject.TypeNames.Insert(0,"Test.Format") | ||
$a | Format-Table -View $view | Out-String | ||
} ).AddArgument($ps1xmlPath).AddArgument($view) | Out-Null | ||
$output = $ps.Invoke() | ||
foreach ($e in $ps.Streams.Error) | ||
{ | ||
Write-Verbose $e.ToString() -Verbose | ||
} | ||
$ps.HadErrors | Should -BeFalse | ||
$output.Replace("`r","").Replace(" ",".").Replace("`n","``") | Should -BeExactly $expectedTable.Replace("`r","").Replace(" ",".").Replace("`n","``") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not for th PR - we have several similar repetitive cycles which we could replace one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please open an issue and mark as code cleanup. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done #6526.