Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a "virtual CWD" for each terminal window #15280

Merged
merged 8 commits into from
May 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cascadia/Remoting/Peasant.idl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.Terminal.Remoting
CommandlineArgs(String[] args, String cwd, UInt32 showWindowCommand);

String[] Commandline { get; set; };
String CurrentDirectory();
String CurrentDirectory { get; };
UInt32 ShowWindowCommand { get; };
};

Expand Down
32 changes: 15 additions & 17 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -551,27 +551,28 @@ namespace winrt::TerminalApp::implementation
// Handle it on a subsequent pass of the UI thread.
co_await wil::resume_foreground(Dispatcher(), CoreDispatcherPriority::Normal);

// If the caller provided a CWD, switch to that directory, then switch
// If the caller provided a CWD, "switch" to that directory, then switch
// back once we're done. This looks weird though, because we have to set
// up the scope_exit _first_. We'll release the scope_exit if we don't
// actually need it.
auto originalCwd{ wil::GetCurrentDirectoryW<std::wstring>() };
auto restoreCwd = wil::scope_exit([&originalCwd]() {

auto originalVirtualCwd{ _WindowProperties.VirtualWorkingDirectory() };
auto restoreCwd = wil::scope_exit([&originalVirtualCwd, this]() {
// ignore errors, we'll just power on through. We'd rather do
// something rather than fail silently if the directory doesn't
// actually exist.
LOG_IF_WIN32_BOOL_FALSE(SetCurrentDirectory(originalCwd.c_str()));
_WindowProperties.VirtualWorkingDirectory(originalVirtualCwd);
});

if (cwd.empty())
{
// We didn't actually need to change the virtual CWD, so we don't
// need to restore it
restoreCwd.release();
}
else
{
// ignore errors, we'll just power on through. We'd rather do
// something rather than fail silently if the directory doesn't
// actually exist.
LOG_IF_WIN32_BOOL_FALSE(SetCurrentDirectory(cwd.c_str()));
_WindowProperties.VirtualWorkingDirectory(cwd);
}

if (auto page{ weakThis.get() })
Expand Down Expand Up @@ -1226,15 +1227,12 @@ namespace winrt::TerminalApp::implementation
// construction, because the connection might not spawn the child
// process until later, on another thread, after we've already
// restored the CWD to its original value.
auto newWorkingDirectory{ settings.StartingDirectory() };
if (newWorkingDirectory.size() == 0 || newWorkingDirectory.size() == 1 &&
!(newWorkingDirectory[0] == L'~' || newWorkingDirectory[0] == L'/'))
{ // We only want to resolve the new WD against the CWD if it doesn't look like a Linux path (see GH#592)
auto cwdString{ wil::GetCurrentDirectoryW<std::wstring>() };
std::filesystem::path cwd{ cwdString };
cwd /= settings.StartingDirectory().c_str();
newWorkingDirectory = winrt::hstring{ cwd.wstring() };
}
const auto currentVirtualDir{ _WindowProperties.VirtualWorkingDirectory() };
const auto cwdString{ std::wstring_view{ currentVirtualDir } };
const auto requestedStartingDir{ settings.StartingDirectory() };
auto newWorkingDirectory = winrt::hstring{
Utils::EvaluateStartingDirectory(cwdString, std::wstring_view{ requestedStartingDir })
};

auto conhostConn = TerminalConnection::ConptyConnection();
auto valueSet = TerminalConnection::ConptyConnection::CreateSettings(settings.Commandline(),
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.idl
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ namespace TerminalApp
String WindowNameForDisplay { get; };
String WindowIdForDisplay { get; };

String VirtualWorkingDirectory { get; set; };

Boolean IsQuakeWindow();
};

Expand Down
6 changes: 5 additions & 1 deletion src/cascadia/TerminalApp/TerminalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1019,11 +1019,14 @@ namespace winrt::TerminalApp::implementation
// returned.
// Arguments:
// - args: an array of strings to process as a commandline. These args can contain spaces
// - cwd: The CWD that this window should treat as its own "virtual" CWD
// Return Value:
// - the result of the first command who's parsing returned a non-zero code,
// or 0. (see TerminalWindow::_ParseArgs)
int32_t TerminalWindow::SetStartupCommandline(array_view<const winrt::hstring> args)
int32_t TerminalWindow::SetStartupCommandline(array_view<const winrt::hstring> args, winrt::hstring cwd)
{
_WindowProperties->SetInitialCwd(std::move(cwd));

// This is called in AppHost::ctor(), before we've created the window
// (or called TerminalWindow::Initialize)
const auto result = _appArgs.ParseArgs(args);
Expand Down Expand Up @@ -1345,6 +1348,7 @@ namespace winrt::TerminalApp::implementation
CATCH_LOG();
}
}

uint64_t WindowProperties::WindowId() const noexcept
{
return _WindowId;
Expand Down
8 changes: 7 additions & 1 deletion src/cascadia/TerminalApp/TerminalWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,14 @@ namespace winrt::TerminalApp::implementation
winrt::hstring WindowNameForDisplay() const noexcept;
bool IsQuakeWindow() const noexcept;

WINRT_OBSERVABLE_PROPERTY(winrt::hstring, VirtualWorkingDirectory, _PropertyChangedHandlers, L"");

WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);

public:
// Used for setting the initial CWD, before we have XAML set up for property change notifications.
void SetInitialCwd(winrt::hstring cwd) { _VirtualWorkingDirectory = std::move(cwd); };

private:
winrt::hstring _WindowName{};
uint64_t _WindowId{ 0 };
Expand All @@ -71,7 +77,7 @@ namespace winrt::TerminalApp::implementation

bool HasCommandlineArguments() const noexcept;

int32_t SetStartupCommandline(array_view<const winrt::hstring> actions);
int32_t SetStartupCommandline(array_view<const winrt::hstring> actions, winrt::hstring cwd);
void SetStartupContent(const winrt::hstring& content, const Windows::Foundation::IReference<Windows::Foundation::Rect>& contentBounds);
int32_t ExecuteCommandline(array_view<const winrt::hstring> actions, const winrt::hstring& cwd);
void SetSettingsStartupArgs(const std::vector<winrt::Microsoft::Terminal::Settings::Model::ActionAndArgs>& actions);
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalWindow.idl
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ namespace TerminalApp

Boolean HasCommandlineArguments();

Int32 SetStartupCommandline(String[] commands);
Int32 SetStartupCommandline(String[] commands, String cwd);
void SetStartupContent(String json, Windows.Foundation.IReference<Windows.Foundation.Rect> bounds);
Int32 ExecuteCommandline(String[] commands, String cwd);
String ParseCommandlineMessage { get; };
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ void AppHost::_HandleCommandlineArgs(const Remoting::WindowRequestedArgs& window
}
else if (args)
{
const auto result = _windowLogic.SetStartupCommandline(args.Commandline());
const auto result = _windowLogic.SetStartupCommandline(args.Commandline(), args.CurrentDirectory());
const auto message = _windowLogic.ParseCommandlineMessage();
if (!message.empty())
{
Expand Down
12 changes: 11 additions & 1 deletion src/cascadia/WindowsTerminal/WindowEmperor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,17 @@ bool WindowEmperor::HandleCommandlineArgs()
{
std::vector<winrt::hstring> args;
_buildArgsFromCommandline(args);
auto cwd{ wil::GetCurrentDirectoryW<std::wstring>() };
const auto cwd{ wil::GetCurrentDirectoryW<std::wstring>() };

{
// ALWAYS change the _real_ CWD of the Terminal to system32, so that we
// don't lock the directory we were spawned in.
std::wstring system32{};
if (SUCCEEDED_LOG(wil::GetSystemDirectoryW<std::wstring>(system32)))
{
LOG_IF_WIN32_BOOL_FALSE(SetCurrentDirectoryW(system32.c_str()));
}
}

// Get the requested initial state of the window from our startup info. For
// something like `start /min`, this will set the wShowWindow member to
Expand Down
3 changes: 3 additions & 0 deletions src/types/inc/utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,7 @@ namespace Microsoft::Console::Utils
// testing easier.
std::wstring_view TrimPaste(std::wstring_view textView) noexcept;

// Same deal, but in TerminalPage::_evaluatePathForCwd
std::wstring EvaluateStartingDirectory(std::wstring_view cwd, std::wstring_view startingDirectory);

}
64 changes: 64 additions & 0 deletions src/types/ut_types/UtilsTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class UtilsTests
TEST_METHOD(TestTrimTrailingWhitespace);
TEST_METHOD(TestDontTrimTrailingWhitespace);

TEST_METHOD(TestEvaluateStartingDirectory);

void _VerifyXTermColorResult(const std::wstring_view wstr, DWORD colorValue);
void _VerifyXTermColorInvalid(const std::wstring_view wstr);
};
Expand Down Expand Up @@ -546,3 +548,65 @@ void UtilsTests::TestDontTrimTrailingWhitespace()
// * trim when there's a tab followed by only whitespace
// * not trim then there's a tab in the middle, and the string ends in whitespace
}

void UtilsTests::TestEvaluateStartingDirectory()
{
// Continue on failures
const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope;

auto test = [](auto& expected, auto& cwd, auto& startingDir) {
VERIFY_ARE_EQUAL(expected, EvaluateStartingDirectory(cwd, startingDir));
};

// A NOTE: EvaluateStartingDirectory makes no attempt to cannonicalize the
// path. So if you do any sort of relative paths, it'll literally just
// append.

{
std::wstring cwd = L"C:\\Windows\\System32";

// Literally blank
test(L"C:\\Windows\\System32\\", cwd, L"");

// Absolute Windows path
test(L"C:\\Windows", cwd, L"C:\\Windows");
test(L"C:/Users/migrie", cwd, L"C:/Users/migrie");

// Relative Windows path
test(L"C:\\Windows\\System32\\.", cwd, L"."); // ?
test(L"C:\\Windows\\System32\\.\\System32", cwd, L".\\System32"); // ?
test(L"C:\\Windows\\System32\\./dev", cwd, L"./dev");

// WSL '~' path
test(L"~", cwd, L"~");
test(L"~/dev", cwd, L"~/dev");

// WSL or Windows / path - this will ultimately be evaluated by the connection
test(L"/", cwd, L"/");
test(L"/dev", cwd, L"/dev");
}

{
std::wstring cwd = L"C:/Users/migrie";

// Literally blank
test(L"C:/Users/migrie\\", cwd, L"");

// Absolute Windows path
test(L"C:\\Windows", cwd, L"C:\\Windows");
test(L"C:/Users/migrie", cwd, L"C:/Users/migrie");

// Relative Windows path
test(L"C:/Users/migrie\\.", cwd, L"."); // ?
test(L"C:/Users/migrie\\.\\System32", cwd, L".\\System32"); // ?
test(L"C:/Users/migrie\\./dev", cwd, L"./dev");

// WSL '~' path
test(L"~", cwd, L"~");
test(L"~/dev", cwd, L"~/dev");

// WSL or Windows / path - this will ultimately be evaluated by the connection
test(L"/", cwd, L"/");
test(L"/dev", cwd, L"/dev");
}
}
24 changes: 24 additions & 0 deletions src/types/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -828,3 +828,27 @@ std::wstring_view Utils::TrimPaste(std::wstring_view textView) noexcept

return textView.substr(0, lastNonSpace + 1);
}

std::wstring Utils::EvaluateStartingDirectory(
std::wstring_view currentDirectory,
std::wstring_view startingDirectory)
{
std::wstring resultPath{ startingDirectory };

// We only want to resolve the new WD against the CWD if it doesn't look
// like a Linux path (see GH#592)

// Append only if it DOESN'T look like a linux-y path. A linux-y path starts
// with `~` or `/`.
const bool looksLikeLinux =
resultPath.size() >= 1 &&
(til::at(resultPath, 0) == L'~' || til::at(resultPath, 0) == L'/');

if (!looksLikeLinux)
{
std::filesystem::path cwd{ currentDirectory };
cwd /= startingDirectory;
resultPath = cwd.wstring();
}
return resultPath;
}
4 changes: 4 additions & 0 deletions tools/bcz.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ if (%1) == (rel) (
echo Manually building release
set _LAST_BUILD_CONF=Release
)
if (%1) == (audit) (
echo Manually building audit mode
set _LAST_BUILD_CONF=AuditMode
)
if (%1) == (no_clean) (
set _MSBUILD_TARGET=Build
)
Expand Down