Skip to content

Commit

Permalink
Add keybinding arg to openSettings (#6299)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
Adds the `target` keybinding arg to `openSettings`. Possible values include: `defaultsFile`, `settingsFile`, and `allFiles`.

## References
#5915 - mini-spec

## PR Checklist
* [x] Closes #2557 
* [x] Tests added/passed

## Detailed Description of the Pull Request / Additional comments
Implemented as discussed in the attached spec. A new enum will be added for the SettingsUI when it becomes available.

## Validation Steps Performed
Added the following to my settings.json:
```json
{ "command": "openSettings", "keys":... },
{ "command": { "action": "openSettings" }, "keys":... },
{ "command": { "action": "openSettings", "target": "settingsFile" }, "keys":... },
{ "command": { "action": "openSettings", "target": "defaultsFile" }, "keys":... },
{ "command": { "action": "openSettings", "target": "allFiles" }, "keys":... }
```
  • Loading branch information
carlos-zamora authored Jun 12, 2020
1 parent 25df527 commit 19fcbce
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 12 deletions.
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>())
{
_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 @@ -682,7 +682,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 @@ -1545,21 +1546,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) {
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());
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 @@ -151,7 +151,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 @@ -279,6 +279,7 @@
{ "command": "toggleFullscreen", "keys": "f11" },
{ "command": "openNewTabDropdown", "keys": "ctrl+shift+space" },
{ "command": "openSettings", "keys": "ctrl+," },
{ "command": { "action": "openSettings", "target": "defaultFile" }, "keys": "ctrl+alt+," },
{ "command": "find", "keys": "ctrl+shift+f" },

// Tab Management
Expand Down

0 comments on commit 19fcbce

Please sign in to comment.