GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||
GeezerVison is showing github.com — [feature] Add -CustomPipeName to `pwsh` and `Enter-PSHostProcess` by TylerLeonhardt · Pull Request #8889 · PowerShell/PowerShell Conversation
|
|
Paul Higinbotham (@PaulHigin) I've refactored this quite a bit based on your offline feedback. The same experience is there for a user at the CLI. For users that are going through the API, the can use: S.M.A.Remoting.RemoteSessionNamedPipeServer.CreateCustomNamedPipeServer("mypipe"); There is 1 thing left to do here:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
d47a632 to
d7df174
Compare
pwsh and Enter-PSHostProcesspwsh and Enter-PSHostProcess
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Addressed all feedback 👍 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM. But please create new Issue about disposing pipe server objects on process exit (please see comment).
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Ok I included a few CodeFactor and Codacy fixes as well. This should be good to go pending CI ✅ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I've addressed Paul Higinbotham (@PaulHigin)'s final point which is that It's mostly used for debugging though... and our usecase is really for debugging... so I've called this out in the His recommendation and what I went with was |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
pwsh and Enter-PSHostProcesspwsh and Enter-PSHostProcess
| case ProcessIdParameterSet: | ||
| Process = GetProcessById(Id); | ||
| VerifyProcess(Process); | ||
| namedPipeRunspace = CreateNamedPipeRunspace(Process.Id, AppDomainName); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Interesting, do we really need the VerifyProcess() check (that is File.Exist())? Perhaps the runtime error is informative enough.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite sure what you're saying. This VerifyProcess is to keep the behavior consistent with the current behavior for Process parameter sets.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
TylerLeonhardt Please create new Issue in PowerShell-Docs repo and add reference to the PR description. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Alright CI is green and codacy and Code Factor are mostly happy. This should be good to go. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM with minor comments.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
TylerLeonhardt Please create new Issue in PowerShell-Docs repo before we merge. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Done, Ilya (@iSazonov)! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
TylerLeonhardt Thanks for your contribution! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…werShell#8889) This allows a user to start PowerShell up with the name of the named pipe that is used for cross process communication (I.e. Enter-PSHostProcess).
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.