set identifying environment variable for new connections#897
set identifying environment variable for new connections#897adiviness merged 5 commits intomicrosoft:masterfrom binarycrusader:fw-sessionid
Conversation
zadjii-msft
left a comment
zadjii-msft
left a comment
There was a problem hiding this comment.
Overall looks really good
ghost
added
Needs-Author-Feedback
DHowett-MSFT
left a comment
DHowett-MSFT
left a comment
There was a problem hiding this comment.
This is really cool. Thanks for doing it!
I'm not 100% comfortable signing off, as I'd like @zadjii-msft and @adiviness to give it a twice-over, but I'm also not going to block.
ghost
added
Needs-Author-Feedback
Set a new 'WT_SESSION' environment variable when creating new terminal connections to allow shells to detect a unique Windows Terminal session. The value of the variable is a stringified GUID as returned by CoCreateGuid. How verified: - "razzle" & vs debug build - runut - manual inspection
* use Utils::GuidToString for guid stringification * expose guid parameter in ITerminalConnection idl
- misc. review fixes - throw if CreateConPty fails in ConhostConnection::Start - apply [[nodiscard]] and noexcept in various places
| void ConhostConnection::Start() | ||
| { | ||
| std::wstring cmdline = _commandline.c_str(); | ||
| std::wstring cmdline{ _commandline.c_str() }; |
There was a problem hiding this comment.
I'd prefer std::wstring cmdline{ _commandline.begin(), _commandline.end() }; because then we get guarantees about the bounds o the string we're using to initialize and we don't need to dip into the behind-the-scenes data members of std::string
There was a problem hiding this comment.
string has an explicit constructor from a T that has a data and size; maybe it can be used here?
There was a problem hiding this comment.
(std::wstring cmdline{_commandline})
|
I am willing to push this with the nits outstanding. @adiviness, would you block on #897 (comment)? |
|
naw, it's probably ok. We can always fix it later as well. |
|
@binarycrusader thanks so much for doing this. 👍 |
|
To reproduce, press the Windows key to open start, then type This now opens a Windows Terminal cmd.exe tab via |
Since v0.1.1431.0, Windows Terminal sets `WT_SESSION` environment variable that we may use to detect it. > See <microsoft/terminal#897> `TERM_PROGRAM` is not supported (yet ?). > See <microsoft/terminal#1828>
Right 😢 Also see #13006. |
6 participants
Summary of the Pull Request
Set a WT_SESSION environment variable to a unique guid on every new connection to allow shell consumers to detect Windows Terminal and uniquely identify the session.
References
Unknown.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Set a new 'WT_SESSION' environment variable when creating new terminal connections to allow shells to detect a unique Windows Terminal session. The value of the variable is a stringified GUID as returned by
CoCreateGuid.
How verified:
Uncertain about what tests to add and where; all of the existing ones passed.
I did some basic research to try to ensure that "WT_SESSION" is a "unique", non-conflicting environment variable and so shouldn't cause any problems with any existing programs. I'm certainly open to suggestions about the name though.