Add ConvertTo-Clixml and ConvertFrom-Clixml cmdlet by Shriram0908 · Pull Request #12845 · PowerShell/PowerShell

Shri Ram K S (Shriram0908) · 2020-05-30T13:41:01Z

PR Summary

ConvertTo-Clixml and ConvertFrom-CliXml to work with clixml objects. Fixes #3898

PR Context

The new cmdlet can be used to work with clixml without needed to save it file.
The cmdlets behave similar to ConvertTo-Csv and ConvertFrom-Csv

PR Checklist

Shri Ram K S (Shriram0908) · 2020-06-02T12:46:42Z

Hi Ilya (@iSazonov), Could you review the PR and provide your feedback.

Ilya (iSazonov) · 2020-06-03T17:31:31Z

Duplicate code?

Ilya (@iSazonov) , I tired to use the same function but I was not sure how to do it so I have created a overloaded the method. This is to make sure that Import-Clixml and ConvertFrom-Clixml use the same helper function.

It is static. So it could be moved to a temp class or a base class.

class ImportXmlHelper does't not derive from any other class.
I tried to convert sting into memory stream so that we dont need this function itself. Let me know if we can implementation this

internal ImportXmlHelper(PSCmdlet cmdlet, string cliObject) { _cmdlet = cmdlet; _memoryStream = new MemoryStream(); StreamWriter memeoryWriter = new StreamWriter(_memoryStream); memeoryWriter.Write(cliObject); memeoryWriter.Flush(); _memoryStream.Position = 0; CreateStream(); }

It depends on whether we want to process "#< CLIXML".

Comands like below produce output with "#< CLIXML"

Start-Process -FilePath $PSHOME\pwsh -RedirectStandardOutput "temp.xml" -Args "-noprofile -nologo -outputformat xml -command get-command import-clixml" -Wait

I have not sure where else this behavior exists.

Shri Ram K S (Shriram0908) · 2020-06-04T07:25:28Z

Ilya (@iSazonov) I have made the requested changes. Please review and provide your feedback.

Rain Sallow (/u/ta11ow) (vexx32) · 2020-06-04T15:06:27Z

I have one main question regarding the code here (there are minor things, but I think the main thing should be addressed first) -- if I'm not mistaken, a fair amount of parameters and the serialization methods are pretty similar to the existing Export/Import-Csv cmdlets. Have we looked at whether some of the existing code (and/or the new code) can be moved into a base class that both cmdlets can implement to reduce code duplication here? Or are we expecting to do that after this PR?

Shri Ram K S (Shriram0908) · 2020-06-04T15:47:28Z

Rain Sallow (/u/ta11ow) (@vexx32), I tried not make too many changes to existing code. I made modifications only to ImportXmlHelper class (which is used by Import-Clixml) to work with ConvertFrom-Clixml.

I am following this approach to reduce the PR size. we can have two separate PR or one single PR. I am okay with it either way.

Rain Sallow (/u/ta11ow) (vexx32)

Looks good to me, apart from a couple smaller comments. 🙂

Rain Sallow (/u/ta11ow) (vexx32) · 2020-06-04T16:08:22Z

It's unusual for a Dispose() method to throw, but iirc it can happen; I'd prefer to see this with a try/finally so that _disposed is always set correctly.

It seems _importXmlHelper.Dispose() can not throw.

Rain Sallow (/u/ta11ow) (vexx32) · 2020-06-04T16:14:56Z

Dongbo Wang (@daxian-dbw) seeing PSObject[] as a parameter type reminds of the weird issue we had with Write-Output where it was enumerating input arrays and wrapping every single item in PSObject during parameter binding. Is that desired here, or should we go with PSObject to avoid it like I think we did with Write-Output?

We use the same pattern in ConvertFrom-Csv.

Rain Sallow (/u/ta11ow) (@vexx32) I changed to PSObject and tried it code execution seems to follow the same path. No difference either way. Maybe using PSObject can improve readability.
I used PSObject[] seeing ConvertFrom-Csv.

Yeah Ilya (@iSazonov) I'm sure a lot of commands still do. I'm not sure if it's the right way to go though.

Shri Ram K S (@Shriram0908) if it works both ways... it's probably fine to leave as is. I would prefer to have someone on the PS team weigh in first though.

Rain Sallow (/u/ta11ow) (@vexx32) Sure okay. let me know anything else needs to be done.

Ilya (iSazonov) · 2020-06-08T07:17:29Z

Suggested change

_stringBuilder.AppendLine( InputObject[0].ToString() );

_stringBuilder.AppendLine(InputObject[0].ToString());

Ilya (iSazonov) · 2020-06-08T07:19:45Z

Suggested change

$content = Get-Command export* -Type Cmdlet | Select-Object -First 3 | ConvertTo-Clixml

$results = ConvertFrom-CliXml $content

$results.Count | Should -Be 3

$results[0].PSTypeNames[0] | Should -BeExactly "Deserialized.System.Management.Automation.CmdletInfo"

$content = Get-Command export* -Type Cmdlet | Select-Object -First 3 | ConvertTo-Clixml

$results = ConvertFrom-CliXml $content

$results.Count | Should -Be 3

$results[0].PSTypeNames[0] | Should -BeExactly "Deserialized.System.Management.Automation.CmdletInfo"

Ilya (iSazonov) · 2020-06-08T07:21:01Z

Suggested change

It "ConvertFrom-CliXml with Rehydration should work" {

$property1 = 256

$property2 = "abcdef"

$isHiddenTestType = [IsHiddenTestType]::New($property1,$property2)

$content = $isHiddenTestType | ConvertTo-Clixml

$results = ConvertFrom-CliXml $content

$results.Property1 | Should -Be $property1

$results.Property2 | Should -BeExactly $property2

It "ConvertFrom-CliXml with Rehydration should work" {

$property1 = 256

$property2 = "abcdef"

$isHiddenTestType = [IsHiddenTestType]::New($property1,$property2)

$content = $isHiddenTestType | ConvertTo-Clixml

$results = ConvertFrom-CliXml $content

$results.Property1 | Should -Be $property1

$results.Property2 | Should -BeExactly $property2

Ilya (iSazonov) · 2020-06-08T07:22:01Z

Suggested change

It "ConvertFrom-CliXml StopProcessing should succeed" {

$content = 1,2,3 | ConvertTo-Clixml

$ps = [PowerShell]::Create()

$ps.AddCommand("Get-Process")

$ps.AddCommand("ConvertFrom-CliXml")

$ps.AddParameter("InputObject", $content)

$ps.BeginInvoke()

$ps.Stop()

$ps.InvocationStateInfo.State | Should -BeExactly "Stopped"

It "ConvertFrom-CliXml StopProcessing should succeed" {

$content = 1,2,3 | ConvertTo-Clixml

$ps = [PowerShell]::Create()

$ps.AddCommand("Get-Process")

$ps.AddCommand("ConvertFrom-CliXml")

$ps.AddParameter("InputObject", $content)

$ps.BeginInvoke()

$ps.Stop()

$ps.InvocationStateInfo.State | Should -BeExactly "Stopped"

Ilya (iSazonov) · 2020-06-08T07:23:32Z

Suggested change

It "ConvertFrom-CliXml -First and -Skip work together for collections" {

$content = @{ a = 1; b = 2; c = 3; d = 4 } | ConvertTo-Clixml

# order not guaranteed, even with [ordered] so we have to be smart here and compare against the full result

$out1 = ConvertFrom-CliXml -InputObject $content # this results in a hashtable

$out2 = ConvertFrom-CliXml -InputObject $content -First 2 -Skip 1 # this results in a dictionary entry

$out2.Count | Should -Be 2

It "ConvertFrom-CliXml -First and -Skip work together for collections" {

$content = @{ a = 1; b = 2; c = 3; d = 4 } | ConvertTo-Clixml

# order not guaranteed, even with [ordered] so we have to be smart here and compare against the full result

$out1 = ConvertFrom-CliXml -InputObject $content # this results in a hashtable

$out2 = ConvertFrom-CliXml -InputObject $content -First 2 -Skip 1 # this results in a dictionary entry

$out2.Count | Should -Be 2

Ilya (iSazonov) · 2020-06-08T07:23:53Z

Indentation.

Ilya (iSazonov) · 2020-06-08T07:24:30Z

Indentation.

Ilya (iSazonov) · 2020-06-08T07:24:39Z

Indentation.

Ilya (@iSazonov) I just realized that the indentation issues is due to mix of tab and space. Which one should I use space or tab or is it fine as long as I use it consistently ?

Spaces is generally the norm in the repo. The only exceptions I usually see are really old files.

Shri Ram K S (Shriram0908) · 2020-06-09T16:11:35Z

Ilya (@iSazonov) Rain Sallow (/u/ta11ow) (@vexx32)
What is the next step. Shall I file a new document request.

Rain Sallow (/u/ta11ow) (vexx32) · 2020-06-09T16:14:05Z

Yeah, please file a documentation issue in the docs repo and put the link in the PR description. 😊

Shri Ram K S (Shriram0908) · 2020-06-26T14:20:32Z

Ilya (@iSazonov) Rain Sallow (/u/ta11ow) (@vexx32) , Please let me know what to do next.

Ilya (iSazonov) · 2020-06-27T12:32:54Z

Waiting MSFT team approve.

Deleted user (ghost) · 2020-07-04T14:00:28Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

bravo-kernel · 2020-09-14T10:11:48Z

Shri Ram K S (@Shriram0908) thank you for this one, hope it gets merged ❤️

Shri Ram K S (Shriram0908) · 2021-01-20T16:55:57Z

Ilya (@iSazonov) Rain Sallow (/u/ta11ow) (@vexx32) Travis Plunk (@TravisEz13) Steve Lee (@SteveL-MSFT)

Its been quite some time. Please let me know if I could help.

Ilya (iSazonov) · 2021-01-20T17:56:45Z

I can not merge without additional reviews.

Deleted user (ghost) · 2021-08-24T23:00:36Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment.

Shri Ram K S (Shriram0908) · 2021-11-17T13:47:23Z

How to re-open this PR ?

Shri Ram K S (Shriram0908) · 2021-11-17T17:29:35Z

Ilya (@iSazonov) Thank you for opening the PR. Seems like Link in internals.md and ADOPTERS.md are failing. Let me know if there is something that I can do.

Ilya (iSazonov) · 2021-11-17T17:36:16Z

Shri Ram K S (@Shriram0908) I think you need to rebase.

Shri Ram K S (Shriram0908) · 2021-11-17T18:20:04Z

Ilya (@iSazonov) Once I rebased the pushed to repo, It worked. Thanks.

Deleted user (ghost) · 2021-11-25T02:00:46Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

Test for ConvertTo-Clixml and ConvertFrom-CliXml

use memory stream instead of string builder code cleanup

convert tab to space

pull-request-quantifier-deprecated · 2022-07-13T15:46:34Z

This PR has 401 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Quantification details

Label      : Extra Large
Size       : +316 -85
Percentile : 80.03%

Total files changed: 6

Change summary by file extension:
.cs : +111 -2
.psd1 : +3 -2
.ps1 : +202 -81

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

Fast and predictable releases to production:
- Optimal size changes are more likely to be reviewed faster with fewer
  iterations.
- Similarity in low PR complexity drives similar review times.
Review quality is likely higher as complexity is lower:
- Bugs are more likely to be detected.
- Code inconsistencies are more likely to be detetcted.
Knowledge sharing is improved within the participants:
- Small portions can be assimilated better.
Better engineering practices are exercised:
- Solving big problems by dividing them in well contained, smaller problems.
- Exercising separation of concerns within the code changes.

What can I do to optimize my changes

Use the PullRequestQuantifier to quantify your PR accurately
- Create a context profile for your repo using the context generator
- Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
- Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
- Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
Change your engineering behaviors
- For PRs that fall outside of the desired spectrum, review the details and check if:
  - Your PR could be split in smaller, self-contained PRs instead
  - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

One line was added: +1 -0
One line was deleted: +0 -1
One line was modified: +1 -1 (git diff doesn't know about modified, it will
interpret that line like one addition plus one deletion)
Change percentiles: Change characteristics (addition, deletion, modification)
of this PR in relation to all other PRs within the repository.

Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

Steve Lee (SteveL-MSFT) · 2023-03-13T16:10:16Z

It appears that Travis Plunk (@TravisEz13)'s feedback hasn't been addressed yet

Travis Plunk (TravisEz13)

Please address the comments from #12845 (comment).

Deleted user (ghost) · 2023-03-28T17:00:55Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment.

ghost assigned Ilya (iSazonov) May 30, 2020

Ilya (iSazonov) reviewed Jun 3, 2020

View reviewed changes

Shri Ram K S (Shriram0908) requested a review from Ilya (iSazonov) June 4, 2020 07:24

Ilya (iSazonov) reviewed Jun 4, 2020

View reviewed changes

Shri Ram K S (Shriram0908) requested a review from Ilya (iSazonov) June 4, 2020 14:25

Ilya (iSazonov) requested review from Steve Lee (SteveL-MSFT), Travis Plunk (TravisEz13) and Rain Sallow (/u/ta11ow) (vexx32) June 4, 2020 14:41

Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Jun 4, 2020

Rain Sallow (/u/ta11ow) (vexx32) reviewed Jun 4, 2020

View reviewed changes

Rain Sallow (/u/ta11ow) (vexx32) approved these changes Jun 5, 2020

View reviewed changes

Ilya (iSazonov) reviewed Jun 8, 2020

View reviewed changes

Shri Ram K S (Shriram0908) requested a review from Ilya (iSazonov) June 8, 2020 11:00

Ilya (iSazonov) approved these changes Jun 8, 2020

View reviewed changes

Shri Ram K S (Shriram0908) mentioned this pull request Jun 9, 2020

Add document for ConvertTo-Clixml and ConvertFrom-CliXml MicrosoftDocs/PowerShell-Docs#6093

Closed

ghost added the Review - Needed The PR is being reviewed label Jul 4, 2020

ghost removed the Review - Needed The PR is being reviewed label Jan 20, 2021

ghost added the Review - Needed The PR is being reviewed label Jan 28, 2021

ghost closed this Sep 8, 2021

ghost removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept Stale labels Nov 17, 2021

Ilya (iSazonov) reopened this Nov 17, 2021

pull-request-quantifier-deprecated Bot added the Extra Large label Nov 17, 2021

Shri Ram K S (Shriram0908) requested a review from Paul Higinbotham (PaulHigin) as a code owner November 17, 2021 17:44

ghost added the Review - Needed The PR is being reviewed label Nov 25, 2021

Michael Klement (mklement0) mentioned this pull request Jan 7, 2022

Allow ConvertTo-Xml to emit CLIXML (the same XML format as Export-Clixml) #8311

Closed

Shri Ram K S (Shriram0908) added 7 commits July 13, 2022 21:13

Add ConvertTo-Clixml and ConvertFrom-Clixml cmdlet

8cb9512

Test for ConvertTo-Clixml and ConvertFrom-CliXml

Add new cmdlet in DefaultCommands.Tests

76b798b

Code cleanup

7d36890

Remove second ImportXmlHelper method

5f28c26

use memory stream instead of string builder code cleanup

Fix comments and code cleanup

156d9ef

Fix code formating

f0195f8

convert tab to space

Corrected class name to ConvertToClixmlCommand

ec24325

ghost removed the Review - Needed The PR is being reviewed label Mar 13, 2023

Travis Plunk (TravisEz13) requested changes Mar 13, 2023

View reviewed changes

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Mar 13, 2023

ghost added the Stale label Mar 28, 2023

ghost closed this Apr 7, 2023

Armaan Mcleod (ArmaanMcleod) mentioned this pull request Jan 6, 2024

Add ConvertTo-CliXml and ConvertFrom-CliXml cmdlets #21011

Closed

22 tasks

This pull request was closed.

	_stringBuilder.AppendLine( InputObject[0].ToString() );
	_stringBuilder.AppendLine(InputObject[0].ToString());

-        $content = Get-Command export* -Type Cmdlet | Select-Object -First 3 | ConvertTo-Clixml
-		$results = ConvertFrom-CliXml $content
-		$results.Count | Should -Be 3
-        $results[0].PSTypeNames[0] | Should -BeExactly "Deserialized.System.Management.Automation.CmdletInfo"
+        $content = Get-Command export* -Type Cmdlet | Select-Object -First 3 | ConvertTo-Clixml
+        $results = ConvertFrom-CliXml $content
+        $results.Count | Should -Be 3
+        $results[0].PSTypeNames[0] | Should -BeExactly "Deserialized.System.Management.Automation.CmdletInfo"

-    It "ConvertFrom-CliXml with Rehydration should work" {
-		$property1 = 256
-		$property2 = "abcdef"
-		$isHiddenTestType = [IsHiddenTestType]::New($property1,$property2)
-		$content = $isHiddenTestType | ConvertTo-Clixml
-		$results = ConvertFrom-CliXml $content
-		$results.Property1 | Should -Be $property1
-		$results.Property2 | Should -BeExactly $property2
+    It "ConvertFrom-CliXml with Rehydration should work" {
+        $property1 = 256
+        $property2 = "abcdef"
+        $isHiddenTestType = [IsHiddenTestType]::New($property1,$property2)
+        $content = $isHiddenTestType | ConvertTo-Clixml
+        $results = ConvertFrom-CliXml $content
+        $results.Property1 | Should -Be $property1
+        $results.Property2 | Should -BeExactly $property2

-    It "ConvertFrom-CliXml StopProcessing should succeed" {
-		$content = 1,2,3 | ConvertTo-Clixml
-		$ps = [PowerShell]::Create()
-		$ps.AddCommand("Get-Process")
-		$ps.AddCommand("ConvertFrom-CliXml")
-		$ps.AddParameter("InputObject", $content)
-		$ps.BeginInvoke()
-		$ps.Stop()
-		$ps.InvocationStateInfo.State | Should -BeExactly "Stopped"
+    It "ConvertFrom-CliXml StopProcessing should succeed" {
+        $content = 1,2,3 | ConvertTo-Clixml
+        $ps = [PowerShell]::Create()
+        $ps.AddCommand("Get-Process")
+        $ps.AddCommand("ConvertFrom-CliXml")
+        $ps.AddParameter("InputObject", $content)
+        $ps.BeginInvoke()
+        $ps.Stop()
+        $ps.InvocationStateInfo.State | Should -BeExactly "Stopped"

-	It "ConvertFrom-CliXml -First and -Skip work together for collections" {
-		$content = @{ a = 1; b = 2; c = 3; d = 4 } | ConvertTo-Clixml
-		# order not guaranteed, even with [ordered] so we have to be smart here and compare against the full result
-		$out1 = ConvertFrom-CliXml -InputObject $content	# this results in a hashtable
-		$out2 = ConvertFrom-CliXml -InputObject $content -First 2 -Skip 1	# this results in a dictionary entry
-		$out2.Count | Should -Be 2
+	It "ConvertFrom-CliXml -First and -Skip work together for collections" {
+            $content = @{ a = 1; b = 2; c = 3; d = 4 } | ConvertTo-Clixml
+            # order not guaranteed, even with [ordered] so we have to be smart here and compare against the full result
+            $out1 = ConvertFrom-CliXml -InputObject $content	# this results in a hashtable
+            $out2 = ConvertFrom-CliXml -InputObject $content -First 2 -Skip 1	# this results in a dictionary entry
+            $out2.Count | Should -Be 2

Uh oh!

Conversation

Shri Ram K S (Shriram0908) commented May 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

Shri Ram K S (Shriram0908) commented Jun 2, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Shri Ram K S (Shriram0908) commented Jun 4, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Jun 4, 2020

Uh oh!

Shri Ram K S (Shriram0908) commented Jun 4, 2020

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shri Ram K S (Shriram0908) commented May 30, 2020 •

edited

Loading