Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ public object[] Property

private object[] _props;

/// <summary>
/// Gets or sets the properties to exclude from formatting.
/// </summary>
[Parameter]
public string[] ExcludeProperty { get; set; }

/// <summary>
/// </summary>
/// <value></value>
Expand All @@ -61,22 +67,36 @@ internal override FormattingCommandLineParameters GetCommandLineParameters()
{
FormattingCommandLineParameters parameters = new();

// Check View conflicts first (before any auto-expansion)
if (!string.IsNullOrEmpty(this.View))
{
// View cannot be used with Property or ExcludeProperty
if ((_props is not null && _props.Length != 0) || (ExcludeProperty is not null && ExcludeProperty.Length != 0))
{
ReportCannotSpecifyViewAndProperty();
}

parameters.viewName = this.View;
}

if (_props != null)
{
ParameterProcessor processor = new(new FormatObjectParameterDefinition());
TerminatingErrorContext invocationContext = new(this);
parameters.mshParameterList = processor.ProcessParameters(_props, invocationContext);
}

if (!string.IsNullOrEmpty(this.View))
if (ExcludeProperty is not null)
{
// we have a view command line switch
if (parameters.mshParameterList.Count != 0)
parameters.excludePropertyFilter = new PSPropertyExpressionFilter(ExcludeProperty);

// ExcludeProperty implies -Property * for better UX
if (_props is null || _props.Length == 0)
{
ReportCannotSpecifyViewAndProperty();
ParameterProcessor processor = new(new FormatObjectParameterDefinition());
TerminatingErrorContext invocationContext = new(this);
parameters.mshParameterList = processor.ProcessParameters(new object[] { "*" }, invocationContext);
}

parameters.viewName = this.View;
}

parameters.groupByParameter = this.ProcessGroupByParameter();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,14 @@ public object Property
private object _prop;

/// <summary>
/// Optional, non positional parameter.
/// Gets or sets the properties to exclude from formatting.
/// </summary>
[Parameter]
public string[] ExcludeProperty { get; set; }

/// <summary>
/// Gets or sets a value indicating whether to autosize the output.
/// </summary>
/// <value></value>
[Parameter]
public SwitchParameter AutoSize
{
Expand Down Expand Up @@ -74,22 +79,36 @@ internal override FormattingCommandLineParameters GetCommandLineParameters()
{
FormattingCommandLineParameters parameters = new();

// Check View conflicts first (before any auto-expansion)
if (!string.IsNullOrEmpty(this.View))
{
// View cannot be used with Property or ExcludeProperty
if (_prop is not null || (ExcludeProperty is not null && ExcludeProperty.Length != 0))
{
ReportCannotSpecifyViewAndProperty();
}

parameters.viewName = this.View;
}

if (_prop != null)
{
ParameterProcessor processor = new(new FormatWideParameterDefinition());
TerminatingErrorContext invocationContext = new(this);
parameters.mshParameterList = processor.ProcessParameters(new object[] { _prop }, invocationContext);
}

if (!string.IsNullOrEmpty(this.View))
if (ExcludeProperty is not null)
{
// we have a view command line switch
if (parameters.mshParameterList.Count != 0)
parameters.excludePropertyFilter = new PSPropertyExpressionFilter(ExcludeProperty);

// ExcludeProperty implies -Property * for better UX
if (_prop is null)
Comment thread
yotsuda marked this conversation as resolved.
{
ReportCannotSpecifyViewAndProperty();
ParameterProcessor processor = new(new FormatWideParameterDefinition());
TerminatingErrorContext invocationContext = new(this);
parameters.mshParameterList = processor.ProcessParameters(new object[] { "*" }, invocationContext);
}

parameters.viewName = this.View;
}

// we cannot specify -column and -autosize, they are mutually exclusive
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,47 +12,6 @@

namespace Microsoft.PowerShell.Commands
{
/// <summary>
/// Helper class to do wildcard matching on PSPropertyExpressions.
/// </summary>
internal sealed class PSPropertyExpressionFilter
{
/// <summary>
/// Initializes a new instance of the <see cref="PSPropertyExpressionFilter"/> class
/// with the specified array of patterns.
/// </summary>
/// <param name="wildcardPatternsStrings">Array of pattern strings to use.</param>
internal PSPropertyExpressionFilter(string[] wildcardPatternsStrings)
{
ArgumentNullException.ThrowIfNull(wildcardPatternsStrings);

_wildcardPatterns = new WildcardPattern[wildcardPatternsStrings.Length];
for (int k = 0; k < wildcardPatternsStrings.Length; k++)
{
_wildcardPatterns[k] = WildcardPattern.Get(wildcardPatternsStrings[k], WildcardOptions.IgnoreCase);
}
}

/// <summary>
/// Try to match the expression against the array of wildcard patterns.
/// The first match shortcircuits the search.
/// </summary>
/// <param name="expression">PSPropertyExpression to test against.</param>
/// <returns>True if there is a match, else false.</returns>
internal bool IsMatch(PSPropertyExpression expression)
{
for (int k = 0; k < _wildcardPatterns.Length; k++)
{
if (_wildcardPatterns[k].IsMatch(expression.ToString()))
return true;
}

return false;
}

private readonly WildcardPattern[] _wildcardPatterns;
}

internal sealed class SelectObjectExpressionParameterDefinition : CommandParameterDefinition
{
protected override void SetEntries()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Management.Automation;
using System.Management.Automation.Internal;
using System.Management.Automation.Runspaces;
using Microsoft.PowerShell.Commands;

namespace Microsoft.PowerShell.Commands.Internal.Format
{
Expand Down Expand Up @@ -729,6 +730,12 @@ public class OuterFormatTableAndListBase : OuterFormatShapeCommandBase
[Parameter(Position = 0)]
public object[] Property { get; set; }

/// <summary>
/// Optional parameter for excluding properties from formatting.
/// </summary>
[Parameter]
public string[] ExcludeProperty { get; set; }

#endregion

internal override FormattingCommandLineParameters GetCommandLineParameters()
Expand All @@ -751,6 +758,18 @@ internal override FormattingCommandLineParameters GetCommandLineParameters()

internal void GetCommandLineProperties(FormattingCommandLineParameters parameters, bool isTable)
{
// Check View conflicts first (before any auto-expansion)
if (!string.IsNullOrEmpty(this.View))
{
// View cannot be used with Property or ExcludeProperty
if ((Property is not null && Property.Length != 0) || (ExcludeProperty is not null && ExcludeProperty.Length != 0))
{
ReportCannotSpecifyViewAndProperty();
}

parameters.viewName = this.View;
}

if (Property != null)
{
CommandParameterDefinition def;
Expand All @@ -765,15 +784,21 @@ internal void GetCommandLineProperties(FormattingCommandLineParameters parameter
parameters.mshParameterList = processor.ProcessParameters(Property, invocationContext);
}

if (!string.IsNullOrEmpty(this.View))
if (ExcludeProperty is not null)
{
// we have a view command line switch
if (parameters.mshParameterList.Count != 0)
parameters.excludePropertyFilter = new PSPropertyExpressionFilter(ExcludeProperty);

// ExcludeProperty implies -Property * for better UX
if (Property is null || Property.Length == 0)
{
ReportCannotSpecifyViewAndProperty();
}
CommandParameterDefinition def = isTable
? new FormatTableParameterDefinition()
: new FormatListParameterDefinition();
ParameterProcessor processor = new ParameterProcessor(def);
TerminatingErrorContext invocationContext = new TerminatingErrorContext(this);

parameters.viewName = this.View;
parameters.mshParameterList = processor.ProcessParameters(new object[] { "*" }, invocationContext);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for the PR.
We have huge places in our code base where we allocate such temp arrays. We could review this and use modern C# feature params ReadOnlySpan<YYY> to pass arguments in methods without allocation.

}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ internal sealed class FormattingCommandLineParameters
/// Extension mechanism for shape specific parameters.
/// </summary>
internal ShapeSpecificParameters shapeParameters = null;

/// <summary>
/// Filter for excluding properties from formatting.
/// </summary>
internal PSPropertyExpressionFilter excludePropertyFilter = null;
Comment thread
yotsuda marked this conversation as resolved.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question about the design. We don't keep property filter but only exclude filter. Is this really necessary? Looking how ApplyExcludePropertyFilter is inserted a question is - wouldn't it be more correct to apply the filter exactly at the time of adding new property in the list?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, storing the filter is necessary to pass it from the cmdlet to ViewGenerator. Here's the class structure:

Format-Table cmdlet
    └── InnerFormatShapeCommand
            └── _viewManager: FormatViewManager
                    └── _viewGenerator: ViewGenerator (TableViewGenerator, etc.)
                            └── activeAssociationList (property list)
  • ExcludeProperty parameter is defined in the cmdlet
  • Property list is built in ViewGenerator.Initialize() using the input object (for wildcard expansion, DefaultPropertySet, etc.)
  • ApplyExcludePropertyFilter() is defined in ViewGenerator (the base class of TableViewGenerator, ListViewGenerator, etc.)

Since the cmdlet and ViewGenerator are separate classes, we need a way to pass ExcludeProperty between them. ViewGenerator.Initialize() already receives FormattingCommandLineParameters, so adding excludePropertyFilter there was a natural way to pass it - same as -Property which is stored as mshParameterList in the same class.

Even with add-time filtering, we would still need to add a member to FormattingCommandLineParameters because that's the bridge between the cmdlet and ViewGenerator.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main concern is that we call ApplyExcludePropertyFilter() 8 times. It is looks as not reliable, it is easy to make an error.
Also we have some places where we access activeAssociationList and this rises a question should it return already filtered list?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback. You're right - calling ApplyExcludePropertyFilter() at multiple exit points is error-prone.

I've refactored both methods to eliminate multiple return statements and call ApplyExcludePropertyFilter() exactly once at the end:

  • Table Initialize(): 3 calls → 1 call
  • Wide SetUpActiveProperty(): 4 calls → 1 call

Regarding activeAssociationList access: The existing design doesn't prevent access to activeAssociationList before Initialize(), but I've verified no such access exists. All external access (e.g., GeneratePayload(), GenerateHeaderInfo()) happens after Initialize() completes, so they always see the filtered list.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good step forward. Thanks!
Nevertheless, can we think about making it more generic? Can it be in base.Initialize() for example? It would be great.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the positive feedback and the suggestion!

I explored moving ApplyExcludePropertyFilter() into base.Initialize(), but found some structural challenges:

Current Structure:

ViewGenerator When activeAssociationList is built
Table During Initialize()
List During Initialize()
Wide Lazily in GeneratePayload() via SetUpActiveProperty()
Complex Not used (uses ComplexViewObjectBrowser)

Challenges with base.Initialize():

  1. Timing: activeAssociationList is built by derived classes after base.Initialize() returns, so it would be null at that point
  2. Wide's lazy initialization: Wide builds the list during GeneratePayload(), not during Initialize()
  3. Complex doesn't use it: Uses a different rendering path

Possible approach - Template Method pattern:

// ViewGenerator (base)
internal virtual void Initialize(...) {
    InitializeHelper();
    BuildActiveAssociationList(so);  // virtual, overridden by derived classes
    ApplyExcludePropertyFilter();
}

However, this wouldn't work for Wide's lazy initialization pattern without additional refactoring.

Current approach:
Each Initialize method calls ApplyExcludePropertyFilter() once at the end, which handles all cases including Wide's lazy pattern.

Would you prefer me to explore the Template Method approach further, or is the current single-call-per-Initialize approach acceptable given these constraints?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally it would be great to have clean class design. But if it requires a lot of refactoring, then it's better to postpone it, i.e. I can accept the PR as it is now, and you can do the refactoring in a subsequent PR if you want.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the flexibility and understanding!

I agree that a cleaner centralized approach would require some additional refactoring. The current approach—calling ApplyExcludePropertyFilter() once at the end of each Initialize method—keeps the changes minimal and focused on the -ExcludeProperty feature.

I've opened #26568 to track the potential refactoring for the future. Please feel free to assign it to me if you'd like.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to grab the work. We are in beginning of next version dev cycle so we can do formatting and refactoring that way there's enough time to stabilize.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I'll take on #26568.
Since the refactoring depends on the code introduced in this PR, I'll start working on it after this PR is merged.

}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System.Collections.Generic;
using System.Linq;
using System.Collections.ObjectModel;
using System.Management.Automation;
using System.Management.Automation.Internal;
Expand Down Expand Up @@ -348,7 +349,19 @@ protected class DataBaseInfo
protected DataBaseInfo dataBaseInfo = new DataBaseInfo();

protected List<MshResolvedExpressionParameterAssociation> activeAssociationList = null;
protected FormattingCommandLineParameters inputParameters = null;
/// <summary>
/// Apply ExcludeProperty filter to activeAssociationList if specified.
/// This method filters and updates "activeAssociationList" instance property.
/// </summary>
protected void ApplyExcludePropertyFilter()
{
if (this.parameters is not null && this.parameters.excludePropertyFilter is not null)
{
this.activeAssociationList = this.activeAssociationList
.Where(item => !this.parameters.excludePropertyFilter.IsMatch(item.ResolvedExpression))
.ToList();
}
}

protected string GetExpressionDisplayValue(PSObject so, int enumerationLimit, PSPropertyExpression ex,
FieldFormattingDirective directive)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Collections.ObjectModel;
using System.Management.Automation;
using System.Management.Automation.Internal;
Expand All @@ -16,7 +17,6 @@ internal override void Initialize(TerminatingErrorContext errorContext, PSProper
PSObject so, TypeInfoDataBase db, FormattingCommandLineParameters parameters)
{
base.Initialize(errorContext, expressionFactory, so, db, parameters);
this.inputParameters = parameters;
}

internal override FormatStartData GenerateStartData(PSObject so)
Expand All @@ -40,7 +40,7 @@ internal override FormatEntryData GeneratePayload(PSObject so, int enumerationLi
private ComplexViewEntry GenerateComplexViewEntryFromProperties(PSObject so, int enumerationLimit)
{
ComplexViewObjectBrowser browser = new ComplexViewObjectBrowser(this.ErrorManager, this.expressionFactory, enumerationLimit);
return browser.GenerateView(so, this.inputParameters);
return browser.GenerateView(so, this.parameters);
}

private ComplexViewEntry GenerateComplexViewEntryFromDataBaseInfo(PSObject so, int enumerationLimit)
Expand Down Expand Up @@ -425,17 +425,18 @@ internal ComplexViewObjectBrowser(FormatErrorManager resultErrorManager, PSPrope
/// of the object.
/// </summary>
/// <param name="so">Object to process.</param>
/// <param name="inputParameters">Parameters from the command line.</param>
/// <param name="parameters">Parameters from the command line.</param>
/// <returns>Complex view entry to send to the output command.</returns>
internal ComplexViewEntry GenerateView(PSObject so, FormattingCommandLineParameters inputParameters)
internal ComplexViewEntry GenerateView(PSObject so, FormattingCommandLineParameters parameters)
{
_complexSpecificParameters = (ComplexSpecificParameters)inputParameters.shapeParameters;
_parameters = parameters;
_complexSpecificParameters = (ComplexSpecificParameters)parameters.shapeParameters;

int maxDepth = _complexSpecificParameters.maxDepth;
TraversalInfo level = new TraversalInfo(0, maxDepth);

List<MshParameter> mshParameterList = null;
mshParameterList = inputParameters.mshParameterList;
mshParameterList = parameters.mshParameterList;

// create a top level entry as root of the tree
ComplexViewEntry cve = new ComplexViewEntry();
Expand Down Expand Up @@ -513,6 +514,14 @@ private void DisplayObject(PSObject so, TraversalInfo currentLevel, List<MshPara
List<MshResolvedExpressionParameterAssociation> activeAssociationList =
AssociationManager.SetupActiveProperties(parameterList, so, _expressionFactory);

// Apply ExcludeProperty filter if specified
if (_parameters != null && _parameters.excludePropertyFilter != null)
{
activeAssociationList = activeAssociationList
.Where(item => !_parameters.excludePropertyFilter.IsMatch(item.ResolvedExpression))
.ToList();
}

// create a format entry
FormatEntry fe = new FormatEntry();
formatValueList.Add(fe);
Expand Down Expand Up @@ -758,6 +767,7 @@ private List<FormatValue> AddIndentationLevel(List<FormatValue> formatValueList)
return feFrame.formatValueList;
}

private FormattingCommandLineParameters _parameters;
private ComplexSpecificParameters _complexSpecificParameters;

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal override void Initialize(TerminatingErrorContext errorContext, PSProper
_listBody = (ListControlBody)this.dataBaseInfo.view.mainControl;
}

this.inputParameters = parameters;
this.parameters = parameters;
SetUpActiveProperties(so);
}

Expand Down Expand Up @@ -227,10 +227,12 @@ private void SetUpActiveProperties(PSObject so)
{
List<MshParameter> mshParameterList = null;

if (this.inputParameters != null)
mshParameterList = this.inputParameters.mshParameterList;
if (this.parameters != null)
mshParameterList = this.parameters.mshParameterList;

this.activeAssociationList = AssociationManager.SetupActiveProperties(mshParameterList, so, this.expressionFactory);

ApplyExcludePropertyFilter();
}
}
}
Loading
Loading