-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Fix Property and ScriptBlock expressions in EntrySelectedBy tags within custom controls #7913
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
Fix Property and ScriptBlock expressions in EntrySelectedBy tags within custom controls #7913
Conversation
Send the current object when evaluating `EntrySelectedBy` in a custom control definition
I think we need tests. |
@SeeminglyScience please add tests to Format-Custom.Tests.ps1. You can look at this as an example. |
@SteveL-MSFT Sorry I saw your reply just a few minutes late. I've added tests, though they are in a different area. I'll go ahead and move them there. |
testing | ||
'@ -replace '\r?\n', [Environment]::NewLine | ||
|
||
$ps.Invoke()[0].Trim() | Should -BeExactly $expectedOutput |
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.
Why index 0 and trim?
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.
Index 0 because I know the collection will only have one item. It's mostly just habit from methods that could exist on the collection as well like ToString
, I can remove it.
Trim because white space isn't important to the test, and it saves some white space when declaring the expected output. That's just cosmetic and can also be changed.
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.
I understand the habit, but prefer to remove the index. With regard to whitespace, I prefer to explicitly check for it as we had regressions previously with regard to whitespace in the output particularly with tables.
@SteveL-MSFT Please update your review. |
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.
LGTM
@SeeminglyScience Thanks for your contribution! |
…in custom controls (PowerShell#7913) Send the current object when evaluating `EntrySelectedBy` in a custom control definition
PR Summary
This change sends the current object to expressions defined in
EntrySelectedBy
tags within custom controls. Currently attempting to use property expression binding will result in aNullReferenceException
, and within a script block expression binding$_/$PSItem
will not be set.Resolves #7847
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
to the beginning of the title and remove the prefix when the PR is ready.[feature]
if the change is significant or affects feature tests