-
Notifications
You must be signed in to change notification settings - Fork 7.2k
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
Add support to experimental features #7242
Changes from 16 commits
eed9842
930350e
8a5ddbb
7770aa5
dc2d741
d8d3344
3c3ddeb
d2e9151
bc8abf8
2fce1bf
1033422
b616cf5
c7c35bd
2c9519b
0ac878f
add14e4
d12230f
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 |
---|---|---|
|
@@ -230,6 +230,10 @@ internal static IEnumerable<ExtendedTypeDefinition> GetFormatData() | |
"System.Management.Automation.PSModuleInfo", | ||
ViewsOf_System_Management_Automation_PSModuleInfo()); | ||
|
||
yield return new ExtendedTypeDefinition( | ||
"System.Management.Automation.ExperimentalFeature", | ||
ViewsOf_System_Management_Automation_ExperimentalFeature()); | ||
|
||
var td46 = new ExtendedTypeDefinition( | ||
"Microsoft.PowerShell.Commands.BasicHtmlWebResponseObject", | ||
ViewsOf_Microsoft_PowerShell_Commands_BasicHtmlWebResponseObject()); | ||
|
@@ -1240,6 +1244,33 @@ private static IEnumerable<FormatViewDefinition> ViewsOf_System_Management_Autom | |
.EndList()); | ||
} | ||
|
||
private static IEnumerable<FormatViewDefinition> ViewsOf_System_Management_Automation_ExperimentalFeature() | ||
{ | ||
yield return new FormatViewDefinition("ExperimentalFeature", | ||
TableControl.Create() | ||
.AddHeader(Alignment.Left, width: 35) | ||
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. Seems like 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. When it's a module feature (feature declared in module), the feature name would be longer than the module name. The width for module name is 35, and I also use 35 here for the feature name. If you still think we should use a smaller size, I can make that change. 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. Ok, for a module it makes sense. Let's leave it at 35 for now. Formatting changes aren't considered breaking so we can adjust later. |
||
.AddHeader(Alignment.Right, width: 7) | ||
.AddHeader(Alignment.Left, width: 35) | ||
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. 35 here also seems wide for the 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. For a module feature, the source is the path to the module manifest file. That's why I also make the width 35. Let me know if you think we should use a smaller size. 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. Ok for now. |
||
.AddHeader(Alignment.Left) | ||
.StartRowDefinition() | ||
.AddPropertyColumn("Name") | ||
.AddPropertyColumn("Enabled") | ||
.AddPropertyColumn("Source") | ||
.AddPropertyColumn("Description") | ||
.EndRowDefinition() | ||
.EndTable()); | ||
|
||
yield return new FormatViewDefinition("ExperimentalFeature", | ||
ListControl.Create() | ||
.StartEntry() | ||
.AddItemProperty("Name") | ||
.AddItemProperty("Enabled") | ||
.AddItemProperty("Source") | ||
.AddItemProperty("Description") | ||
.EndEntry() | ||
.EndList()); | ||
} | ||
|
||
private static IEnumerable<FormatViewDefinition> ViewsOf_Microsoft_PowerShell_Commands_BasicHtmlWebResponseObject() | ||
{ | ||
yield return new FormatViewDefinition("Microsoft.PowerShell.Commands.BasicHtmlWebResponseObject", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1756,13 +1756,15 @@ private List<CompletionResult> GetResultForAttributeArgument(CompletionContext c | |
{ | ||
PropertyInfo[] propertyInfos = attributeType.GetProperties(BindingFlags.Public | BindingFlags.Instance); | ||
List<CompletionResult> result = new List<CompletionResult>(); | ||
foreach (PropertyInfo pro in propertyInfos) | ||
foreach (PropertyInfo property in propertyInfos) | ||
{ | ||
//Ignore TypeId (all attributes inherit it) | ||
if (pro.Name != "TypeId" && (pro.Name.StartsWith(argName, StringComparison.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. why did the 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. The property |
||
// Ignore getter-only properties, including 'TypeId' (all attributes inherit it). | ||
if (!property.CanWrite) { continue; } | ||
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. Seems our convention is if (!property.CanWrite) continue; or if (!property.CanWrite)
{
continue;
} I'd prefer second. 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 use 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. Certainly agree that we should always use braces. In a world where we no longer use teletypes (i.e. where screens are big and newlines are cheap), I tend to feel that we should always just hang the line so that as my eyes scan down a document at an approximate indentation level and always know what's going on. But, that's just a style preference, and inline-braces are still pretty good to me. |
||
|
||
if (property.Name.StartsWith(argName, StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
result.Add(new CompletionResult(pro.Name, pro.Name, CompletionResultType.Property, | ||
pro.PropertyType.ToString() + " " + pro.Name)); | ||
result.Add(new CompletionResult(property.Name, property.Name, CompletionResultType.Property, | ||
property.PropertyType.ToString() + " " + property.Name)); | ||
} | ||
} | ||
return result; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1698,12 +1698,10 @@ private static void ProcessParameter( | |
|
||
foreach (ValidateArgumentsAttribute att in parameter.Parameter.ValidationAttributes) | ||
{ | ||
if (att is ValidateSetAttribute) | ||
if (att is ValidateSetAttribute setAtt) | ||
{ | ||
RemoveLastNullCompletionResult(result); | ||
|
||
var setAtt = (ValidateSetAttribute)att; | ||
|
||
string wordToComplete = context.WordToComplete; | ||
string quote = HandleDoubleAndSingleQuote(ref wordToComplete); | ||
|
||
|
@@ -1712,6 +1710,8 @@ private static void ProcessParameter( | |
|
||
foreach (string value in setAtt.ValidValues) | ||
{ | ||
if (value == string.Empty) { continue; } | ||
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 assume we're deliberately treating 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. It's impossible for a null to appear in |
||
|
||
if (wordToComplete.Equals(value, StringComparison.OrdinalIgnoreCase)) | ||
{ | ||
string completionText = quote == string.Empty ? value : quote + value + quote; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,14 +34,31 @@ internal CommandProcessorBase() | |
/// The metadata about the command to run. | ||
/// </param> | ||
/// | ||
internal CommandProcessorBase( | ||
CommandInfo commandInfo) | ||
internal CommandProcessorBase(CommandInfo commandInfo) | ||
{ | ||
if (commandInfo == null) | ||
{ | ||
throw PSTraceSource.NewArgumentNullException("commandInfo"); | ||
} | ||
|
||
if (commandInfo is IScriptCommandInfo scriptCommand) | ||
{ | ||
ExperimentalAttribute expAttribute = scriptCommand.ScriptBlock.ExperimentalAttribute; | ||
if (expAttribute != null && expAttribute.ToHide) | ||
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. If 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. Yes. This is to handle users running a script block like the following
|
||
{ | ||
string errorTemplate = expAttribute.ExperimentAction == ExperimentAction.Hide | ||
? DiscoveryExceptions.ScriptDisabledWhenFeatureOn | ||
: DiscoveryExceptions.ScriptDisabledWhenFeatureOff; | ||
string errorMsg = StringUtil.Format(errorTemplate, expAttribute.ExperimentName); | ||
ErrorRecord errorRecord = new ErrorRecord( | ||
new InvalidOperationException(errorMsg), | ||
"ScriptCommandDisabled", | ||
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. No preference in any direction, but do we need new resource entries for new error IDs? 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. Not necessarily. Here, we use the same error id but the message could change depending on the experiment action. |
||
ErrorCategory.InvalidOperation, | ||
commandInfo); | ||
throw new CmdletInvocationException(errorRecord); | ||
} | ||
} | ||
|
||
CommandInfo = commandInfo; | ||
} | ||
|
||
|
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.
Is there a space needed between
file.
and%n
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.
%n
is a new line, so no space is needed here. ForException: %1 %n Message: %2 %n StackTrace: %3 %n
, the space character before%n
is not necessary, but it's been that way in other places of this manifest file, so I followed the existing pattern.