Force the use of v2 (non-legacy) conhost when in ConPTY mode #1935
Conversation
zadjii-msft
added
the
Product-Conpty
src/host/exemain.cpp
Outdated
| { | ||
| return fForceV1 || !ShouldUseConhostV2(); | ||
| return (args.GetForceV1() || !ShouldUseConhostV2()) && !args.InConptyMode(); |
There was a problem hiding this comment.
OK this is what we need, but can you make this a little less mentally stressful to read like
if (args.InConptyMode())
{
return false;
}
// etc...
Also, if you think ShouldUseConhostV2 is poorly named, please just rename it right now to be DoesRegistryPrefSayUseConhostV2 or something to clarify it's just the registry lookup and not something more elaborate than that.
There was a problem hiding this comment.
I hate that the most ideal name for that function is IsConhostV2NotForcedOffInRegistry (because it's unset or nonzero = v2, but set to 0 = v1)
There was a problem hiding this comment.
DoesRegistryForceConhostV1?
There was a problem hiding this comment.
That's not unreasonable. I want to keep the change scoped down so that it's more easily serviceable, though, and that requires us to flip the sense on the function
There was a problem hiding this comment.
Admittedly, it is clearer.
DHowett-MSFT
force-pushed
the
dev/duhowett/conptylegacy
branch
from
ea464ea
to
f24920e
Compare
|
🎉 Handy links: |
ghost
mentioned this pull request
Fixes #1838.
Summary of the Pull Request
This pull request ensures that conhost, when started in conpty mode (that is: with a signal pipe, an inPipe or an outPipe (the latter two of which are no longer supported)), launches in modern v2 mode and doesn't delegate to ConhostV1.dll.
PR Checklist
Use legacy consoleoption prevents Terminal from launching ConPTYs correctly #1838Detailed Description of the Pull Request / Additional comments
I chose to move the check into
ShouldUseLegacyConhost, but it could equally well belong inShouldUseConhostV2. Right now, "ShouldUse...V2" is dedicated to checking the registry. It's simple to change it, but this seemed like the most scoped change.The narrow scope of the change is intentional: perhaps we can try to service this because it represents a complete scenario breakage.
Validation Steps Performed
Manually replaced conhost on my machine and attempted to launch a pseudoconsole with pwincon while legacy mode was engaged.