Skip to content
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

ConvertTo-Json: terminate cut off branches with a ellipsis string rather than a full (.Net) type name #8381

Closed
iRon7 opened this issue Dec 2, 2018 · 9 comments
Labels
Breaking-Change breaking change that may affect users Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Enhancement the issue is more of a feature request than a bug Resolution-By Design The reported behavior is by design. WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module

Comments

@iRon7
Copy link

iRon7 commented Dec 2, 2018

This enhancement request is related to:
Make ConvertTo-Json detect circular references
In ConvertTo-Json the max allowed depth is 100. However, we should add code to dynamically check if we are running out of stack, and serialize objects with bigger depths

Apparently almost every programmer goes through the pitfall of the default limited -Depth of the ConvertTo-Json cmdlet,
See Sackoverflow: Unexpected ConvertTo-Json results? Answer: it has a default -Depth of 2

I guess this goes along with the fact that ConvertTo-Json terminates branches that are deeper than the default -Depth (2) with a (.Net) full type name. Therefore programmers assume a bug or a cmdlet limitation and do not read the help or about.
To my opinion, adding a full type name to a terminated branch doesn’t add much value, knowing that:

  • Json hardly preserves any (.Net) type names at all
  • You can’t rebuild the object from the type name without its data.

Meaning, that a simple ellipsis (three dots: ...) at the end of the cut off branch could suffice and have a clearer meaning that indicates an intentional omission of additional child properties.

Definition of an ellipsis (from Wikipedia):

An ellipsis (plural ellipses; from the Ancient Greek: ἔλλειψις, élleipsis, 'omission' or 'falling short') is a series of dots (typically three, such as ) that usually indicates an intentional omission of a word, sentence, or whole section from a text without altering its original meaning.

For compatibility, I would not use the Unicode ellipses character (U+2026) but simply 3 (ascii) dots: …

Taken the following commands:

$Test = @{Guid = New-Guid}
$Test.Parent = $Test
$Test | ConvertTo-Json

In stead of producing:

{
    "Guid":  "a274d017-5188-4d91-b960-023c06159dcc",
    "Parent":  {
                   "Guid":  "a274d017-5188-4d91-b960-023c06159dcc",
                   "Parent":  {
                                  "Guid":  "a274d017-5188-4d91-b960-023c06159dcc",
                                  "Parent":  "System.Collections.Hashtable"
                              }
               }
}

It should produce:

{
    "Guid":  "a274d017-5188-4d91-b960-023c06159dcc",
    "Parent":  {
                   "Guid":  "a274d017-5188-4d91-b960-023c06159dcc",
                   "Parent":  {
                                  "Guid":  "a274d017-5188-4d91-b960-023c06159dcc",
                                  "Parent":  "..."
                              }
               }
}
@iRon7 iRon7 changed the title ConvertTo-Json: terminate cut off branches with a ellipsis string rather than a (.Net) type ConvertTo-Json: terminate cut off branches with a ellipsis string rather than a full (.Net) type name Dec 2, 2018
@iSazonov
Copy link
Collaborator

iSazonov commented Dec 3, 2018

Related #8199
/cc @louistio

@iSazonov iSazonov added Issue-Enhancement the issue is more of a feature request than a bug WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module labels Dec 3, 2018
@iSazonov
Copy link
Collaborator

iSazonov commented Dec 3, 2018

Formally it is a breaking change and should be approved by PowerShell Committee. /cc @SteveL-MSFT

@adamgauthier
Copy link
Contributor

My understanding is that this change wouldn't really fix the root problem with ConvertTo-Json (the fact that default Depth is 2). However, it may help a user understanding what's happening? Not even sure, I have no strong opinion about this. @markekraus might?

@SteveL-MSFT SteveL-MSFT added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Dec 3, 2018
@mklement0
Copy link
Contributor

@louistio: Indeed it wouldn't fix the default -Depth problem, but the proposal makes sense independently of that.

Yes, it would provider a clearer (though not unambiguous) visual signal that something was truncated.

@mklement0
Copy link
Contributor

As for the -Depth 2 problem in Convert*To*-Json: please see #8393

@SteveL-MSFT SteveL-MSFT added Breaking-Change breaking change that may affect users Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Dec 5, 2018
@SteveL-MSFT
Copy link
Member

@PowerShell/powershell-committee reviewed this and it appears that the typename is the result of ToString() not being implemented so the type name is returned. Also, one can override ToString() to serialize to something more useful whereas producing "..." would limit this.

@markekraus
Copy link
Contributor

@SteveL-MSFT for clarity, what is the recommended implimentation?

if the type has ToString(), use that. If not, use ...

or

always use ...

It is not clear from your comment.

@mklement0
Copy link
Contributor

@markekraus, I think the recommendation is not to make any changes, based on the rationale that .ToString() representation may be meaningful.

That said, for programmatic processing, .ToString() output should never be relied upon.

Therefore, it comes down to:

  • If I am interested in the objects at a given level, I need to adjust -Depth accordingly, as only that gives me parseable output (within the limits of JSON).

  • Otherwise, how the objects at that level stringify is - by definition - of no interest, which is why a generic representation makes sense as a clear visual signal that the input had additional depth that was intentionally ignored.

@SteveL-MSFT
Copy link
Member

Sorry if not clear, but the recommendation as @mklement0 stated is to not make any change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking-Change breaking change that may affect users Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Enhancement the issue is more of a feature request than a bug Resolution-By Design The reported behavior is by design. WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module
Projects
None yet
Development

No branches or pull requests

6 participants