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

Add keybinding arg to openSettings #6299

Merged
5 commits merged into from
Jun 12, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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
27 changes: 27 additions & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,32 @@
}
]
},
"OpenSettingsAction": {
"description": "Arguments corresponding to a Open Settings Action",
"allOf": [
{
"$ref": "#/definitions/ShortcutAction"
},
{
"properties": {
"action": {
"type": "string",
"pattern": "openSettings"
},
"target": {
"type": "string",
"default": "settingsFile",
"description": "The settings file to open.",
"enum": [
"settingsFile",
"defaultsFile",
"allFiles"
]
}
}
}
]
},
"Keybinding": {
"additionalProperties": false,
"properties": {
Expand All @@ -245,6 +271,7 @@
{ "$ref": "#/definitions/MoveFocusAction" },
{ "$ref": "#/definitions/ResizePaneAction" },
{ "$ref": "#/definitions/SplitPaneAction" },
{ "$ref": "#/definitions/OpenSettingsAction" },
{ "type": "null" }
]
},
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/ActionArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@
#include "MoveFocusArgs.g.cpp"
#include "AdjustFontSizeArgs.g.cpp"
#include "SplitPaneArgs.g.cpp"
#include "OpenSettingsArgs.g.cpp"
60 changes: 60 additions & 0 deletions src/cascadia/TerminalApp/ActionArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "MoveFocusArgs.g.h"
#include "AdjustFontSizeArgs.g.h"
#include "SplitPaneArgs.g.h"
#include "OpenSettingsArgs.g.h"

#include "../../cascadia/inc/cppwinrt_utils.h"
#include "Utils.h"
Expand Down Expand Up @@ -381,6 +382,65 @@ namespace winrt::TerminalApp::implementation
return { *args, {} };
}
};

// Possible SettingsTarget values
// TODO:GH#2550/#3475 - move these to a centralized deserializing place
static constexpr std::string_view SettingsFileString{ "settingsFile" };
static constexpr std::string_view DefaultsFileString{ "defaultsFile" };
static constexpr std::string_view AllFilesString{ "allFiles" };

// Function Description:
// - Helper function for parsing a SettingsTarget from a string
// Arguments:
// - targetString: the string to attempt to parse
// Return Value:
// - The encoded SettingsTarget value, or SettingsTarget::SettingsFile if it was an invalid string
static TerminalApp::SettingsTarget ParseSettingsTarget(const std::string& targetString)
{
if (targetString == SettingsFileString)
{
return TerminalApp::SettingsTarget::SettingsFile;
}
else if (targetString == DefaultsFileString)
{
return TerminalApp::SettingsTarget::DefaultsFile;
}
else if (targetString == AllFilesString)
{
return TerminalApp::SettingsTarget::AllFiles;
}
// default behavior for invalid data
return TerminalApp::SettingsTarget::SettingsFile;
};

struct OpenSettingsArgs : public OpenSettingsArgsT<OpenSettingsArgs>
{
OpenSettingsArgs() = default;
GETSET_PROPERTY(TerminalApp::SettingsTarget, Target, TerminalApp::SettingsTarget::SettingsFile);

static constexpr std::string_view TargetKey{ "target" };

public:
bool Equals(const IActionArgs& other)
{
auto otherAsUs = other.try_as<OpenSettingsArgs>();
if (otherAsUs)
{
return otherAsUs->_Target == _Target;
}
return false;
};
static FromJsonResult FromJson(const Json::Value& json)
{
// LOAD BEARING: Not using make_self here _will_ break you in the future!
auto args = winrt::make_self<OpenSettingsArgs>();
if (auto targetString{ json[JsonKey(TargetKey)] })
{
args->_Target = ParseSettingsTarget(targetString.asString());
}
return { *args, {} };
}
};
}

namespace winrt::TerminalApp::factory_implementation
Expand Down
12 changes: 12 additions & 0 deletions src/cascadia/TerminalApp/ActionArgs.idl
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ namespace TerminalApp
Duplicate = 1
};

enum SettingsTarget
{
SettingsFile = 0,
DefaultsFile,
AllFiles
};

[default_interface] runtimeclass NewTerminalArgs {
NewTerminalArgs();
String Commandline;
Expand Down Expand Up @@ -90,4 +97,9 @@ namespace TerminalApp
NewTerminalArgs TerminalArgs { get; };
SplitType SplitMode { get; };
};

[default_interface] runtimeclass OpenSettingsArgs : IActionArgs
{
SettingsTarget Target { get; };
};
}
8 changes: 5 additions & 3 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,11 @@ namespace winrt::TerminalApp::implementation
void TerminalPage::_HandleOpenSettings(const IInspectable& /*sender*/,
const TerminalApp::ActionEventArgs& args)
{
// TODO:GH#2557 Add an optional arg for opening the defaults here
_LaunchSettings(false);
args.Handled(true);
if (const auto& realArgs = args.ActionArgs().try_as<TerminalApp::OpenSettingsArgs>())
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
{
_LaunchSettings(realArgs.Target());
args.Handled(true);
}
}

void TerminalPage::_HandlePasteText(const IInspectable& /*sender*/,
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ static const std::map<ShortcutAction, ParseActionFunction, std::less<>> argParse

{ ShortcutAction::SplitPane, winrt::TerminalApp::implementation::SplitPaneArgs::FromJson },

{ ShortcutAction::OpenSettings, winrt::TerminalApp::implementation::OpenSettingsArgs::FromJson },

{ ShortcutAction::Invalid, nullptr },
};

Expand Down
30 changes: 22 additions & 8 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,8 @@ namespace winrt::TerminalApp::implementation
const bool altPressed = WI_IsFlagSet(lAltState, CoreVirtualKeyStates::Down) ||
WI_IsFlagSet(rAltState, CoreVirtualKeyStates::Down);

_LaunchSettings(altPressed);
const auto target = altPressed ? SettingsTarget::DefaultsFile : SettingsTarget::SettingsFile;
_LaunchSettings(target);
}

// Method Description:
Expand Down Expand Up @@ -1532,21 +1533,34 @@ namespace winrt::TerminalApp::implementation
// - Called when the settings button is clicked. ShellExecutes the settings
// file, as to open it in the default editor for .json files. Does this in
// a background thread, as to not hang/crash the UI thread.
fire_and_forget TerminalPage::_LaunchSettings(const bool openDefaults)
fire_and_forget TerminalPage::_LaunchSettings(const SettingsTarget target)
{
// This will switch the execution of the function to a background (not
// UI) thread. This is IMPORTANT, because the Windows.Storage API's
// (used for retrieving the path to the file) will crash on the UI
// thread, because the main thread is a STA.
co_await winrt::resume_background();

const auto settingsPath = openDefaults ? CascadiaSettings::GetDefaultSettingsPath() :
CascadiaSettings::GetSettingsPath();
auto openFile = [](const auto filePath) {
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
HINSTANCE res = ShellExecute(nullptr, nullptr, filePath.c_str(), nullptr, nullptr, SW_SHOW);
if (static_cast<int>(reinterpret_cast<uintptr_t>(res)) <= 32)
{
ShellExecute(nullptr, nullptr, L"notepad", filePath.c_str(), nullptr, SW_SHOW);
}
};

HINSTANCE res = ShellExecute(nullptr, nullptr, settingsPath.c_str(), nullptr, nullptr, SW_SHOW);
if (static_cast<int>(reinterpret_cast<uintptr_t>(res)) <= 32)
{
ShellExecute(nullptr, nullptr, L"notepad", settingsPath.c_str(), nullptr, SW_SHOW);
switch (target)
{
case SettingsTarget::DefaultsFile:
openFile(CascadiaSettings::GetDefaultSettingsPath());
break;
case SettingsTarget::SettingsFile:
openFile(CascadiaSettings::GetSettingsPath());
break;
case SettingsTarget::AllFiles:
openFile(CascadiaSettings::GetDefaultSettingsPath());
openFile(CascadiaSettings::GetSettingsPath());
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
break;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ namespace winrt::TerminalApp::implementation
void _PasteText();
static fire_and_forget PasteFromClipboard(winrt::Microsoft::Terminal::TerminalControl::PasteFromClipboardEventArgs eventArgs);

fire_and_forget _LaunchSettings(const bool openDefaults);
fire_and_forget _LaunchSettings(const winrt::TerminalApp::SettingsTarget target);

void _OnTabClick(const IInspectable& sender, const Windows::UI::Xaml::Input::PointerRoutedEventArgs& eventArgs);
void _OnTabSelectionChanged(const IInspectable& sender, const Windows::UI::Xaml::Controls::SelectionChangedEventArgs& eventArgs);
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/defaults.json
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@
{ "command": "toggleFullscreen", "keys": "f11" },
{ "command": "openNewTabDropdown", "keys": "ctrl+shift+space" },
{ "command": "openSettings", "keys": "ctrl+," },
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regular openSettings command here was left untouched because when Settings UI comes out, we'll sneakily change the default behavior from target: settingsFile to target: UI.

{ "command": { "action": "openSettings", "target": "defaultFile" }, "keys": "ctrl+alt+," },
{ "command": "find", "keys": "ctrl+shift+f" },

// Tab Management
Expand Down