diff --git a/libaegisub/common/mru.cpp b/libaegisub/common/mru.cpp index ac5520ce7..dd7cfde92 100644 --- a/libaegisub/common/mru.cpp +++ b/libaegisub/common/mru.cpp @@ -22,35 +22,63 @@ #include "libaegisub/option.h" #include "libaegisub/option_value.h" +namespace { +const char *mru_names[] = { + "Audio", + "Find", + "Keyframes", + "Replace", + "Subtitle", + "Timecodes", + "Video", +}; + +const char *option_names[] = { + "Limits/MRU", + "Limits/Find Replace", + "Limits/MRU", + "Limits/Find Replace", + "Limits/MRU", + "Limits/MRU", + "Limits/MRU", +}; + +int mru_index(const char *key) { + int i; + switch (*key) { + case 'A': i = 0; break; + case 'F': i = 1; break; + case 'K': i = 2; break; + case 'R': i = 3; break; + case 'S': i = 4; break; + case 'T': i = 5; break; + case 'V': i = 6; break; + default: return -1; + } + return strcmp(key, mru_names[i]) == 0 ? i : -1; +} +} + namespace agi { MRUManager::MRUManager(agi::fs::path const& config, std::pair default_config, agi::Options *options) : config_name(config) , options(options) -, option_names({ - {"Audio", "Limits/MRU"}, - {"Find", "Limits/Find Replace"}, - {"Keyframes", "Limits/MRU"}, - {"Replace", "Limits/Find Replace"}, - {"Subtitle", "Limits/MRU"}, - {"Timecodes", "Limits/MRU"}, - {"Video", "Limits/MRU"}, -}) { LOG_D("agi/mru") << "Loading MRU List"; auto root = json_util::file(config, default_config); for (auto const& it : static_cast(root)) - Load(it.first, it.second); + Load(it.first.c_str(), it.second); } -MRUManager::MRUListMap &MRUManager::Find(std::string const& key) { - auto index = mru.find(key); - if (index == mru.end()) - throw MRUErrorInvalidKey("Invalid key value"); - return index->second; +MRUManager::MRUListMap &MRUManager::Find(const char *key) { + auto index = mru_index(key); + if (index == -1) + throw MRUError("Invalid key value"); + return mru[index]; } -void MRUManager::Add(std::string const& key, agi::fs::path const& entry) { +void MRUManager::Add(const char *key, agi::fs::path const& entry) { MRUListMap &map = Find(key); auto it = find(begin(map), end(map), entry); if (it == begin(map) && it != end(map)) @@ -65,20 +93,20 @@ void MRUManager::Add(std::string const& key, agi::fs::path const& entry) { Flush(); } -void MRUManager::Remove(std::string const& key, agi::fs::path const& entry) { +void MRUManager::Remove(const char *key, agi::fs::path const& entry) { auto& map = Find(key); map.erase(remove(begin(map), end(map), entry), end(map)); Flush(); } -const MRUManager::MRUListMap* MRUManager::Get(std::string const& key) { +const MRUManager::MRUListMap* MRUManager::Get(const char *key) { return &Find(key); } -agi::fs::path const& MRUManager::GetEntry(std::string const& key, const size_t entry) { +agi::fs::path const& MRUManager::GetEntry(const char *key, const size_t entry) { const auto map = Get(key); if (entry >= map->size()) - throw MRUErrorIndexOutOfRange("Requested element index is out of range."); + throw MRUError("Requested element index is out of range."); return *next(map->begin(), entry); } @@ -86,34 +114,38 @@ agi::fs::path const& MRUManager::GetEntry(std::string const& key, const size_t e void MRUManager::Flush() { json::Object out; - for (auto const& mru_map : mru) { - json::Array &array = out[mru_map.first]; - for (auto const& p : mru_map.second) + for (size_t i = 0; i < mru.size(); ++i) { + json::Array &array = out[mru_names[i]]; + for (auto const& p : mru[i]) array.push_back(p.string()); } agi::JsonWriter::Write(out, io::Save(config_name).Get()); } -void MRUManager::Prune(std::string const& key, MRUListMap& map) const { +void MRUManager::Prune(const char *key, MRUListMap& map) const { size_t limit = 16u; if (options) { - auto it = option_names.find(key); - if (it != option_names.end()) - limit = (size_t)options->Get(it->second)->GetInt(); + int idx = mru_index(key); + if (idx != -1) + limit = (size_t)options->Get(option_names[idx])->GetInt(); } map.resize(std::min(limit, map.size())); } -void MRUManager::Load(std::string const& key, const json::Array& array) { +void MRUManager::Load(const char *key, const json::Array& array) { + int idx = mru_index(key); + if (idx == -1) return; + try { - transform(begin(array), end(array), - back_inserter(mru[key]), [](std::string const& s) { return s; }); + mru[idx].reserve(array.size()); + for (std::string const& str : array) + mru[idx].push_back(str); } catch (json::Exception const&) { // Out of date MRU file; just discard the data and skip it } - Prune(key, mru[key]); + Prune(key, mru[idx]); } } diff --git a/libaegisub/include/libaegisub/mru.h b/libaegisub/include/libaegisub/mru.h index a2b24bd10..9ce3dd43f 100644 --- a/libaegisub/include/libaegisub/mru.h +++ b/libaegisub/include/libaegisub/mru.h @@ -12,8 +12,8 @@ // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +#include #include -#include #include #include @@ -28,8 +28,6 @@ namespace agi { class Options; DEFINE_EXCEPTION(MRUError, Exception); -DEFINE_EXCEPTION(MRUErrorInvalidKey, MRUError); -DEFINE_EXCEPTION(MRUErrorIndexOutOfRange, MRUError); /// @class MRUManager /// @brief Most Recently Used (MRU) list handling @@ -54,25 +52,25 @@ public: /// @brief Add entry to the list. /// @param key List name /// @param entry Entry to add - /// @exception MRUErrorInvalidKey thrown when an invalid key is used. - void Add(std::string const& key, agi::fs::path const& entry); + /// @exception MRUError thrown when an invalid key is used. + void Add(const char *key, agi::fs::path const& entry); /// @brief Remove entry from the list. /// @param key List name /// @param entry Entry to add - /// @exception MRUErrorInvalidKey thrown when an invalid key is used. - void Remove(std::string const& key, agi::fs::path const& entry); + /// @exception MRUError thrown when an invalid key is used. + void Remove(const char *key, agi::fs::path const& entry); /// @brief Return list /// @param key List name - /// @exception MRUErrorInvalidKey thrown when an invalid key is used. - const MRUListMap* Get(std::string const& key); + /// @exception MRUError thrown when an invalid key is used. + const MRUListMap* Get(const char *key); /// @brief Return A single entry in a list. /// @param key List name /// @param entry 0-base position of entry - /// @exception MRUErrorInvalidKey thrown when an invalid key is used. - agi::fs::path const& GetEntry(std::string const& key, const size_t entry); + /// @exception MRUError thrown when an invalid key is used. + agi::fs::path const& GetEntry(const char *key, const size_t entry); /// Write MRU lists to disk. void Flush(); @@ -84,25 +82,17 @@ private: /// User preferences object for maximum number of items to list agi::Options *const options; - /// @brief Map for MRUListMap values. - /// @param std::string Name - /// @param MRUListMap instance. - using MRUMap = std::map; - /// Internal MRUMap values. - MRUMap mru; - - /// Map from MRU name to option name - const std::map option_names; + std::array mru; /// @brief Load MRU Lists. /// @param key List name. /// @param array json::Array of values. - void Load(std::string const& key, ::json::Array const& array); + void Load(const char *key, ::json::Array const& array); /// @brief Prune MRUListMap to the desired length. /// This uses the user-set values for MRU list length. - void Prune(std::string const& key, MRUListMap& map) const; - MRUListMap &Find(std::string const& key); + void Prune(const char *key, MRUListMap& map) const; + MRUListMap &Find(const char *key); }; } // namespace agi diff --git a/src/compat.cpp b/src/compat.cpp index 7ea800e1d..1892eaa30 100644 --- a/src/compat.cpp +++ b/src/compat.cpp @@ -4,7 +4,7 @@ #include -wxArrayString lagi_MRU_wxAS(std::string const& list) { +wxArrayString lagi_MRU_wxAS(const char *list) { auto const& vec = *config::mru->Get(list); wxArrayString ret; ret.reserve(vec.size()); diff --git a/src/compat.h b/src/compat.h index 3d34e1e94..f17a7bc4c 100644 --- a/src/compat.h +++ b/src/compat.h @@ -16,4 +16,4 @@ wxArrayString to_wx(std::vector const& vec); inline agi::Color from_wx(wxColour color) { return agi::Color(color.Red(), color.Green(), color.Blue(), 255 - color.Alpha()); } inline std::string from_wx(wxString const& str) { return std::string(str.utf8_str()); } -wxArrayString lagi_MRU_wxAS(std::string const& list); +wxArrayString lagi_MRU_wxAS(const char *list); diff --git a/src/menu.cpp b/src/menu.cpp index 3f83154da..5b4cef151 100644 --- a/src/menu.cpp +++ b/src/menu.cpp @@ -84,7 +84,7 @@ public: } void Update() { - const auto mru = config::mru->Get(type); + const auto mru = config::mru->Get(type.c_str()); Resize(mru->size()); diff --git a/tests/setup.bat b/tests/setup.bat index 9886040e5..d2a6ec564 100644 --- a/tests/setup.bat +++ b/tests/setup.bat @@ -20,9 +20,9 @@ icacls data\dir_access_denied /deny %USERNAME%:F mkdir data\dir_read_only icacls data\dir_read_only /deny %USERNAME%:W -echo {"Valid" : ["Entry One", "Entry Two"]} > data/mru_ok.json +echo {"Video" : ["Entry One", "Entry Two"]} > data/mru_ok.json -echo {"Invalid" : [1, 3]} > data/mru_invalid.json +echo {"Video" : [1, 3]} > data/mru_invalid.json echo '1234567890' > data\ten_bytes echo '' > data\touch_mod_time diff --git a/tests/setup.sh b/tests/setup.sh index e41f595b2..240811f5c 100755 --- a/tests/setup.sh +++ b/tests/setup.sh @@ -23,9 +23,9 @@ chmod 000 data/dir_access_denied mkdir data/dir_read_only chmod 444 data/dir_read_only -echo '{"Valid" : ["Entry One", "Entry Two"]}' > data/mru_ok.json +echo '{"Video" : ["Entry One", "Entry Two"]}' > data/mru_ok.json -echo '{"Invalid" : [1, 3]}' > data/mru_invalid.json +echo '{"Video" : [1, 3]}' > data/mru_invalid.json printf %s '1234567890' > data/ten_bytes touch -r $0 data/touch_mod_time diff --git a/tests/tests/mru.cpp b/tests/tests/mru.cpp index 4aa0f0c34..017aa9c00 100644 --- a/tests/tests/mru.cpp +++ b/tests/tests/mru.cpp @@ -12,99 +12,129 @@ // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -#include #include +#include +#include +#include + #include -static const char default_mru[] = "{\"Valid\" : []}"; +static const char default_mru[] = "{\"Video\" : []}"; +static const char conf_ok[] = "data/mru_ok.json"; -class lagi_mru : public libagi { -protected: - std::string conf_ok; - - void SetUp() override { - conf_ok = "./data/mru_ok.json"; - } -}; - - -TEST_F(lagi_mru, load_from_file) { +TEST(lagi_mru, load_from_file) { ASSERT_NO_THROW(agi::MRUManager mru(conf_ok, default_mru)); agi::MRUManager mru(conf_ok, default_mru); - ASSERT_NO_THROW(mru.Get("Valid")); - ASSERT_EQ(2u, mru.Get("Valid")->size()); - auto entry = mru.Get("Valid")->begin(); + ASSERT_NO_THROW(mru.Get("Video")); + ASSERT_EQ(2u, mru.Get("Video")->size()); + auto entry = mru.Get("Video")->begin(); EXPECT_STREQ("Entry One", (*entry++).string().c_str()); EXPECT_STREQ("Entry Two", (*entry++).string().c_str()); - EXPECT_TRUE(mru.Get("Valid")->end() == entry); + EXPECT_TRUE(mru.Get("Video")->end() == entry); } -TEST_F(lagi_mru, load_from_default_string) { +TEST(lagi_mru, load_from_default_string) { agi::fs::Remove("data/mru_tmp"); agi::MRUManager mru("data/mru_tmp", default_mru); } -TEST_F(lagi_mru, load_from_invalid_file) { +TEST(lagi_mru, load_from_invalid_file) { agi::fs::Copy("data/mru_invalid.json", "data/mru_tmp"); agi::MRUManager mru("data/mru_tmp", default_mru); - EXPECT_TRUE(mru.Get("Invalid")->empty()); + EXPECT_TRUE(mru.Get("Video")->empty()); } -TEST_F(lagi_mru, add_entry) { +TEST(lagi_mru, add_entry) { agi::fs::Copy("data/mru_ok.json", "data/mru_tmp"); agi::MRUManager mru("data/mru_tmp", default_mru); - EXPECT_NO_THROW(mru.Add("Valid", "/path/to/file")); - EXPECT_STREQ("/path/to/file", mru.Get("Valid")->front().string().c_str()); + EXPECT_NO_THROW(mru.Add("Video", "/path/to/file")); + EXPECT_STREQ("/path/to/file", mru.Get("Video")->front().string().c_str()); } -TEST_F(lagi_mru, remove_entry) { +TEST(lagi_mru, remove_entry) { agi::fs::Copy("data/mru_ok.json", "data/mru_tmp"); agi::MRUManager mru("data/mru_tmp", default_mru); - EXPECT_NO_THROW(mru.Add("Valid", "/path/to/file")); - EXPECT_NO_THROW(mru.Remove("Valid", "/path/to/file")); - EXPECT_STRNE("/path/to/file", mru.Get("Valid")->front().string().c_str()); + EXPECT_NO_THROW(mru.Add("Video", "/path/to/file")); + EXPECT_NO_THROW(mru.Remove("Video", "/path/to/file")); + EXPECT_STRNE("/path/to/file", mru.Get("Video")->front().string().c_str()); } -TEST_F(lagi_mru, invalid_mru_key_throws) { +TEST(lagi_mru, invalid_mru_key_throws) { agi::fs::Copy("data/mru_ok.json", "data/mru_tmp"); agi::MRUManager mru("data/mru_tmp", default_mru); - EXPECT_THROW(mru.Add("Invalid", "/path/to/file"), agi::MRUErrorInvalidKey); - EXPECT_THROW(mru.Get("Invalid"), agi::MRUErrorInvalidKey); + EXPECT_THROW(mru.Add("Invalid", "/path/to/file"), agi::MRUError); + EXPECT_THROW(mru.Get("Invalid"), agi::MRUError); } -TEST_F(lagi_mru, valid_mru_key_doesnt_throw) { +TEST(lagi_mru, valid_mru_key_doesnt_throw) { agi::fs::Copy("data/mru_ok.json", "data/mru_tmp"); agi::MRUManager mru("data/mru_tmp", default_mru); - EXPECT_NO_THROW(mru.Add("Valid", "/path/to/file")); + EXPECT_NO_THROW(mru.Add("Video", "/path/to/file")); } -TEST_F(lagi_mru, adding_existing_moves_to_front) { +TEST(lagi_mru, adding_existing_moves_to_front) { agi::fs::Remove("data/mru_tmp"); agi::MRUManager mru("data/mru_tmp", default_mru); - EXPECT_NO_THROW(mru.Add("Valid", "/file/1")); - EXPECT_NO_THROW(mru.Add("Valid", "/file/2")); - EXPECT_NO_THROW(mru.Add("Valid", "/file/3")); - EXPECT_NO_THROW(mru.Add("Valid", "/file/1")); - EXPECT_NO_THROW(mru.Add("Valid", "/file/1")); - EXPECT_NO_THROW(mru.Add("Valid", "/file/3")); + EXPECT_NO_THROW(mru.Add("Video", "/file/1")); + EXPECT_NO_THROW(mru.Add("Video", "/file/2")); + EXPECT_NO_THROW(mru.Add("Video", "/file/3")); + EXPECT_NO_THROW(mru.Add("Video", "/file/1")); + EXPECT_NO_THROW(mru.Add("Video", "/file/1")); + EXPECT_NO_THROW(mru.Add("Video", "/file/3")); - EXPECT_STREQ("/file/3", mru.GetEntry("Valid", 0).string().c_str()); - EXPECT_STREQ("/file/1", mru.GetEntry("Valid", 1).string().c_str()); - EXPECT_STREQ("/file/2", mru.GetEntry("Valid", 2).string().c_str()); - EXPECT_THROW(mru.GetEntry("Valid", 3), agi::MRUErrorIndexOutOfRange); + EXPECT_STREQ("/file/3", mru.GetEntry("Video", 0).string().c_str()); + EXPECT_STREQ("/file/1", mru.GetEntry("Video", 1).string().c_str()); + EXPECT_STREQ("/file/2", mru.GetEntry("Video", 2).string().c_str()); + EXPECT_THROW(mru.GetEntry("Video", 3), agi::MRUError); +} + +TEST(lagi_mru, all_valid_keys_work) { + agi::fs::Remove("data/mru_tmp"); + agi::MRUManager mru("data/mru_tmp", default_mru); + EXPECT_NO_THROW(mru.Add("Audio", "/file")); + EXPECT_NO_THROW(mru.Add("Find", "/file")); + EXPECT_NO_THROW(mru.Add("Keyframes", "/file")); + EXPECT_NO_THROW(mru.Add("Replace", "/file")); + EXPECT_NO_THROW(mru.Add("Subtitle", "/file")); + EXPECT_NO_THROW(mru.Add("Timecodes", "/file")); + EXPECT_NO_THROW(mru.Add("Video", "/file")); +} + +TEST(lagi_mru, prune_works) { + agi::fs::Remove("data/mru_tmp"); + agi::MRUManager mru("data/mru_tmp", default_mru); + + for (int i = 0; i < 20; ++i) { + ASSERT_NO_THROW(mru.Add("Audio", std::to_string(i))); + } + + EXPECT_EQ(16, mru.Get("Audio")->size()); + EXPECT_EQ("19", mru.Get("Audio")->front()); +} + +TEST(lagi_mru, prune_obeys_option) { + static const char opt[] = R"raw({"Limits": {"MRU": 1}})raw"; + agi::Options options("", opt, agi::Options::FLUSH_SKIP); + + agi::fs::Remove("data/mru_tmp"); + agi::MRUManager mru("data/mru_tmp", {default_mru, sizeof(default_mru)}, &options); + + ASSERT_NO_THROW(mru.Add("Audio", "1")); + ASSERT_NO_THROW(mru.Add("Audio", "2")); + + EXPECT_EQ(1, mru.Get("Audio")->size()); } // Check to make sure an entry is really removed. This was fixed in // r4347, the entry was being removed from a copy of the map internally. -TEST_F(lagi_mru, MRUEntryRemove_r4347) { - +TEST(lagi_mru, MRUEntryRemove_r4347) { agi::MRUManager mru(conf_ok, default_mru); - EXPECT_NO_THROW(mru.Add("Valid", "/path/to/file")); - EXPECT_NO_THROW(mru.Remove("Valid", "/path/to/file")); + EXPECT_NO_THROW(mru.Add("Video", "/path/to/file")); + EXPECT_NO_THROW(mru.Remove("Video", "/path/to/file")); - const agi::MRUManager::MRUListMap *map_list = mru.Get("Valid"); + const agi::MRUManager::MRUListMap *map_list = mru.Get("Video"); auto i_lst = map_list->begin(); if ((i_lst != map_list->end()) && (*i_lst == "/path/to/file"))