Add the converter for text tables to the repository #29

JamesWTruher · 2023-02-17T22:26:15Z

PR Summary

This adds the code and tests for converting text tables to json or psobjects
Also fixed a few failing tests for the Compare-Text cmdlet

PR Context

Fix some tests in Compare-Text.

Also create xml result file.

Small changes in text parser.

build.ps1

src/Microsoft.PowerShell.TextUtility.psd1

SteveL-MSFT · 2023-02-21T22:44:03Z

src/Microsoft.PowerShell.TextUtility.psd1

    )
    PrivateData = @{
        PSData = @{
            LicenseUri = 'https://github.com/PowerShell/TextUtility/blob/main/LICENSE'
            ProjectUri = 'https://github.com/PowerShell/TextUtility'
            ReleaseNotes = 'Initial release'
-            Prerelease = 'Preview1'
+            Prerelease = 'Preview2'


I wonder if we should just leave this as Preview since we're updating the version anyways

sure - I'll do that

Can we please remove the prerelease label? This module hasn't reached 1.0 to indicate stable yet. Not having any stable versions of a package is a hassle to install since -Prerelease has to be used. The main benefit to having prerelease in my opinion is when a stable version already exists.

SteveL-MSFT · 2023-02-21T22:49:06Z

src/code/TextTableParser.cs

+            }
+        }
+
+        /*


If you're keeping this commented out code, should have a comment explaining why otherwise remove it

now that it's in a commit, I don't need it any more

SteveL-MSFT · 2023-02-21T22:49:46Z

src/code/TextTableParser.cs

+        public SwitchParameter NoHeader { get; set; }
+
+        [Parameter()]
+        public SwitchParameter AsPsObject { get; set; }


I think it makes more sense to output PSObject by default and have AsJson to opt-in

SteveL-MSFT · 2023-02-21T22:50:38Z

src/code/TextTableParser.cs

+        public int MaximumWidth { get; set; } = 200;
+
+        [Parameter()]
+        public int WindowSize { get; set; } = 0; // 0 is all


WindowSize is the number of rows to analyze? If so, maybe call it AnalyzeRowCount?

I used WindowSize to be closer to Compare-Object SyncWindow but I like AnalyzeRowCount best, thanks

SteveL-MSFT · 2023-02-21T22:52:27Z

src/code/TextTableParser.cs

+            {
+                if (data[j] is string)
+                {
+                    // TODO: json encode the string


I don't understand this TODO, isn't line 315 encoding as json?

yah - I missed cleaning this up

SteveL-MSFT · 2023-02-21T22:53:26Z

src/code/TextTableParser.cs

+            return ColumnInfoList;
+        }
+
+        // TODO: still not right


Can you specify what isn't right?

it's mostly right now - another missed cleanup

Co-authored-by: Steve Lee <slee@microsoft.com>

`Array.Fill` not available, so we will do it manually.

Update build to default to debug configuration.

Also take advantage of Pester's ability to compare arrays.

…istent.

Add an additional test for OutputRendering = 'PlainText'

Remove a number of misleading comments. Change to be PSObject output by default (now we have an AsJson parameter). Change WindowSize parameter to AnalyzeRowCount. Fix test to not clone the convertArgs hashtable before modifying. Remove extraneous using statements. Build for netstandard2.0 rather than 2.1.

SteveL-MSFT · 2023-03-16T19:53:51Z

.gitignore

@@ -4,6 +4,7 @@ obj/
 out/
 project.lock.json
 .DS_Store
+Microsoft.PowerShell.TextUtility.xml


is this the xml help generated during build? we should just exclude the build folder than this specific file

no, it's generated by Invoke-Pester when -test is used with build

SteveL-MSFT · 2023-03-16T20:04:06Z

yaml/template/runtest.yml

@@ -4,7 +4,7 @@ parameters:
  jobDisplayName: 'Run test'

 jobs:
-  - job: '${{ parameters.jobName }}_netstandard20'
+  - job: '${{ parameters.jobName }}_netstandard21'


didn't you fix this to be netstandard20 compatible?

ThomasNieto · 2023-03-16T20:19:53Z

src/code/TextTableParser.cs

+        }
+    }
+
+    [Cmdlet("Convert","TextTable")]


Suggested change

[Cmdlet("Convert","TextTable")]

[Cmdlet("ConvertFrom","TextTable")]

During the community call it was suggested to change verb to ConvertFrom.

ThomasNieto · 2023-03-16T20:20:27Z

src/code/TextTableParser.cs

+        public int HeaderLine { get; set; } = 0; // Assume the first line is the header
+
+        [Parameter()]
+        public string[] Typename { get; set; }


Suggested change

public string[] Typename { get; set; }

public string[] TypeName { get; set; }

ThomasNieto · 2023-03-16T20:25:28Z

src/code/TextTableParser.cs

+        public int[] ColumnOffset { get; set; }
+
+        [Parameter()]
+        public SwitchParameter Convert { get; set; }


I don't have a good suggestion on this but the parameter name is ambiguous especially since the verb is Convert maybe something like ConvertPropertyValue or ConvertPropertyType or CastPropertyValue or CastPropertyType.

Something to indicate from the parameter name what is being changed in the output.

I liked ConvertPropertyValue the best, so that's what I went with.

ThomasNieto · 2023-03-17T20:16:42Z

src/code/Microsoft.PowerShell.TextUtility.csproj

@@ -17,6 +17,9 @@
        <PackageReference Include="PowerShellStandard.Library" Version="5.1.0-*">
            <PrivateAssets>all</PrivateAssets>
        </PackageReference>
+        <PackageReference Include="System.Text.Json" Version="6.0.6" >


Since this module supports PS 5.1 ensure that System.Text.Json dll gets included in the module.

ThomasNieto · 2023-03-29T16:54:33Z

test/TextTableParser.Tests.ps1

+            FileName = "tasklist.01.txt"
+            convertArgs = @{}
+            Rows = 46
+            Results = @{ ROW = 0;  "Image_Name" = "========================="; "PID" = "========"; "Session_Name" = "================"; "Session#" = "==========="; "Mem_Usage" = "============" },


The conversion process should check for header symbols and remove them from parsed output. For example, if all the properties of a row contain the same character (=, -, _, etc) it should be removed.

it's the etc I'm worried about. Right now, I'm not spending any time determining the content of the data, just the column delineation. There might be a performance penalty if I have to carefully inspect the data. Also, I need those separators as part of the determination of what a column, so I can't preprocess it away.

I've opened #34 to track

JamesWTruher · 2023-06-21T06:35:59Z

I made too many changes in the process of getting to a successful release build, so I'm closing this and opening a new PR. @ThomasNieto I've incorporated nearly all your requests into the new branch. I still need to address #29 (comment) , though, I'm concerned about over elimination (don't want to throw away input which might turn into valid data). I may leave that to a future preview.

Change name to ConvertFrom-TextTable. Remove Preview tag (since we're still a 0.x version). Change Convert parameter To ConvertPropertyValue, Typename to TypeName.

* Initial commit of text table converter with tests. * Address feedback in PR #29 (now closed). Change name to ConvertFrom-TextTable. Remove Preview tag (since we're still a 0.x version). Change Convert parameter To ConvertPropertyValue, Typename to TypeName.

JamesWTruher added 9 commits February 17, 2023 13:25

Incorporate ConvertTextTable code into module.

21e9af4

Fix some tests in Compare-Text.

Remove commented out code.

3669a38

Update yml files for new build script.

76d8eb5

Install Pester 4.10.1 if it is not available.

6582c30

Also create xml result file.

Get some information about the test environment.

0578174

Small changes in text parser.

Add some debugging output in case the test fails.

36bf7a4

Change the expected text for compare-text.

2f49085

Small change to expected string for side-by-side.

7d85284

Another small change to side-by-side expected value.

e5dd55e

SteveL-MSFT requested changes Feb 21, 2023

View reviewed changes

JamesWTruher and others added 9 commits February 21, 2023 15:06

Update src/Microsoft.PowerShell.TextUtility.psd1

36deecc

Co-authored-by: Steve Lee <slee@microsoft.com>

Change to use netstandard2.0

24c0ca9

`Array.Fill` not available, so we will do it manually.

Be sure to check the proper location for the assembly for publishing.

e4468b4

Update build to default to debug configuration.

Change the tests slightly to hopefully handler different versions of PS.

44afeb6

Also take advantage of Pester's ability to compare arrays.

Mark side-by-side test to be pending because ps versions are not cons…

1092661

…istent.

Mark the side-by-side test as pending on the filed issue.

3b140f8

Add an additional test for OutputRendering = 'PlainText'

reduce the text lines in df test.

01fdf7f

Change parameter ColumnWidth to ColumnOffset since that's what it is.

cdb8fb3

SteveL-MSFT requested changes Mar 16, 2023

View reviewed changes

ThomasNieto reviewed Mar 16, 2023

View reviewed changes

ThomasNieto reviewed Mar 17, 2023

View reviewed changes

ThomasNieto reviewed Mar 29, 2023

View reviewed changes

JamesWTruher closed this Jun 21, 2023

JamesWTruher mentioned this pull request Jun 21, 2023

Add ConvertFrom-TextTable cmdlet #33

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the converter for text tables to the repository #29

Add the converter for text tables to the repository #29

JamesWTruher commented Feb 17, 2023

SteveL-MSFT Feb 21, 2023

JamesWTruher Feb 24, 2023

ThomasNieto Mar 16, 2023

SteveL-MSFT Feb 21, 2023

JamesWTruher Feb 24, 2023

SteveL-MSFT Feb 21, 2023

SteveL-MSFT Feb 21, 2023

JamesWTruher Feb 24, 2023

SteveL-MSFT Feb 21, 2023

JamesWTruher Feb 24, 2023

SteveL-MSFT Feb 21, 2023

JamesWTruher Feb 24, 2023

SteveL-MSFT Mar 16, 2023

JamesWTruher Jun 21, 2023

SteveL-MSFT Mar 16, 2023

ThomasNieto Mar 16, 2023

ThomasNieto Mar 16, 2023

ThomasNieto Mar 16, 2023

JamesWTruher Jun 28, 2023

ThomasNieto Mar 17, 2023

ThomasNieto Mar 29, 2023

JamesWTruher Jun 21, 2023

JamesWTruher Jun 28, 2023

JamesWTruher commented Jun 21, 2023

+                          }
+                      }
+                      /*

	[Cmdlet("Convert","TextTable")]
	[Cmdlet("ConvertFrom","TextTable")]

	public string[] Typename { get; set; }
	public string[] TypeName { get; set; }

Add the converter for text tables to the repository #29

Add the converter for text tables to the repository #29

Conversation

JamesWTruher commented Feb 17, 2023

PR Summary

PR Context

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JamesWTruher commented Jun 21, 2023