From c81376f5bc1a4f444f140341c0faffd057c80587 Mon Sep 17 00:00:00 2001 From: Adam Lederer Date: Tue, 27 Aug 2024 00:59:16 -0700 Subject: [PATCH 1/5] in command palette, sort matches by quality/exactness --- src/gui/commandPalette.cpp | 115 +++++++++++++++++++++++++++++-------- 1 file changed, 92 insertions(+), 23 deletions(-) diff --git a/src/gui/commandPalette.cpp b/src/gui/commandPalette.cpp index e4cca5bf9..82cde0be8 100644 --- a/src/gui/commandPalette.cpp +++ b/src/gui/commandPalette.cpp @@ -26,17 +26,78 @@ #include #include "../ta-log.h" -static inline bool matchFuzzy(const char* haystack, const char* needle) { +// @TODO: when there's a tie on both within and before-needle costs, we have options. +// It's reasonable to let the original order stand, but also reasonable to favor +// minimizing the number of chars that follow afterward. Leaving this code in for now, +// but disabling until further thought/discussion. +// #define MATCH_SCORE_PREFER_LOWER_CHARS_AFTER_NEEDLE + +// negative is failure (doesn't contain needle), 0 is perfect (exactly matches needle), +struct MatchScore { + bool valid=true; + enum Cost { COST_BEFORE_NEEDLE, COST_WITHIN_NEEDLE, COST_AFTER_NEEDLE, COST_COUNT }; + size_t costs[COST_COUNT] = {0, 0, 0}; + + static MatchScore INVALID() { + MatchScore score; + score.valid=false; + return score; + } + + static bool IsFirstPreferable(const MatchScore& a, const MatchScore& b) { + if(1) return false; + auto PreferenceForAAmount=[&](Cost cost) { + // prefer a if lower cost + return b.costs[cost]-a.costs[cost]; + }; + + if (a.valid && b.valid) { + int prefA; + prefA=PreferenceForAAmount(COST_WITHIN_NEEDLE); + if (prefA!=0) return prefA>0; + prefA=PreferenceForAAmount(COST_BEFORE_NEEDLE); + if (prefA!=0) return prefA>0; +#ifdef MATCH_SCORE_PREFER_LOWER_CHARS_AFTER_NEEDLE + // prefA=PreferenceForAAmount(COST_AFTER_NEEDLE); + // if (prefA!=0) return prefA>0; +#endif + return false; + } else { + return a.valid; + } + } +}; + +static inline MatchScore matchFuzzy(const char* haystack, const char* needle) { + MatchScore score; size_t h_i=0; // haystack idx size_t n_i=0; // needle idx while (needle[n_i]!='\0') { - for (; std::tolower(haystack[h_i])!=std::tolower(needle[n_i]); h_i++) { + size_t cost=0; + for (; std::tolower(haystack[h_i])!=std::tolower(needle[n_i]); h_i++, cost++) { + // needle not completed, return invalid if (haystack[h_i]=='\0') - return false; + return MatchScore::INVALID(); } + + // contribute this run of non-matches toward pre-needle or within-needle cost + if (n_i==0) { + score.costs[MatchScore::COST_BEFORE_NEEDLE]=cost; + } else { + score.costs[MatchScore::COST_WITHIN_NEEDLE]+=cost; + } + n_i+=1; } - return true; + +#ifdef MATCH_SCORE_PREFER_LOWER_CHARS_AFTER_NEEDLE + // count the remaining chars in haystack as a tie-breaker (we won't reach this if it's a failed + // match anyway) + for (; haystack[h_i]!='\0'; h_i++, score.costs[MatchScore::COST_AFTER_NEEDLE]++) {} +#endif + + score.valid=true; + return score; } void FurnaceGUI::drawPalette() { @@ -69,43 +130,42 @@ void FurnaceGUI::drawPalette() { if (ImGui::InputTextWithHint("##CommandPaletteSearch",hint,&paletteQuery) || paletteFirstFrame) { paletteSearchResults.clear(); + std::vector matchScores; + + auto Evaluate=[&](int i, const char* name) { + MatchScore score=matchFuzzy(name,paletteQuery.c_str()); + if (score.valid) { + paletteSearchResults.push_back(i); + matchScores.push_back(score); + } + }; switch (curPaletteType) { case CMDPAL_TYPE_MAIN: for (int i=0; isong.insLen; i++) { String s=fmt::sprintf("%02X: %s", i, e->song.ins[i]->name.c_str()); - if (matchFuzzy(s.c_str(),paletteQuery.c_str())) { - paletteSearchResults.push_back(i+1); // because over here ins=0 is 'None' - } + Evaluate(i+1,s.c_str()); // because over here ins=0 is 'None' } break; case CMDPAL_TYPE_SAMPLES: for (int i=0; isong.sampleLen; i++) { - if (matchFuzzy(e->song.sample[i]->name.c_str(),paletteQuery.c_str())) { - paletteSearchResults.push_back(i); - } + Evaluate(i,e->song.sample[i]->name.c_str()); } break; @@ -113,9 +173,7 @@ void FurnaceGUI::drawPalette() { for (int i=0; availableSystems[i]; i++) { int ds=availableSystems[i]; const char* sysname=getSystemName((DivSystem)ds); - if (matchFuzzy(sysname,paletteQuery.c_str())) { - paletteSearchResults.push_back(ds); - } + Evaluate(ds,sysname); } break; @@ -124,6 +182,17 @@ void FurnaceGUI::drawPalette() { ImGui::CloseCurrentPopup(); break; }; + + // sort indices by match quality + std::vector sortingIndices(paletteSearchResults.size()); + for (size_t i=0; i Date: Tue, 27 Aug 2024 01:20:58 -0700 Subject: [PATCH 2/5] oh my god i left it disabled from when i was capturing screens --- src/gui/commandPalette.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/gui/commandPalette.cpp b/src/gui/commandPalette.cpp index 82cde0be8..c528ebf3e 100644 --- a/src/gui/commandPalette.cpp +++ b/src/gui/commandPalette.cpp @@ -32,7 +32,6 @@ // but disabling until further thought/discussion. // #define MATCH_SCORE_PREFER_LOWER_CHARS_AFTER_NEEDLE -// negative is failure (doesn't contain needle), 0 is perfect (exactly matches needle), struct MatchScore { bool valid=true; enum Cost { COST_BEFORE_NEEDLE, COST_WITHIN_NEEDLE, COST_AFTER_NEEDLE, COST_COUNT }; @@ -45,7 +44,6 @@ struct MatchScore { } static bool IsFirstPreferable(const MatchScore& a, const MatchScore& b) { - if(1) return false; auto PreferenceForAAmount=[&](Cost cost) { // prefer a if lower cost return b.costs[cost]-a.costs[cost]; From e44a6561c3522abb3a35532fa5001ce7296a29f2 Mon Sep 17 00:00:00 2001 From: Adam Lederer Date: Thu, 29 Aug 2024 19:08:12 -0700 Subject: [PATCH 3/5] use non-greedy search to find optimal fuzzy match order for command palette --- src/gui/commandPalette.cpp | 195 +++++++++++++++++++++++-------------- src/gui/gui.h | 3 +- 2 files changed, 123 insertions(+), 75 deletions(-) diff --git a/src/gui/commandPalette.cpp b/src/gui/commandPalette.cpp index c528ebf3e..804ff29cc 100644 --- a/src/gui/commandPalette.cpp +++ b/src/gui/commandPalette.cpp @@ -26,76 +26,117 @@ #include #include "../ta-log.h" -// @TODO: when there's a tie on both within and before-needle costs, we have options. -// It's reasonable to let the original order stand, but also reasonable to favor -// minimizing the number of chars that follow afterward. Leaving this code in for now, -// but disabling until further thought/discussion. -// #define MATCH_SCORE_PREFER_LOWER_CHARS_AFTER_NEEDLE - struct MatchScore { - bool valid=true; - enum Cost { COST_BEFORE_NEEDLE, COST_WITHIN_NEEDLE, COST_AFTER_NEEDLE, COST_COUNT }; - size_t costs[COST_COUNT] = {0, 0, 0}; - - static MatchScore INVALID() { - MatchScore score; - score.valid=false; - return score; - } - + size_t charsBeforeNeedle=0; + size_t charsWithinNeedle=0; static bool IsFirstPreferable(const MatchScore& a, const MatchScore& b) { - auto PreferenceForAAmount=[&](Cost cost) { - // prefer a if lower cost - return b.costs[cost]-a.costs[cost]; - }; - - if (a.valid && b.valid) { - int prefA; - prefA=PreferenceForAAmount(COST_WITHIN_NEEDLE); - if (prefA!=0) return prefA>0; - prefA=PreferenceForAAmount(COST_BEFORE_NEEDLE); - if (prefA!=0) return prefA>0; -#ifdef MATCH_SCORE_PREFER_LOWER_CHARS_AFTER_NEEDLE - // prefA=PreferenceForAAmount(COST_AFTER_NEEDLE); - // if (prefA!=0) return prefA>0; -#endif - return false; - } else { - return a.valid; - } + int aBetter; + aBetter=b.charsWithinNeedle-a.charsWithinNeedle; + if (aBetter!=0) return aBetter>0; + aBetter=b.charsBeforeNeedle-a.charsBeforeNeedle; + if (aBetter!=0) return aBetter>0; + return false; } }; -static inline MatchScore matchFuzzy(const char* haystack, const char* needle) { +struct MatchResult { MatchScore score; - size_t h_i=0; // haystack idx - size_t n_i=0; // needle idx - while (needle[n_i]!='\0') { - size_t cost=0; - for (; std::tolower(haystack[h_i])!=std::tolower(needle[n_i]); h_i++, cost++) { - // needle not completed, return invalid - if (haystack[h_i]=='\0') - return MatchScore::INVALID(); - } + std::vector highlightChars; +}; - // contribute this run of non-matches toward pre-needle or within-needle cost - if (n_i==0) { - score.costs[MatchScore::COST_BEFORE_NEEDLE]=cost; - } else { - score.costs[MatchScore::COST_WITHIN_NEEDLE]+=cost; - } +static bool charMatch(const char* a, const char* b) { + // stub for future utf8 support, possibly with matching for related chars? + return std::tolower(*a)==std::tolower(*b); +} - n_i+=1; +// #define MATCH_GREEDY + +static bool matchFuzzy(const char* haystack, int haystackLen, const char* needle, int needleLen, MatchResult* result) { + if (needleLen==0) { + result->score.charsBeforeNeedle=0; + result->score.charsWithinNeedle=0; + result->highlightChars.clear(); + return true; } -#ifdef MATCH_SCORE_PREFER_LOWER_CHARS_AFTER_NEEDLE - // count the remaining chars in haystack as a tie-breaker (we won't reach this if it's a failed - // match anyway) - for (; haystack[h_i]!='\0'; h_i++, score.costs[MatchScore::COST_AFTER_NEEDLE]++) {} + std::vector matchPool(needleLen+1); + std::vector unusedMatches(needleLen+1); + std::vector matchesByLen(needleLen+1); + for (int i=0; i=0; matchLen--) { + MatchResult*& m=matchesByLen[matchLen]; + + // ignore null matches except for 0 + if (matchLen>0 && !m) continue; + +#ifdef MATCH_GREEDY + // in greedy mode, don't start any new matches once we've already started matching. + // this will still return the correct bool result, but its score could be much poorer + // than the optimal match. consider the case: + // + // find "gl" in "google" + // + // greedy will see the match "g...l.", which has charsWithinNeedle of 3, while the + // fully algorithm will find the tighter match "...gl.", which has + // charsWithinNeedle of 0 + + if (matchLen==0 && unusedMatches.size() < matchPool.size()) { + continue; + } #endif - score.valid=true; - return score; + // check match! + if (charMatch(haystack+hIdx, needle+matchLen)) { + + // pull a fresh match from the pool if necessary + if (matchLen==0) { + m=unusedMatches.back(); + unusedMatches.pop_back(); + m->score.charsBeforeNeedle=hIdx; + m->score.charsWithinNeedle=0; + m->highlightChars.clear(); + } + + m->highlightChars.push_back(hIdx); + + // advance, replacing the previous match of an equal len, which can only have been + // worse because it existed before us, so we can prune it out + if (matchesByLen[matchLen+1]) { + unusedMatches.push_back(matchesByLen[matchLen+1]); + } + + matchesByLen[matchLen+1]=m; + m=NULL; + + } else { + // tally up charsWithinNeedle + if (matchLen>0) { + matchesByLen[matchLen]->score.charsWithinNeedle++; + } + } + } + } + + if (matchesByLen[needleLen]) { + if (result) *result=*matchesByLen[needleLen]; + return true; + } + + return false; +} + +static void matchFuzzyTest() { + String hay="a__i_a_i__o"; + String needle="aio"; + MatchResult match; + matchFuzzy(hay.c_str(), hay.length(), needle.c_str(), needle.length(), &match); + logI( "match.score.charsWithinNeedle: %d", match.score.charsWithinNeedle ); } void FurnaceGUI::drawPalette() { @@ -126,15 +167,19 @@ void FurnaceGUI::drawPalette() { break; } + // matchFuzzyTest(); + if (ImGui::InputTextWithHint("##CommandPaletteSearch",hint,&paletteQuery) || paletteFirstFrame) { paletteSearchResults.clear(); std::vector matchScores; - auto Evaluate=[&](int i, const char* name) { - MatchScore score=matchFuzzy(name,paletteQuery.c_str()); - if (score.valid) { - paletteSearchResults.push_back(i); - matchScores.push_back(score); + auto Evaluate=[&](int i, const char* name, int nameLen) { + MatchResult result; + if (matchFuzzy(name, nameLen, paletteQuery.c_str(), paletteQuery.length(), &result)) { + paletteSearchResults.emplace_back(); + paletteSearchResults.back().id=i; + paletteSearchResults.back().highlightChars=std::move(result.highlightChars); + matchScores.push_back(result.score); } }; @@ -142,28 +187,30 @@ void FurnaceGUI::drawPalette() { case CMDPAL_TYPE_MAIN: for (int i=0; isong.insLen; i++) { String s=fmt::sprintf("%02X: %s", i, e->song.ins[i]->name.c_str()); - Evaluate(i+1,s.c_str()); // because over here ins=0 is 'None' + Evaluate(i+1,s.c_str(),s.length()); // because over here ins=0 is 'None' } break; + } case CMDPAL_TYPE_SAMPLES: for (int i=0; isong.sampleLen; i++) { - Evaluate(i,e->song.sample[i]->name.c_str()); + Evaluate(i,e->song.sample[i]->name.c_str(),e->song.sample[i]->name.length()); } break; @@ -171,7 +218,7 @@ void FurnaceGUI::drawPalette() { for (int i=0; availableSystems[i]; i++) { int ds=availableSystems[i]; const char* sysname=getSystemName((DivSystem)ds); - Evaluate(ds,sysname); + Evaluate(ds,sysname,strlen(sysname)); } break; @@ -189,8 +236,8 @@ void FurnaceGUI::drawPalette() { }); // update paletteSearchResults from sorted indices (taking care not to stomp while we iterate - for (size_t i=0; i paletteSearchResultsCopy=paletteSearchResults; + for (size_t i=0; i0) { - int i=paletteSearchResults[curPaletteChoice]; + int i=paletteSearchResults[curPaletteChoice].id; switch (curPaletteType) { case CMDPAL_TYPE_MAIN: doAction(i); diff --git a/src/gui/gui.h b/src/gui/gui.h index 4246a1982..31ec1f9f1 100644 --- a/src/gui/gui.h +++ b/src/gui/gui.h @@ -1604,10 +1604,11 @@ class FurnaceGUI { String mmlStringSNES[DIV_MAX_CHIPS]; String folderString; + struct PaletteSearchResult { int id; std::vector highlightChars; }; std::vector sysSearchResults; std::vector> sampleBankSearchResults; std::vector newSongSearchResults; - std::vector paletteSearchResults; + std::vector paletteSearchResults; FixedQueue recentFile; std::vector makeInsTypeList; std::vector waveSizeList; From 29526d47fb6344a3bda5a75e91a754e195d1bf7c Mon Sep 17 00:00:00 2001 From: Adam Lederer Date: Thu, 29 Aug 2024 19:46:37 -0700 Subject: [PATCH 4/5] color the search characters in the command palette --- src/gui/commandPalette.cpp | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/gui/commandPalette.cpp b/src/gui/commandPalette.cpp index 804ff29cc..c031bb0de 100644 --- a/src/gui/commandPalette.cpp +++ b/src/gui/commandPalette.cpp @@ -50,6 +50,7 @@ static bool charMatch(const char* a, const char* b) { } // #define MATCH_GREEDY +// #define RUN_MATCH_TEST static bool matchFuzzy(const char* haystack, int haystackLen, const char* needle, int needleLen, MatchResult* result) { if (needleLen==0) { @@ -131,6 +132,7 @@ static bool matchFuzzy(const char* haystack, int haystackLen, const char* needle return false; } +#ifdef RUN_MATCH_TEST static void matchFuzzyTest() { String hay="a__i_a_i__o"; String needle="aio"; @@ -138,6 +140,7 @@ static void matchFuzzyTest() { matchFuzzy(hay.c_str(), hay.length(), needle.c_str(), needle.length(), &match); logI( "match.score.charsWithinNeedle: %d", match.score.charsWithinNeedle ); } +#endif void FurnaceGUI::drawPalette() { bool accepted=false; @@ -167,7 +170,9 @@ void FurnaceGUI::drawPalette() { break; } - // matchFuzzyTest(); +#ifdef RUN_MATCH_TEST + matchFuzzyTest(); +#endif if (ImGui::InputTextWithHint("##CommandPaletteSearch",hint,&paletteQuery) || paletteFirstFrame) { paletteSearchResults.clear(); @@ -294,10 +299,25 @@ void FurnaceGUI::drawPalette() { break; }; - if (ImGui::Selectable(s.c_str(),current)) { + ImGui::PushID(s.c_str()); + bool selectable=ImGui::Selectable("##paletteSearchItem",current,ImGuiSelectableFlags_SpanAllColumns|ImGuiSelectableFlags_AllowOverlap); + const char* str=s.c_str(); + size_t chCursor=0; + const std::vector& highlights=paletteSearchResults[i].highlightChars; + for (size_t ch=0; ch Date: Thu, 29 Aug 2024 20:08:29 -0700 Subject: [PATCH 5/5] command palette show shortcuts (for actions) --- src/gui/commandPalette.cpp | 146 +++++++++++++++++++++---------------- 1 file changed, 82 insertions(+), 64 deletions(-) diff --git a/src/gui/commandPalette.cpp b/src/gui/commandPalette.cpp index c031bb0de..868b5f1af 100644 --- a/src/gui/commandPalette.cpp +++ b/src/gui/commandPalette.cpp @@ -25,6 +25,7 @@ #include #include #include "../ta-log.h" +#include "util.h" struct MatchScore { size_t charsBeforeNeedle=0; @@ -249,76 +250,93 @@ void FurnaceGUI::drawPalette() { avail.y-=ImGui::GetFrameHeightWithSpacing(); if (ImGui::BeginChild("CommandPaletteList",avail,false,0)) { - bool navigated=false; - if (ImGui::IsKeyPressed(ImGuiKey_UpArrow) && curPaletteChoice>0) { - curPaletteChoice-=1; - navigated=true; - } - if (ImGui::IsKeyPressed(ImGuiKey_DownArrow)) { - curPaletteChoice+=1; - navigated=true; - } + bool navigated=false; + if (ImGui::IsKeyPressed(ImGuiKey_UpArrow) && curPaletteChoice>0) { + curPaletteChoice-=1; + navigated=true; + } + if (ImGui::IsKeyPressed(ImGuiKey_DownArrow)) { + curPaletteChoice+=1; + navigated=true; + } - if (paletteSearchResults.size()>0 && curPaletteChoice<0) { - curPaletteChoice=0; - navigated=true; - } - if (curPaletteChoice>=(int)paletteSearchResults.size()) { - curPaletteChoice=paletteSearchResults.size()-1; - navigated=true; - } + if (paletteSearchResults.size()>0 && curPaletteChoice<0) { + curPaletteChoice=0; + navigated=true; + } + if (curPaletteChoice>=(int)paletteSearchResults.size()) { + curPaletteChoice=paletteSearchResults.size()-1; + navigated=true; + } - for (int i=0; i<(int)paletteSearchResults.size(); i++) { - bool current=(i==curPaletteChoice); - int id=paletteSearchResults[i].id; + int columnCount=curPaletteType==CMDPAL_TYPE_MAIN ? 2 : 1; + if (ImGui::BeginTable("##commandPaletteTable",columnCount,ImGuiTableFlags_SizingStretchProp)) { + // ImGui::TableSetupColumn("##action",ImGuiTableColumnFlags_WidthStretch); + // ImGui::TableSetupColumn("##shortcut"); + for (int i=0; i<(int)paletteSearchResults.size(); i++) { + bool current=(i==curPaletteChoice); + int id=paletteSearchResults[i].id; - String s="???"; - switch (curPaletteType) { - case CMDPAL_TYPE_MAIN: - s=guiActions[id].friendlyName; - break; - case CMDPAL_TYPE_RECENT: - s=recentFile[id].c_str(); - break; - case CMDPAL_TYPE_INSTRUMENTS: - case CMDPAL_TYPE_INSTRUMENT_CHANGE: - if (id==0) { - s=_("- None -"); - } else { - s=fmt::sprintf("%02X: %s", id-1, e->song.ins[id-1]->name.c_str()); + String s="???"; + switch (curPaletteType) { + case CMDPAL_TYPE_MAIN: + s=guiActions[id].friendlyName; + break; + case CMDPAL_TYPE_RECENT: + s=recentFile[id].c_str(); + break; + case CMDPAL_TYPE_INSTRUMENTS: + case CMDPAL_TYPE_INSTRUMENT_CHANGE: + if (id==0) { + s=_("- None -"); + } else { + s=fmt::sprintf("%02X: %s", id-1, e->song.ins[id-1]->name.c_str()); + } + break; + case CMDPAL_TYPE_SAMPLES: + s=e->song.sample[id]->name.c_str(); + break; + case CMDPAL_TYPE_ADD_CHIP: + s=getSystemName((DivSystem)id); + break; + default: + logE(_("invalid command palette type")); + break; + }; + + ImGui::TableNextRow(); + ImGui::TableNextColumn(); + + ImGui::PushID(s.c_str()); + bool selectable=ImGui::Selectable("##paletteSearchItem",current,ImGuiSelectableFlags_SpanAllColumns|ImGuiSelectableFlags_AllowOverlap); + const char* str=s.c_str(); + size_t chCursor=0; + const std::vector& highlights=paletteSearchResults[i].highlightChars; + for (size_t ch=0; chsong.sample[id]->name.c_str(); - break; - case CMDPAL_TYPE_ADD_CHIP: - s=getSystemName((DivSystem)id); - break; - default: - logE(_("invalid command palette type")); - break; - }; + ImGui::SameLine(0.0f,0.0f); + ImGui::Text("%.*s", (int)(s.length()-chCursor), str+chCursor); - ImGui::PushID(s.c_str()); - bool selectable=ImGui::Selectable("##paletteSearchItem",current,ImGuiSelectableFlags_SpanAllColumns|ImGuiSelectableFlags_AllowOverlap); - const char* str=s.c_str(); - size_t chCursor=0; - const std::vector& highlights=paletteSearchResults[i].highlightChars; - for (size_t ch=0; ch