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

Remove type data for `System.Array` type #3222

Closed
PetSerAl opened this Issue Feb 27, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@PetSerAl
Contributor

PetSerAl commented Feb 27, 2017

Type data for System.Array type contains single alias property Count -> Length. It was good for consistency, as ICollection.Count is implemented explicitly by arrays and was not visible while other collections types do not have Length property. But since PowerShell v3 it is possible to reference explicitly implemented interface members. So, this alias property no longer necessary. It also cause some headaches with ConvertTo-Json cmdlet:

PS> ConvertTo-Json (,1)
[
  1
]
PS> ConvertTo-Json ([PSObject](,1))
{
  "value": [
    1
  ],
  "Count": 1
}
PS> Remove-TypeData System.Array
PS> ConvertTo-Json (,1)
[
  1
]
PS> ConvertTo-Json ([PSObject](,1))
[
  1
]
@daxian-dbw

This comment has been minimized.

Show comment
Hide comment
@daxian-dbw

daxian-dbw Feb 28, 2017

Member

It feels reasonable to remove the AliasProperty Count from System.Array, given that powershell now exposes the Count property implementation from ICollection interface. Feel free to submit a PR, the AliasProperty is defined here.

As for ConvertTo-Json, the issue you listed seems to have been fixed in powershell core:

Member

daxian-dbw commented Feb 28, 2017

It feels reasonable to remove the AliasProperty Count from System.Array, given that powershell now exposes the Count property implementation from ICollection interface. Feel free to submit a PR, the AliasProperty is defined here.

As for ConvertTo-Json, the issue you listed seems to have been fixed in powershell core:

@iSazonov

This comment has been minimized.

Show comment
Hide comment
@iSazonov

iSazonov Feb 28, 2017

Collaborator

@daxian-dbw What build do you use? I see the issue in alfa.16 (Windows 10 14936):

PS C:\Program Files\PowerShell\6.0.0.16> ConvertTo-Json ([psobject](,1))
{
    "value":  [
                  1
              ],
    "Count":  1
}
Collaborator

iSazonov commented Feb 28, 2017

@daxian-dbw What build do you use? I see the issue in alfa.16 (Windows 10 14936):

PS C:\Program Files\PowerShell\6.0.0.16> ConvertTo-Json ([psobject](,1))
{
    "value":  [
                  1
              ],
    "Count":  1
}
@PetSerAl

This comment has been minimized.

Show comment
Hide comment
@PetSerAl

PetSerAl Feb 28, 2017

Contributor

@daxian-dbw Is it OK to edit Types_Ps1Xml.generated.cs directly? I mean it have generated in name and this header:

//------------------------------------------------------------------------------
// <auto-generated>
//     This code was generated by a tool.
//
//     Changes to this file may cause incorrect behavior and will be lost if
//     the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------

It looks like it was automatically generated from Types.ps1xml file, but I can not find this file in repository.

Contributor

PetSerAl commented Feb 28, 2017

@daxian-dbw Is it OK to edit Types_Ps1Xml.generated.cs directly? I mean it have generated in name and this header:

//------------------------------------------------------------------------------
// <auto-generated>
//     This code was generated by a tool.
//
//     Changes to this file may cause incorrect behavior and will be lost if
//     the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------

It looks like it was automatically generated from Types.ps1xml file, but I can not find this file in repository.

@iSazonov

This comment has been minimized.

Show comment
Hide comment
@iSazonov

iSazonov Feb 28, 2017

Collaborator

@PetSerAl Now Types.ps1xml is in code for faster Powershell start. You can modify Types_Ps1Xml.generated.cs (see the file history on GitHub).

Collaborator

iSazonov commented Feb 28, 2017

@PetSerAl Now Types.ps1xml is in code for faster Powershell start. You can modify Types_Ps1Xml.generated.cs (see the file history on GitHub).

@lzybkr

This comment has been minimized.

Show comment
Hide comment
@lzybkr

lzybkr Feb 28, 2017

Member

The file was generated to keep the ps1xml in sync, but we no longer maintain the ps1xml, so we should remove the comment and rename the file.

Member

lzybkr commented Feb 28, 2017

The file was generated to keep the ps1xml in sync, but we no longer maintain the ps1xml, so we should remove the comment and rename the file.

@daxian-dbw

This comment has been minimized.

Show comment
Hide comment
@daxian-dbw

daxian-dbw Feb 28, 2017

Member

@iSazonov My bad, I must have tried that after Remove-TypeData. Removing the alias property Count will fix that.

Member

daxian-dbw commented Feb 28, 2017

@iSazonov My bad, I must have tried that after Remove-TypeData. Removing the alias property Count will fix that.

@iSazonov

This comment has been minimized.

Show comment
Hide comment
@iSazonov

iSazonov Feb 28, 2017

Collaborator

@daxian-dbw Clear.

@lzybkr I searched *generated.* - there are multiple files. Are there files that we also should fix?

Collaborator

iSazonov commented Feb 28, 2017

@daxian-dbw Clear.

@lzybkr I searched *generated.* - there are multiple files. Are there files that we also should fix?

@lzybkr

This comment has been minimized.

Show comment
Hide comment
@lzybkr

lzybkr Feb 28, 2017

Member

For now, I would just change:

GetEvent_Types_Ps1Xml.generated.cs
TypesV3_Ps1Xml.generated.cs
Types_Ps1Xml.generated.cs

The files in GraphicalHost can be ignored, and the ones in the interpreter come from another project, so it would be better to not rename those.

Member

lzybkr commented Feb 28, 2017

For now, I would just change:

GetEvent_Types_Ps1Xml.generated.cs
TypesV3_Ps1Xml.generated.cs
Types_Ps1Xml.generated.cs

The files in GraphicalHost can be ignored, and the ones in the interpreter come from another project, so it would be better to not rename those.

@iSazonov

This comment has been minimized.

Show comment
Hide comment
@iSazonov

iSazonov Feb 28, 2017

Collaborator

I opened Issue #3230

Collaborator

iSazonov commented Feb 28, 2017

I opened Issue #3230

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment