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

General improvements in preparation for #16598 #16601

Merged
merged 1 commit into from
Feb 6, 2024
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
1 change: 1 addition & 0 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ chh
chshdng
CHT
Cic
CLASSSTRING
CLE
cleartype
CLICKACTIVE
Expand Down
7 changes: 1 addition & 6 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,7 @@ namespace winrt::TerminalApp::implementation
return appLogic->GetSettings();
}

AppLogic::AppLogic() :
_reloadState{ std::chrono::milliseconds(100), []() { ApplicationState::SharedInstance().Reload(); } }
AppLogic::AppLogic()
{
// For your own sanity, it's better to do setup outside the ctor.
// If you do any setup in the ctor that ends up throwing an exception,
Expand Down Expand Up @@ -327,10 +326,6 @@ namespace winrt::TerminalApp::implementation
{
_reloadSettings->Run();
}
else if (ApplicationState::SharedInstance().IsStatePath(modifiedBasename))
{
_reloadState();
}
});
}

Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ namespace winrt::TerminalApp::implementation
::TerminalApp::AppCommandlineArgs _settingsAppArgs;

std::shared_ptr<ThrottledFuncTrailing<>> _reloadSettings;
til::throttled_func_trailing<> _reloadState;

std::vector<Microsoft::Terminal::Settings::Model::SettingsLoadWarnings> _warnings{};

Expand Down
10 changes: 10 additions & 0 deletions src/cascadia/TerminalApp/DebugTapConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ namespace winrt::Microsoft::TerminalApp::implementation
void TerminalOutput(const winrt::event_token& token) noexcept { _wrappedConnection.TerminalOutput(token); };
winrt::event_token StateChanged(const TypedEventHandler<ITerminalConnection, IInspectable>& handler) { return _wrappedConnection.StateChanged(handler); };
void StateChanged(const winrt::event_token& token) noexcept { _wrappedConnection.StateChanged(token); };
winrt::guid SessionId() const noexcept { return {}; }
ConnectionState State() const noexcept { return _wrappedConnection.State(); }

private:
Expand Down Expand Up @@ -98,6 +99,15 @@ namespace winrt::Microsoft::TerminalApp::implementation
_wrappedConnection = nullptr;
}

guid DebugTapConnection::SessionId() const noexcept
{
if (const auto c = _wrappedConnection.get())
{
return c.SessionId();
}
return {};
}

ConnectionState DebugTapConnection::State() const noexcept
{
if (auto strongConnection{ _wrappedConnection.get() })
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/DebugTapConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ namespace winrt::Microsoft::TerminalApp::implementation
void WriteInput(const hstring& data);
void Resize(uint32_t rows, uint32_t columns);
void Close();

winrt::guid SessionId() const noexcept;
winrt::Microsoft::Terminal::TerminalConnection::ConnectionState State() const noexcept;

void SetInputTap(const Microsoft::Terminal::TerminalConnection::ITerminalConnection& inputTap);
Expand Down
65 changes: 29 additions & 36 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1210,7 +1210,7 @@ namespace winrt::TerminalApp::implementation
TerminalConnection::ITerminalConnection connection{ nullptr };

auto connectionType = profile.ConnectionType();
winrt::guid sessionGuid{};
Windows::Foundation::Collections::ValueSet valueSet;

if (connectionType == TerminalConnection::AzureConnection::ConnectionType() &&
TerminalConnection::AzureConnection::IsAzureConnectionAvailable())
Expand All @@ -1226,23 +1226,16 @@ namespace winrt::TerminalApp::implementation
connection = TerminalConnection::ConptyConnection{};
}

auto valueSet = TerminalConnection::ConptyConnection::CreateSettings(azBridgePath.native(),
L".",
L"Azure",
false,
L"",
nullptr,
settings.InitialRows(),
settings.InitialCols(),
winrt::guid(),
profile.Guid());

if constexpr (Feature_VtPassthroughMode::IsEnabled())
{
valueSet.Insert(L"passthroughMode", Windows::Foundation::PropertyValue::CreateBoolean(settings.VtPassthrough()));
}

connection.Initialize(valueSet);
valueSet = TerminalConnection::ConptyConnection::CreateSettings(azBridgePath.native(),
L".",
L"Azure",
false,
L"",
nullptr,
settings.InitialRows(),
settings.InitialCols(),
winrt::guid(),
profile.Guid());
}

else
Expand All @@ -1267,38 +1260,38 @@ namespace winrt::TerminalApp::implementation
// process until later, on another thread, after we've already
// restored the CWD to its original value.
auto newWorkingDirectory{ _evaluatePathForCwd(settings.StartingDirectory()) };
auto conhostConn = TerminalConnection::ConptyConnection();
auto valueSet = TerminalConnection::ConptyConnection::CreateSettings(settings.Commandline(),
newWorkingDirectory,
settings.StartingTitle(),
settings.ReloadEnvironmentVariables(),
_WindowProperties.VirtualEnvVars(),
environment,
settings.InitialRows(),
settings.InitialCols(),
winrt::guid(),
profile.Guid());

valueSet.Insert(L"passthroughMode", Windows::Foundation::PropertyValue::CreateBoolean(settings.VtPassthrough()));
connection = TerminalConnection::ConptyConnection{};
valueSet = TerminalConnection::ConptyConnection::CreateSettings(settings.Commandline(),
newWorkingDirectory,
settings.StartingTitle(),
settings.ReloadEnvironmentVariables(),
_WindowProperties.VirtualEnvVars(),
environment,
settings.InitialRows(),
settings.InitialCols(),
winrt::guid(),
profile.Guid());

if (inheritCursor)
{
valueSet.Insert(L"inheritCursor", Windows::Foundation::PropertyValue::CreateBoolean(true));
}
}

conhostConn.Initialize(valueSet);

sessionGuid = conhostConn.Guid();
connection = conhostConn;
if constexpr (Feature_VtPassthroughMode::IsEnabled())
{
valueSet.Insert(L"passthroughMode", Windows::Foundation::PropertyValue::CreateBoolean(settings.VtPassthrough()));
}

connection.Initialize(valueSet);

TraceLoggingWrite(
g_hTerminalAppProvider,
"ConnectionCreated",
TraceLoggingDescription("Event emitted upon the creation of a connection"),
TraceLoggingGuid(connectionType, "ConnectionTypeGuid", "The type of the connection"),
TraceLoggingGuid(profile.Guid(), "ProfileGuid", "The profile's GUID"),
TraceLoggingGuid(sessionGuid, "SessionGuid", "The WT_SESSION's GUID"),
TraceLoggingGuid(connection.SessionId(), "SessionGuid", "The WT_SESSION's GUID"),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));

Expand Down
10 changes: 8 additions & 2 deletions src/cascadia/TerminalConnection/AzureConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,14 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
{
if (settings)
{
_initialRows = gsl::narrow<til::CoordType>(winrt::unbox_value_or<uint32_t>(settings.TryLookup(L"initialRows").try_as<Windows::Foundation::IPropertyValue>(), _initialRows));
_initialCols = gsl::narrow<til::CoordType>(winrt::unbox_value_or<uint32_t>(settings.TryLookup(L"initialCols").try_as<Windows::Foundation::IPropertyValue>(), _initialCols));
_initialRows = unbox_prop_or<uint32_t>(settings, L"initialRows", _initialRows);
_initialCols = unbox_prop_or<uint32_t>(settings, L"initialCols", _initialCols);
_sessionId = unbox_prop_or<guid>(settings, L"sessionId", _sessionId);
}

if (_sessionId == guid{})
{
_sessionId = Utils::CreateGuid();
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalConnection/AzureConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
#include <mutex>
#include <condition_variable>

#include "ConnectionStateHolder.h"
#include "BaseTerminalConnection.h"
#include "AzureClient.h"

namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
{
struct AzureConnection : AzureConnectionT<AzureConnection>, ConnectionStateHolder<AzureConnection>
struct AzureConnection : AzureConnectionT<AzureConnection>, BaseTerminalConnection<AzureConnection>
{
static winrt::guid ConnectionType() noexcept;
static bool IsAzureConnectionAvailable() noexcept;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,28 @@
namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
{
template<typename T>
struct ConnectionStateHolder
struct BaseTerminalConnection
{
public:
ConnectionState State() const noexcept { return _connectionState; }
winrt::guid SessionId() const noexcept
{
return _sessionId;
}

ConnectionState State() const noexcept
{
return _connectionState;
}

TYPED_EVENT(StateChanged, ITerminalConnection, winrt::Windows::Foundation::IInspectable);

protected:
template<typename U>
U unbox_prop_or(const Windows::Foundation::Collections::ValueSet& blob, std::wstring_view key, U defaultValue)
{
return winrt::unbox_value_or<U>(blob.TryLookup(key).try_as<Windows::Foundation::IPropertyValue>(), defaultValue);
}

#pragma warning(push)
#pragma warning(disable : 26447) // Analyzer is still upset about noexcepts throwing even with function level try.
// Method Description:
Expand Down Expand Up @@ -86,6 +101,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
return _isStateOneOf(ConnectionState::Connected);
}

winrt::guid _sessionId{};

private:
std::atomic<ConnectionState> _connectionState{ ConnectionState::NotConnected };
mutable std::mutex _stateMutex;
Expand Down
40 changes: 11 additions & 29 deletions src/cascadia/TerminalConnection/ConptyConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,12 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
auto environment = _initialEnv;

{
// Convert connection Guid to string and ignore the enclosing '{}'.
auto wsGuid{ Utils::GuidToString(_guid) };
wsGuid.pop_back();

const auto guidSubStr = std::wstring_view{ wsGuid }.substr(1);

// Ensure every connection has the unique identifier in the environment.
environment.as_map().insert_or_assign(L"WT_SESSION", guidSubStr.data());
// Convert connection Guid to string and ignore the enclosing '{}'.
environment.as_map().insert_or_assign(L"WT_SESSION", Utils::GuidToPlainString(_sessionId));

// The profile Guid does include the enclosing '{}'
const auto profileGuid{ Utils::GuidToString(_profileGuid) };
environment.as_map().insert_or_assign(L"WT_PROFILE_ID", profileGuid.data());
environment.as_map().insert_or_assign(L"WT_PROFILE_ID", Utils::GuidToString(_profileGuid));

// WSLENV is a colon-delimited list of environment variables (+flags) that should appear inside WSL
// https://devblogs.microsoft.com/commandline/share-environment-vars-between-wsl-and-windows/
Expand Down Expand Up @@ -171,7 +165,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
g_hTerminalConnectionProvider,
"ConPtyConnected",
TraceLoggingDescription("Event emitted when ConPTY connection is started"),
TraceLoggingGuid(_guid, "SessionGuid", "The WT_SESSION's GUID"),
TraceLoggingGuid(_sessionId, "SessionGuid", "The WT_SESSION's GUID"),
TraceLoggingWideString(_clientName.c_str(), "Client", "The attached client process"),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
Expand All @@ -189,7 +183,6 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
TERMINAL_STARTUP_INFO startupInfo) :
_rows{ 25 },
_cols{ 80 },
_guid{ Utils::CreateGuid() },
_inPipe{ hIn },
_outPipe{ hOut }
{
Expand Down Expand Up @@ -249,12 +242,6 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
return vs;
}

template<typename T>
T unbox_prop_or(const Windows::Foundation::Collections::ValueSet& blob, std::wstring_view key, T defaultValue)
{
return winrt::unbox_value_or<T>(blob.TryLookup(key).try_as<Windows::Foundation::IPropertyValue>(), defaultValue);
}

void ConptyConnection::Initialize(const Windows::Foundation::Collections::ValueSet& settings)
{
if (settings)
Expand All @@ -268,7 +255,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
_startingTitle = unbox_prop_or<winrt::hstring>(settings, L"startingTitle", _startingTitle);
_rows = unbox_prop_or<uint32_t>(settings, L"initialRows", _rows);
_cols = unbox_prop_or<uint32_t>(settings, L"initialCols", _cols);
_guid = unbox_prop_or<winrt::guid>(settings, L"guid", _guid);
_sessionId = unbox_prop_or<winrt::guid>(settings, L"sessionId", _sessionId);
_environment = settings.TryLookup(L"environment").try_as<Windows::Foundation::Collections::ValueSet>();
if constexpr (Feature_VtPassthroughMode::IsEnabled())
{
Expand Down Expand Up @@ -299,17 +286,12 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
_initialEnv = til::env::from_current_environment();
}
}

if (_guid == guid{})
{
_guid = Utils::CreateGuid();
}
}
}

winrt::guid ConptyConnection::Guid() const noexcept
{
return _guid;
if (_sessionId == guid{})
{
_sessionId = Utils::CreateGuid();
}
}

winrt::hstring ConptyConnection::Commandline() const
Expand Down Expand Up @@ -382,7 +364,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
g_hTerminalConnectionProvider,
"ConPtyConnectedToDefterm",
TraceLoggingDescription("Event emitted when ConPTY connection is started, for a defterm session"),
TraceLoggingGuid(_guid, "SessionGuid", "The WT_SESSION's GUID"),
TraceLoggingGuid(_sessionId, "SessionGuid", "The WT_SESSION's GUID"),
TraceLoggingWideString(_clientName.c_str(), "Client", "The attached client process"),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
Expand Down Expand Up @@ -686,7 +668,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
TraceLoggingWrite(g_hTerminalConnectionProvider,
"ReceivedFirstByte",
TraceLoggingDescription("An event emitted when the connection receives the first byte"),
TraceLoggingGuid(_guid, "SessionGuid", "The WT_SESSION's GUID"),
TraceLoggingGuid(_sessionId, "SessionGuid", "The WT_SESSION's GUID"),
TraceLoggingFloat64(delta.count(), "Duration"),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance));
Expand Down
6 changes: 2 additions & 4 deletions src/cascadia/TerminalConnection/ConptyConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@
#pragma once

#include "ConptyConnection.g.h"
#include "ConnectionStateHolder.h"
#include "BaseTerminalConnection.h"

#include "ITerminalHandoff.h"
#include <til/env.h>

namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
{
struct ConptyConnection : ConptyConnectionT<ConptyConnection>, ConnectionStateHolder<ConptyConnection>
struct ConptyConnection : ConptyConnectionT<ConptyConnection>, BaseTerminalConnection<ConptyConnection>
{
ConptyConnection(const HANDLE hSig,
const HANDLE hIn,
Expand All @@ -36,7 +36,6 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation

void ReparentWindow(const uint64_t newParent);

winrt::guid Guid() const noexcept;
winrt::hstring Commandline() const;
winrt::hstring StartingTitle() const;
WORD ShowWindow() const noexcept;
Expand Down Expand Up @@ -77,7 +76,6 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
hstring _startingTitle{};
bool _initialVisibility{ true };
Windows::Foundation::Collections::ValueSet _environment{ nullptr };
guid _guid{}; // A unique session identifier for connected client
hstring _clientName{}; // The name of the process hosted by this ConPTY connection (as of launch).

bool _receivedFirstByte{ false };
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalConnection/ConptyConnection.idl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ namespace Microsoft.Terminal.TerminalConnection
[default_interface] runtimeclass ConptyConnection : ITerminalConnection
{
ConptyConnection();
Guid Guid { get; };
String Commandline { get; };
String StartingTitle { get; };
UInt16 ShowWindow { get; };
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalConnection/EchoConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation

void Initialize(const Windows::Foundation::Collections::ValueSet& /*settings*/) const noexcept {};

winrt::guid SessionId() const noexcept { return {}; }
ConnectionState State() const noexcept { return ConnectionState::Connected; }

WINRT_CALLBACK(TerminalOutput, TerminalOutputHandler);
Expand Down
Loading
Loading