Skip to content

Don't add trailing spaces to last column when using Format-Table #5568

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.

Sign up for GitHub

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

Merged
adityapatwardhan merged 4 commits into from
Feb 22, 2018
Merged

Don't add trailing spaces to last column when using Format-Table #5568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7fdec21
don't add trailing space to last column in table
SteveL-MSFT Nov 28, 2017
5a1cd96
fix out-file tests to new behavior of no padding trailing spaces
SteveL-MSFT Nov 29, 2017
06ba885
address PR feedback
SteveL-MSFT Feb 7, 2018
4b69e9a
address Jim's feedback
SteveL-MSFT Feb 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 31 additions & 11 deletions src/System.Management.Automation/FormatAndOutput/common/TableWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,18 @@ private string[] GenerateTableRow(string[] values, int[] alignment, DisplayCells
return null;

StringCollection[] scArray = new StringCollection[validColumnCount];
bool addPadding = true;
for (int k = 0; k < scArray.Length; k++)
{
// for the last column, don't pad it with trailing spaces
if (k == scArray.Length-1)
{
addPadding = false;
}
Copy link
Collaborator

@iSazonov iSazonov Nov 30, 2017

Choose a reason for hiding this comment

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

See my comment below.


// obtain a set of tokens for each field
scArray[k] = GenerateMultiLineRowField(values[validColumnArray[k]], validColumnArray[k],
alignment[validColumnArray[k]], ds);
alignment[validColumnArray[k]], ds, addPadding);

// NOTE: the following padding operations assume that we
// pad with a blank (or any character that ALWAYS maps to a single screen cell
Expand Down Expand Up @@ -283,7 +290,7 @@ private string[] GenerateTableRow(string[] values, int[] alignment, DisplayCells
}

// add padding for the columns that are shorter
for (int col = 0; col < scArray.Length; col++)
for (int col = 0; col < scArray.Length-1; col++)
{
int paddingBlanks = _si.columnInfo[validColumnArray[col]].width;
if (col > 0)
Expand Down Expand Up @@ -317,15 +324,18 @@ private string[] GenerateTableRow(string[] values, int[] alignment, DisplayCells
return rows;
}

private StringCollection GenerateMultiLineRowField(string val, int k, int alignment, DisplayCells dc)
private StringCollection GenerateMultiLineRowField(string val, int k, int alignment, DisplayCells dc, bool addPadding)
{
StringCollection sc = StringManipulationHelper.GenerateLines(dc, val,
_si.columnInfo[k].width, _si.columnInfo[k].width);
// if length is shorter, do some padding
for (int col = 0; col < sc.Count; col++)
if (addPadding)
{
if (dc.Length(sc[col]) < _si.columnInfo[k].width)
sc[col] = GenerateRowField(sc[col], _si.columnInfo[k].width, alignment, dc);
// if length is shorter, do some padding
for (int col = 0; col < sc.Count; col++)
{
if (dc.Length(sc[col]) < _si.columnInfo[k].width)
sc[col] = GenerateRowField(sc[col], _si.columnInfo[k].width, alignment, dc, addPadding);
}
}
return sc;
}
Expand All @@ -334,8 +344,15 @@ private string GenerateRow(string[] values, int[] alignment, DisplayCells dc)
{
StringBuilder sb = new StringBuilder();

bool addPadding = true;
for (int k = 0; k < _si.columnInfo.Length; k++)
{
// don't pad the last column
if (k == _si.columnInfo.Length -1)
{
addPadding = false;
}
Copy link
Collaborator

@iSazonov iSazonov Nov 30, 2017

Choose a reason for hiding this comment

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

Is performance important here? If yes we could move last column processing below the for cycle.

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Nov 30, 2017

Choose a reason for hiding this comment

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

I think for outputting table which is mostly an interactive experience, I don't think performance has a big implication here.

Copy link
Collaborator

@iSazonov iSazonov Dec 1, 2017

Choose a reason for hiding this comment

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

We have CPU spikes mainly because of the reoutput of symbols.

Closed.


if (_si.columnInfo[k].width <= 0)
{
// skip columns that are not at least a single character wide
Expand All @@ -357,20 +374,20 @@ private string GenerateRow(string[] values, int[] alignment, DisplayCells dc)
sb.Append(StringUtil.Padding(_startColumn));
}
}
sb.Append(GenerateRowField(values[k], _si.columnInfo[k].width, alignment[k], dc));
sb.Append(GenerateRowField(values[k], _si.columnInfo[k].width, alignment[k], dc, addPadding));
}
return sb.ToString();
}

private static string GenerateRowField(string val, int width, int alignment, DisplayCells dc)
private static string GenerateRowField(string val, int width, int alignment, DisplayCells dc, bool addPadding)
{
// make sure the string does not have any embedded <CR> in it
string s = StringManipulationHelper.TruncateAtNewLine(val) ?? "";

string currentValue = s;
int currentValueDisplayLength = dc.Length(currentValue);

if (currentValueDisplayLength < width)
if (addPadding && currentValueDisplayLength < width)
{
// the string is shorter than the width of the column
// need to pad with with blanks to reach the desired width
Expand Down Expand Up @@ -499,7 +516,10 @@ private static string GenerateRowField(string val, int width, int alignment, Dis
default:
{
// left align is the default
s += " ";
if (addPadding)
{
s += " ";
}
}
break;
}
Expand Down
59 changes: 24 additions & 35 deletions test/powershell/Modules/Microsoft.PowerShell.Utility/Format-Table.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,44 +137,23 @@ Describe "Format-Table DRT Unit Tests" -Tags "CI" {
$result | Should Match "Jim\s+5678\s+False"
}

It "Format-Table with No Objects for End-To-End should work"{
$p = @{}
$result = $p|Format-Table -Property "foo","bar"|Out-String
It "Format-Table with '<testName>' should return `$null" -TestCases @(
@{ testName = "empty array"; testObject = @{} },
@{ testName = "null" ; testObject = $null }
) {
param ($testObject)
$result = $testObject | Format-Table -Property "foo","bar" | Out-String
$result | Should BeNullOrEmpty
}

It "Format-Table with Null Objects for End-To-End should work"{
$p = $null
$result = $p|Format-Table -Property "foo","bar"|Out-String
$result | Should BeNullOrEmpty
}

#pending on issue#900
It "Format-Table with single line string for End-To-End should work" -pending{
$p = "single line string"
$result = $p|Format-Table -Property "foo","bar"|Out-String
$result | Should BeNullOrEmpty
}

#pending on issue#900
It "Format-Table with multiple line string for End-To-End should work" -pending{
$p = "Line1\nLine2"
$result = $p|Format-Table -Property "foo","bar"|Out-String
$result | Should BeNullOrEmpty
}

#pending on issue#900
It "Format-Table with string sequence for End-To-End should work" -pending{
$p = "Line1","Line2"
$result = $p|Format-Table -Property "foo","bar"|Out-String
$result | Should BeNullOrEmpty
}

#pending on issue#900
It "Format-Table with string sequence for End-To-End should work" -pending{
$p = "Line1","Line2"
$result = $p|Format-Table -Property "foo","bar"|Out-String
$result | Should BeNullOrEmpty
It "Format-Table with '<testName>' string and -Force should output table with requested properties" -TestCases @(
@{ testName = "single line"; testString = "single line string" },
@{ testName = "multi line" ; testString = "line1`nline2" },
@{ testName = "array" ; testString = "line1","line2" }
) {
param ($testString)
$result = $testString | Format-Table -Property "foo","bar" -Force | Out-String
$result.Replace(" ","").Replace([Environment]::NewLine,"") | Should BeExactly "foobar------"
}

It "Format-Table with complex object for End-To-End should work" {
Expand Down Expand Up @@ -211,4 +190,14 @@ Describe "Format-Table DRT Unit Tests" -Tags "CI" {
$result3 | Should Match "name\s+sub"
$result3 | Should Match "this is name\s+{x 0 y 0, x 1 y 1}"
}

It "Format-Table should not add trailing whitespace to the header" {
$out = "hello" | Format-Table -Property foo -Force | Out-String
$out.Replace([System.Environment]::NewLine, "") | Should BeExactly "foo---"
}

It "Format-Table should not add trailing whitespace to rows" {
$out = [pscustomobject]@{a=1;b=2} | Format-Table -HideTableHeaders | Out-String
$out.Replace([System.Environment]::NewLine, "") | Should BeExactly "1 2"
}
}
6 changes: 3 additions & 3 deletions test/powershell/Modules/Microsoft.PowerShell.Utility/Out-File.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ Describe "Out-File" -Tags "CI" {
$actual = Get-Content $testfile

$actual[0] | Should Be ""
$actual[1] | Should Be "text "
$actual[2] | Should Be "---- "
$actual[1] | Should Be "text"
$actual[2] | Should Be "----"
$actual[3] | Should Be "some te..."
}

Expand Down Expand Up @@ -129,7 +129,7 @@ Describe "Out-File" -Tags "CI" {
# reset to not read only so it can be deleted
Set-ItemProperty -Path $testfile -Name IsReadOnly -Value $false
}

It "Should be able to use the 'Path' alias for the 'FilePath' parameter" {
{ Out-File -Path $testfile } | Should Not Throw
}
Expand Down