Skip to content

Commit

Permalink
Support for writing and reading multiple shortcuts per action
Browse files Browse the repository at this point in the history
  • Loading branch information
@mikekazakov
mikekazakov committed Dec 26, 2024
1 parent 7943cf7 commit 847271e
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 20 deletions.
38 changes: 30 additions & 8 deletions Source/NimbleCommander/NimbleCommander/Core/ActionsShortcutsManager.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,14 +527,27 @@ static constexpr auto make_array_n(T &&value)
return;

m_ShortcutsOverrides.clear();
for( auto i = v.MemberBegin(), e = v.MemberEnd(); i != e; ++i ) {
if( i->name.GetType() == kStringType && i->value.GetType() == kStringType ) {
auto att = g_ActionToTag.find(std::string_view{i->name.GetString()});
if( att != g_ActionToTag.end() ) {
m_ShortcutsOverrides[att->second] = WithoutEmptyShortcuts(Shortcuts{Shortcut{i->value.GetString()}});
for( auto it = v.MemberBegin(), e = v.MemberEnd(); it != e; ++it ) {
if( it->name.GetType() != kStringType )
continue;

const auto att = g_ActionToTag.find(std::string_view{it->name.GetString()});
if( att == g_ActionToTag.end() )
continue;

if( it->value.GetType() == kStringType ) {
m_ShortcutsOverrides[att->second] = WithoutEmptyShortcuts(Shortcuts{Shortcut{it->value.GetString()}});
}
if( it->value.GetType() == kArrayType ) {
Shortcuts shortcuts;
const unsigned shortcuts_size = it->value.Size();
for( unsigned idx = 0; idx < shortcuts_size; ++idx ) {
const auto &shortcut = it->value[idx];
if( shortcut.IsString() )
shortcuts.push_back(Shortcut{shortcut.GetString()});
}
m_ShortcutsOverrides[att->second] = WithoutEmptyShortcuts(shortcuts);
}
// TODO: support for array values when multiple shortcuts are stored
}
}

Expand Down Expand Up @@ -681,15 +694,24 @@ static constexpr auto make_array_n(T &&value)
using namespace rapidjson;
nc::config::Value overrides{kObjectType};

// TODO: add support for storing multiple shortcuts
for( auto &i : g_ActionsTags ) {
auto scover = m_ShortcutsOverrides.find(i.second);
if( scover != m_ShortcutsOverrides.end() ) {
if( scover == m_ShortcutsOverrides.end() ) {
continue;
}
if( scover->second.size() < 2 ) {
const std::string shortcut = scover->second.empty() ? std::string{} : scover->second.front().ToPersString();
overrides.AddMember(nc::config::MakeStandaloneString(i.first),
nc::config::MakeStandaloneString(shortcut),
nc::config::g_CrtAllocator);
}
else {
nc::config::Value shortcuts{kArrayType};
for( const Shortcut &sc : scover->second ) {
shortcuts.PushBack(nc::config::MakeStandaloneString(sc.ToPersString()), nc::config::g_CrtAllocator);
}
overrides.AddMember(nc::config::MakeStandaloneString(i.first), shortcuts, nc::config::g_CrtAllocator);
}
}

m_Config.Set(g_OverridesConfigPath, overrides);
Expand Down
147 changes: 135 additions & 12 deletions Source/NimbleCommander/NimbleCommander/Tests/ActionsShortcutsManager_UT.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
#include <NimbleCommander/Core/ActionsShortcutsManager.h>
#include <Config/ConfigImpl.h>
#include <Config/NonPersistentOverwritesStorage.h>
#include <fmt/format.h>

using Catch::Matchers::UnorderedEquals;
using ASM = nc::core::ActionsShortcutsManager;
using AS = ASM::Shortcut;
using ASs = ASM::Shortcuts;
using nc::config::ConfigImpl;
using nc::config::NonPersistentOverwritesStorage;

#define PREFIX "nc::core::ActionsShortcutsManager "

Expand All @@ -29,7 +31,7 @@ TEST_CASE(PREFIX "ActionFromTag")

TEST_CASE(PREFIX "ShortCutFromAction")
{
nc::config::ConfigImpl config{g_EmptyConfigJSON, std::make_shared<nc::config::NonPersistentOverwritesStorage>("")};
ConfigImpl config{g_EmptyConfigJSON, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};

SECTION("Non-existent")
Expand Down Expand Up @@ -64,7 +66,7 @@ TEST_CASE(PREFIX "ShortCutFromAction")

TEST_CASE(PREFIX "ShortCutFromTag")
{
nc::config::ConfigImpl config{g_EmptyConfigJSON, std::make_shared<nc::config::NonPersistentOverwritesStorage>("")};
ConfigImpl config{g_EmptyConfigJSON, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};

REQUIRE(manager.ShortcutsFromTag(346'242) == std::nullopt);
Expand All @@ -75,7 +77,7 @@ TEST_CASE(PREFIX "ShortCutFromTag")

TEST_CASE(PREFIX "DefaultShortCutFromTag")
{
nc::config::ConfigImpl config{g_EmptyConfigJSON, std::make_shared<nc::config::NonPersistentOverwritesStorage>("")};
ConfigImpl config{g_EmptyConfigJSON, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};

REQUIRE(manager.DefaultShortcutsFromTag(346'242) == std::nullopt);
Expand All @@ -86,7 +88,7 @@ TEST_CASE(PREFIX "DefaultShortCutFromTag")

TEST_CASE(PREFIX "RevertToDefaults")
{
nc::config::ConfigImpl config{g_EmptyConfigJSON, std::make_shared<nc::config::NonPersistentOverwritesStorage>("")};
ConfigImpl config{g_EmptyConfigJSON, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};

REQUIRE(manager.SetShortcutOverride("menu.edit.copy", AS("⌘j")));
Expand All @@ -96,7 +98,7 @@ TEST_CASE(PREFIX "RevertToDefaults")

TEST_CASE(PREFIX "ActionTagsFromShortCut")
{
nc::config::ConfigImpl config{g_EmptyConfigJSON, std::make_shared<nc::config::NonPersistentOverwritesStorage>("")};
ConfigImpl config{g_EmptyConfigJSON, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};

SECTION("Non-existent shortcut")
Expand Down Expand Up @@ -164,8 +166,8 @@ TEST_CASE(PREFIX "ActionTagsFromShortCut")

TEST_CASE(PREFIX "FirstOfActionTagsFromShortCut")
{
nc::config::ConfigImpl config{g_EmptyConfigJSON, std::make_shared<nc::config::NonPersistentOverwritesStorage>("")};
ASM manager{config};
ConfigImpl config{g_EmptyConfigJSON, std::make_shared<NonPersistentOverwritesStorage>("")};
const ASM manager{config};
REQUIRE(manager.FirstOfActionTagsFromShortcut({}, AS("⌘1")) == std::nullopt);
REQUIRE(manager.FirstOfActionTagsFromShortcut(std::initializer_list<int>{346'242}, AS("⌘1")) == std::nullopt);
REQUIRE(manager.FirstOfActionTagsFromShortcut(
Expand All @@ -175,16 +177,13 @@ TEST_CASE(PREFIX "FirstOfActionTagsFromShortCut")
std::initializer_list<int>{ASM::TagFromAction("menu.go.quick_lists.parent_folders").value()},
AS("⌘1"),
"menu.") == ASM::TagFromAction("menu.go.quick_lists.parent_folders").value());

REQUIRE(manager.FirstOfActionTagsFromShortcut(
std::initializer_list<int>{ASM::TagFromAction("menu.go.quick_lists.parent_folders").value()},
AS("⌘1"),
"viewer.") == std::nullopt);

REQUIRE(manager.FirstOfActionTagsFromShortcut(
std::initializer_list<int>{ASM::TagFromAction("viewer.toggle_text").value()}, AS("⌘1")) ==
ASM::TagFromAction("viewer.toggle_text").value());

REQUIRE(manager.FirstOfActionTagsFromShortcut(
std::initializer_list<int>{ASM::TagFromAction("viewer.toggle_text").value()}, AS("⌘1"), "menu.") ==
std::nullopt);
Expand All @@ -193,4 +192,128 @@ TEST_CASE(PREFIX "FirstOfActionTagsFromShortCut")
ASM::TagFromAction("viewer.toggle_text").value());
}

// TODO: unit tests for overrides persistence. include both variants of overrides.
TEST_CASE(PREFIX "Configuration persistence")
{
SECTION("Loading from config - single empty override")
{
const auto json = R"({
"hotkeyOverrides_v1": {
"menu.edit.copy": ""
}
})";
ConfigImpl config{json, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};
REQUIRE(manager.ShortcutsFromAction("menu.edit.copy") == ASs{});
}
SECTION("Loading from config - single override")
{
const auto json = R"({
"hotkeyOverrides_v1": {
"menu.edit.copy": "⌘j"
}
})";
ConfigImpl config{json, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};
REQUIRE(manager.ShortcutsFromAction("menu.edit.copy") == ASs{AS("⌘j")});
}
SECTION("Loading from config - single empty array")
{
const auto json = R"({
"hotkeyOverrides_v1": {
"menu.edit.copy": []
}
})";
ConfigImpl config{json, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};
REQUIRE(manager.ShortcutsFromAction("menu.edit.copy") == ASs{});
}
SECTION("Loading from config - single array with one shortcut")
{
const auto json = R"({
"hotkeyOverrides_v1": {
"menu.edit.copy": ["⌘j"]
}
})";
ConfigImpl config{json, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};
REQUIRE(manager.ShortcutsFromAction("menu.edit.copy") == ASs{AS("⌘j")});
}
SECTION("Loading from config - single array with two shortcuts")
{
const auto json = R"({
"hotkeyOverrides_v1": {
"menu.edit.copy": ["⌘j", "⌘k"]
}
})";
ConfigImpl config{json, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};
REQUIRE(manager.ShortcutsFromAction("menu.edit.copy") == ASs{AS("⌘j"), AS("⌘k")});
}
SECTION("Loading from config - mixed usage")
{
const auto json = R"({
"hotkeyOverrides_v1": {
"menu.edit.copy": ["⌘j", "⌘k"],
"menu.window.zoom": "⇧^⌘⌥j"
}
})";
ConfigImpl config{json, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};
REQUIRE(manager.ShortcutsFromAction("menu.edit.copy") == ASs{AS("⌘j"), AS("⌘k")});
REQUIRE(manager.ShortcutsFromAction("menu.window.zoom") == ASs{AS("⇧^⌘⌥j")});
}
SECTION("Writing to config - single empty override")
{
ConfigImpl config{g_EmptyConfigJSON, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};
REQUIRE(manager.SetShortcutsOverride("menu.edit.copy", {}));
const auto expected_json = R"({
"hotkeyOverrides_v1": {
"menu.edit.copy": ""
}
})";
ConfigImpl expected_config{expected_json, std::make_shared<NonPersistentOverwritesStorage>("")};
REQUIRE(config.Get("hotkeyOverrides_v1") == expected_config.Get("hotkeyOverrides_v1"));
}
SECTION("Writing to config - single override")
{
ConfigImpl config{g_EmptyConfigJSON, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};
REQUIRE(manager.SetShortcutsOverride("menu.edit.copy", std::array{AS("⌘j")}));
const auto expected_json = R"({
"hotkeyOverrides_v1": {
"menu.edit.copy": "⌘j"
}
})";
ConfigImpl expected_config{expected_json, std::make_shared<NonPersistentOverwritesStorage>("")};
REQUIRE(config.Get("hotkeyOverrides_v1") == expected_config.Get("hotkeyOverrides_v1"));
}
SECTION("Writing to config - single override with two hotkeys ")
{
ConfigImpl config{g_EmptyConfigJSON, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};
REQUIRE(manager.SetShortcutsOverride("menu.edit.copy", std::array{AS("⌘j"), AS("⌘k")}));
const auto expected_json = R"({
"hotkeyOverrides_v1": {
"menu.edit.copy": ["⌘j", "⌘k"]
}
})";
ConfigImpl expected_config{expected_json, std::make_shared<NonPersistentOverwritesStorage>("")};
REQUIRE(config.Get("hotkeyOverrides_v1") == expected_config.Get("hotkeyOverrides_v1"));
}
SECTION("Writing to config - mixed usage")
{
ConfigImpl config{g_EmptyConfigJSON, std::make_shared<NonPersistentOverwritesStorage>("")};
ASM manager{config};
REQUIRE(manager.SetShortcutsOverride("menu.edit.copy", std::array{AS("⌘j"), AS("⌘k")}));
REQUIRE(manager.SetShortcutOverride("menu.window.zoom", AS("⇧^⌘⌥j")));
const auto expected_json = R"({
"hotkeyOverrides_v1": {
"menu.edit.copy": ["⌘j", "⌘k"],
"menu.window.zoom": "⇧^⌥⌘j"
}
})";
ConfigImpl expected_config{expected_json, std::make_shared<NonPersistentOverwritesStorage>("")};
REQUIRE(config.Get("hotkeyOverrides_v1") == expected_config.Get("hotkeyOverrides_v1"));
}
}

0 comments on commit 847271e

Please sign in to comment.