-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Invariable order of $PSVersionTable #3530
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ internal class PSVersionInfo | |
internal const string PSVersionName = "PSVersion"; | ||
internal const string SerializationVersionName = "SerializationVersion"; | ||
internal const string WSManStackVersionName = "WSManStackVersion"; | ||
private static Hashtable s_psVersionTable = null; | ||
private static PSVersionHashTable s_psVersionTable = null; | ||
|
||
/// <summary> | ||
/// A constant to track current PowerShell Version. | ||
|
@@ -54,7 +54,7 @@ internal class PSVersionInfo | |
// Static Constructor. | ||
static PSVersionInfo() | ||
{ | ||
s_psVersionTable = new Hashtable(StringComparer.OrdinalIgnoreCase); | ||
s_psVersionTable = new PSVersionHashTable(StringComparer.OrdinalIgnoreCase); | ||
|
||
s_psVersionTable[PSVersionInfo.PSVersionName] = s_psV6Version; | ||
s_psVersionTable["PSEdition"] = PSEditionValue; | ||
|
@@ -71,7 +71,7 @@ static PSVersionInfo() | |
#endif | ||
} | ||
|
||
internal static Hashtable GetPSVersionTable() | ||
internal static PSVersionHashTable GetPSVersionTable() | ||
{ | ||
return s_psVersionTable; | ||
} | ||
|
@@ -315,6 +315,43 @@ internal static SemanticVersion PSV6Version | |
#endregion | ||
} | ||
|
||
/// <summary> | ||
/// Represents an implementation of '$PSVersionTable' variable. | ||
/// The implementation contains ordered 'Keys' and 'GetEnumerator' to get user-friendly output. | ||
/// </summary> | ||
public sealed class PSVersionHashTable : Hashtable, IEnumerable | ||
{ | ||
internal PSVersionHashTable(IEqualityComparer equalityComparer) : base(equalityComparer) | ||
{ | ||
} | ||
|
||
/// <summary> | ||
/// Returns ordered collection with Keys of 'PSVersionHashTable' | ||
/// </summary> | ||
public override ICollection Keys | ||
{ | ||
get | ||
{ | ||
Array arr = new string[base.Keys.Count]; | ||
base.Keys.CopyTo(arr, 0); | ||
Array.Sort(arr, StringComparer.OrdinalIgnoreCase); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I like this sorting - today There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree 100%. About the only thing I ever look for from this info is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have several open discussions on adding additional rows. Alphabetical order will be even more useful after that. If you want only PSVersion why don't use I wonder how it makes Windows PowerShell for I could slightly change this code to move PSVersion in the first place without moving to Update: Moving to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is a good point. Thanks @iSazonov There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider putting PSVersion and PSEdition in the first and second positions. After that, I don't care much about the order. I suppose alphabetical after the first two would be fine. |
||
return arr; | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Returns an enumerator for 'PSVersionHashTable'. | ||
/// The enumeration is ordered (based on ordered version of 'Keys'). | ||
/// </summary> | ||
IEnumerator IEnumerable.GetEnumerator() | ||
{ | ||
foreach (string key in Keys) | ||
{ | ||
yield return new System.Collections.DictionaryEntry(key, this[key]); | ||
} | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// An implementation of semantic versioning (http://semver.org) | ||
/// that can be converted to/from <see cref="System.Version"/>. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hashtable implements IEnumerable, so do you still need to specify
IEnumerable
here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
VS Code detects this as error and build failed too without explicity adding
IEnumerable
.