maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Use the "base" profile for incoming handoff and new commands (#11022)

Browse source 
This pull request introduces our first use of the "base" profile as an
actual profile. Incoming commandlines from `wt foo` *and* default
terminal handoffs will be hosted in the base profile.

**THIS IS A BREAKING CHANGE** for user behavior.

The original behavior where commandlines were hosted in the "default"
profile (in most cases, Windows PowerShell) led to user confusion: "why
does cmd use my powershell icon?" and "why does the title say
PowerShell?". Making this change unifies the user experience so that we
can land commandline detection in #10952.

Users who want the original behavior can get it back for commandline
invocation by specifying a profile using the `-p` argument, as in `wt -p
PowerShell -- cmd`.

As a temporary stopgap, users who attempt to duplicate the base profile
will get their specified default profile until we land #5047.

This feature is hidden behind the same feature flag that controls the
visibility of base/"Defaults" in the settings UI.

Fixes #10669
Related to #6776
This commit is contained in:
Dustin L. Howett 2021-08-25 15:41:42 -07:00 committed by GitHub
parent ee8800c739
commit ea58e4036b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 83 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp
View file

@ -65,7 +65,7 @@ namespace SettingsModelLocalTests
const std::string settingsJson{ R"(
{
"defaultProfile": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
"profiles": [
"profiles": { "list": [
{
"name": "profile0",
"guid": "{6239a42c-0000-49a3-80bd-e8fdd045185c}",
@ -84,6 +84,9 @@ namespace SettingsModelLocalTests
"commandline": "wsl.exe"
}
],
"defaults": {
"historySize": 29
} },
"keybindings": [
{ "keys": ["ctrl+a"], "command": { "action": "splitPane", "split": "vertical" } },
{ "keys": ["ctrl+b"], "command": { "action": "splitPane", "split": "vertical", "profile": "{6239a42c-1111-49a3-80bd-e8fdd045185c}" } },
@ -219,9 +222,18 @@ namespace SettingsModelLocalTests
const auto profile{ settings.GetProfileForArgs(realArgs.TerminalArgs()) };
const auto settingsStruct{ TerminalSettings::CreateWithNewTerminalArgs(settings, realArgs.TerminalArgs(), nullptr) };
const auto termSettings = settingsStruct.DefaultSettings();
VERIFY_ARE_EQUAL(guid0, profile.Guid());
if constexpr (Feature_ShowProfileDefaultsInSettings::IsEnabled())
{
// This action specified a command but no profile; it gets reassigned to the base profile
VERIFY_ARE_EQUAL(settings.ProfileDefaults(), profile);
VERIFY_ARE_EQUAL(29, termSettings.HistorySize());
}
else
{
VERIFY_ARE_EQUAL(guid0, profile.Guid());
VERIFY_ARE_EQUAL(1, termSettings.HistorySize());
}
VERIFY_ARE_EQUAL(L"foo.exe", termSettings.Commandline());
VERIFY_ARE_EQUAL(1, termSettings.HistorySize());
}
{
KeyChord kc{ true, false, false, false, static_cast<int32_t>('F'), 0 };

4
src/cascadia/TerminalApp/TabManagement.cpp
View file

@ -350,8 +350,10 @@ namespace winrt::TerminalApp::implementation
// current control's live settings (which will include changes
// made through VT).
if (const auto profile = tab.GetFocusedProfile())
if (auto profile = tab.GetFocusedProfile())
{
// TODO GH#5047 If we cache the NewTerminalArgs, we no longer need to do this.
profile = GetClosestProfileForDuplicationOfProfile(profile);
const auto settingsCreateResult{ TerminalSettings::CreateWithProfile(_settings, profile, *_bindings) };
const auto workingDirectory = tab.GetActiveTerminalControl().WorkingDirectory();
const auto validWorkingDirectory = !workingDirectory.empty();

50
src/cascadia/TerminalApp/TerminalPage.cpp
View file

@ -814,12 +814,6 @@ namespace winrt::TerminalApp::implementation
TerminalConnection::ITerminalConnection TerminalPage::_CreateConnectionFromSettings(Profile profile,
TerminalSettings settings)
{
if (!profile)
{
// Use the default profile if we didn't get one as an argument.
profile = _settings.FindProfile(_settings.GlobalSettings().DefaultProfile());
}
TerminalConnection::ITerminalConnection connection{ nullptr };
winrt::guid connectionType = profile.ConnectionType();
@ -1369,6 +1363,8 @@ namespace winrt::TerminalApp::implementation
profile = tab.GetFocusedProfile();
if (profile)
{
// TODO GH#5047 If we cache the NewTerminalArgs, we no longer need to do this.
profile = GetClosestProfileForDuplicationOfProfile(profile);
controlSettings = TerminalSettings::CreateWithProfile(_settings, profile, *_bindings);
const auto workingDirectory = tab.GetActiveTerminalControl().WorkingDirectory();
const auto validWorkingDirectory = !workingDirectory.empty();
@ -2534,13 +2530,28 @@ namespace winrt::TerminalApp::implementation
// and wait on it hence the locking mechanism.
if (Dispatcher().HasThreadAccess())
{
// TODO: GH 9458 will give us more context so we can try to choose a better profile.
auto hr = _OpenNewTab(nullptr, connection);
try
{
NewTerminalArgs newTerminalArgs{};
// TODO GH#10952: When we pass the actual commandline (or originating application), the
// settings model can choose the right settings based on command matching, or synthesize
// a profile from the registry/link settings (TODO GH#9458).
// TODO GH#9458: Get and pass the LNK/EXE filenames.
// Passing in a commandline forces GetProfileForArgs to use Base Layer instead of Default Profile;
// in the future, it can make a better decision based on the value we pull out of the process handle.
// TODO GH#5047: When we hang on to the N.T.A., try not to spawn "default... .exe" :)
newTerminalArgs.Commandline(L"default-terminal-invocation-placeholder");
const auto profile{ _settings.GetProfileForArgs(newTerminalArgs) };
const auto settings{ TerminalSettings::CreateWithProfile(_settings, profile, *_bindings) };
// Request a summon of this window to the foreground
_SummonWindowRequestedHandlers(*this, nullptr);
_CreateNewTabWithProfileAndSettings(profile, settings, connection);
return hr;
// Request a summon of this window to the foreground
_SummonWindowRequestedHandlers(*this, nullptr);
}
CATCH_RETURN();
return S_OK;
}
else
{
@ -2548,9 +2559,8 @@ namespace winrt::TerminalApp::implementation
HRESULT finalVal = S_OK;
Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [&]() {
finalVal = _OpenNewTab(nullptr, connection);
_SummonWindowRequestedHandlers(*this, nullptr);
// Re-running ourselves under the dispatcher will cause us to take the first branch above.
finalVal = _OnNewConnection(connection);
latch.count_down();
});
@ -3048,4 +3058,16 @@ namespace winrt::TerminalApp::implementation
{
return WindowName() == QuakeWindowName;
}
// Method Description:
// - This function stops people from duplicating the base profile, because
// it gets ~ ~ weird ~ ~ when they do. Remove when TODO GH#5047 is done.
Profile TerminalPage::GetClosestProfileForDuplicationOfProfile(const Profile& profile) const noexcept
{
if (profile == _settings.ProfileDefaults())
{
return _settings.FindProfile(_settings.GlobalSettings().DefaultProfile());
}
return profile;
}
}

2
src/cascadia/TerminalApp/TerminalPage.h
View file

@ -358,6 +358,8 @@ namespace winrt::TerminalApp::implementation
void _SetFocusMode(const bool inFocusMode);
winrt::Microsoft::Terminal::Settings::Model::Profile GetClosestProfileForDuplicationOfProfile(const winrt::Microsoft::Terminal::Settings::Model::Profile& profile) const noexcept;
#pragma region ActionHandlers
// These are all defined in AppActionHandlers.cpp
#define ON_ALL_ACTIONS(action) DECLARE_ACTION_HANDLER(action);

28
src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
View file

@ -59,6 +59,7 @@ CascadiaSettings::CascadiaSettings(winrt::hstring json) :
{
const auto jsonString{ til::u16u8(json) };
_ParseJsonString(jsonString, false);
_ApplyDefaultsFromUserSettings();
LayerJson(_userSettings);
_ValidateSettings();
}
@ -832,7 +833,32 @@ winrt::Microsoft::Terminal::Settings::Model::Profile CascadiaSettings::GetProfil
profileByName = _GetProfileGuidByName(newTerminalArgs.Profile());
}
return FindProfile(til::coalesce_value(profileByName, profileByIndex, _globals->DefaultProfile()));
if (profileByName)
{
return FindProfile(*profileByName);
}
if (profileByIndex)
{
return FindProfile(*profileByIndex);
}
if constexpr (Feature_ShowProfileDefaultsInSettings::IsEnabled())
{
// If the user has access to the "Defaults" profile, and no profile was otherwise specified,
// what we do is dependent on whether there was a commandline.
// If there was a commandline (case 1), we we'll launch in the "Defaults" profile.
// If there wasn't a commandline or there wasn't a NewTerminalArgs (case 2), we'll
// launch in the user's actual default profile.
// Case 2 above could be the result of a "nt" or "sp" invocation that doesn't specify anything.
// TODO GH#10952: Detect the profile based on the commandline (add matching support)
return (!newTerminalArgs || newTerminalArgs.Commandline().empty()) ?
FindProfile(GlobalSettings().DefaultProfile()) :
ProfileDefaults();
}
// For compatibility with the stable version's behavior, return the default by GUID in all other cases.
return FindProfile(GlobalSettings().DefaultProfile());
}
// Method Description: