Revert refactoring changes that broke remoting to Windows PowerShell 5.1 #5321
Conversation
…ws PowerShell 5.1
SteveL-MSFT
requested review from
BrucePay,
daxian-dbw and
lzybkr
as code owners
lzybkr
left a comment
lzybkr
left a comment
There was a problem hiding this comment.
Sigh - it's silly that the protocol requires the names of the private fields. I suppose it's just an expedient implementation detail - just use reflection, but it's so brittle.
|
@lzybkr I agree now that I've learned how this aspect of the protocol works |
| public sealed | ||
| class ChoiceDescription | ||
| { | ||
| // DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting |
There was a problem hiding this comment.
I think the warning messages should say "it will break remoting compatibility with Windows PowerShell".
There was a problem hiding this comment.
This is incomplete. To fully fix this incompatibility we need to revert field names for all encoded class/structs. Here is the supported list:
https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/engine/remoting/common/WireDataFormat/RemoteHostEncoder.cs#L41
There was a problem hiding this comment.
I reused an existing comment. I can update all occurrences of that comment to be more specific. Thanks for the pointer to the list of types, I'll fix those as well.
|
@SteveL-MSFT One of the commits is not linked to your github account. |
SteveL-MSFT
force-pushed
the
remoting-callback
branch
from
930e4ab to
0c14e56
Compare
|
@TravisEz13 fixed |
| public sealed | ||
| class ChoiceDescription | ||
| { | ||
| // DO NOT REMOVE OR RENAME THESE FIELDS - it will break remoting compatibility with Windows PowerShell compatibility with Windows PowerShell |
There was a problem hiding this comment.
What about putting these fields within a region to make the grouping more explicit? The "Do Not Remove or Rename" region?
There was a problem hiding this comment.
Good idea. Will make an update.
|
@TravisEz13 can you merge? |
6 participants
Call backs depend on private members to have the same name across versions of PowerShell. Specifically fixes prompts and clear.
@PaulHigin are there other callbacks I need to fix? I noticed that ctrl+c doesn't work from PSCore6 to Windows PowerShell 5.1.
Fix #4615