Implement OSC 7 for setting the CWD#20019
Conversation
carlos-zamora
left a comment
There was a problem hiding this comment.
Looks good! Just the minor comments and it looks like you need to run the code formatter. I'll circle back to this then.
DHowett
left a comment
There was a problem hiding this comment.
blocking to think about my opinion
| // and the resulting path can never be longer than the URI. | ||
| const auto ptr = path.data(); | ||
| auto len = gsl::narrow<DWORD>(path.size()); | ||
| THROW_IF_FAILED(PathCreateFromUrlW(ptr, ptr, &len, 0)); |
There was a problem hiding this comment.
this will add a dependency from conhost on the library containing PathCreateFromUrlW; is that acceptable?
There was a problem hiding this comment.
api-ms-win-core-url-l1-1-0
PathCreateFromUrlW
hmm where are you hosted little guy
There was a problem hiding this comment.
> $d.Apis["PathCreateFromUrlW"][1].Binary
Architecture : Amd64
BinplacePath : kernelbase.dll
wellp
|
|
||
| winrt::hstring ControlCore::CurrentWorkingDirectory() const | ||
| { | ||
| return winrt::hstring{ _terminal->GetWorkingDirectory() }; |
There was a problem hiding this comment.
i s2g, we just had a whole-ass duplicate of this function?
There was a problem hiding this comment.
Yeah lol. I was super surprised about that too. We don't have skeletons in our basement, we got entire graveyards apparently lmao.
| } | ||
|
|
||
| WIN32_FILE_ATTRIBUTE_DATA data; | ||
| const auto ok = GetFileAttributesExW(path, GetFileExInfoStandard, &data); |
There was a problem hiding this comment.
instead of std::filesystem::access or whatever? cool w/ me
There was a problem hiding this comment.
Yeah, I just hate that the std::filesystem::path objects are expensive to construct. Under the hood, it gets passed to __std_fs_get_stats, which - and you will never guess this - takes a null-terminated string. 🤦♂️
Hurr durr "C++ is for low level zero overhead real man's man engineering."
Meanwhile: Wrap in wstring to unwrap in pointer. lmao
This adds support for OSC 7 which is the same as OSC 9;9 but using file URIs. As per the previous discussion in #8214, the problem is that WSL shells emit URIs that can't work because the hostname in the URI translates to an UNC path with an non-existing hostname. In my opinion this doesn't deter the fact though that OSC 7 works just fine for a native Windows application. In the future we should consider trying to detect WSL file URIs and translating it to the `--cd` argument for `wsl.exe`. All of the heavy lifting for parsing the URI is done by `PathCreateFromUrlW`. It just had to be plugged into the OSC 9;9 code. Closes #3158 ## Validation Steps Performed * Launch fish-shell in WSL * Duplicate tab works ✅ * (And because it doesn't work without using `IsValidDirectory` I know that OSC 7 works ✅) (cherry picked from commit 4ce79d7) Service-Card-Id: PVTI_lADOAF3p4s4BQX0-zgpBLe8 Service-Version: 1.25
|
I just updated to version |
|
@austin-beer Be more specific when reporting issues please. OSC 9;9 + Duplicate Tab works as expected for me. |
|
Unfortunately I was also unable to repro any breakage here! There might be some additional conditions we aren't aware of 😅 |
|
The only difference for OSC 9;9 is that we now check whether the target directory truly is accessible as a directory before spawning a process there. If somebody was smuggling Linux paths in via OSC 9;9, it may have worked previously, but it would have been out of spec. |
|
Chatting with Leonard a bit more, we have some ideas on how to make this less terrible for everyone. |
|
Okay, yes, I am providing Linux paths, not Windows paths, so that's why it's now broken for me. I need to change my command prompt to use the documented approach that converts to Windows paths first, correct? |
|
That'll fix it for now while we evaluate how best to fix it for everyone. Thanks for using Preview! :) |

This adds support for OSC 7 which is the same as OSC 9;9 but using
file URIs. As per the previous discussion in #8214, the problem is
that WSL shells emit URIs that can't work because the hostname in
the URI translates to an UNC path with an non-existing hostname.
In my opinion this doesn't deter the fact though that OSC 7 works
just fine for a native Windows application.
In the future we should consider trying to detect WSL file URIs
and translating it to the
--cdargument forwsl.exe.All of the heavy lifting for parsing the URI is done by
PathCreateFromUrlW. It just had to be plugged into the OSC 9;9 code.Closes #3158
Validation Steps Performed
IsValidDirectoryI know that OSC 7 works ✅)