feat(search): keep 100 past queries instead of 30 - #822
Merged
Conversation
The Search tab's Name-field history held 30 entries, eMule's CCustomAutoComplete default. A history is only useful as far back as it reaches, and 30 queries is a short reach for anyone who searches often; clearing it has been a deliberate, confirmed action since amule-project#754, so there is no longer much cost to keeping more of it around. Raised to 100 on request (amule-project#755). The constant moves from an anonymous namespace in SearchDlg.cpp to SearchHistory.h, next to the function that consumes it. The test that claimed to lock "the constant CSearchDlg actually passes in" was hardcoding its own copy of the number, so it would have gone on passing while the GUI used something else entirely; it now reads the same symbol the GUI does and cannot drift from it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the first half of #755.
The Search tab's Name-field history held 30 entries — eMule's
CCustomAutoCompletedefault, adopted in the #643 review rather than chosen for aMule. A history is only useful as far back as it reaches, and 30 queries is a short reach for anyone who searches often. Clearing it has been a deliberate, confirmed action since #754, so there is less cost to keeping more of it around.Eviction is LRU and stays that way:
ApplySearchHistoryEntrymoves a searched term to the front and drops any earlier case-insensitive copy, so the tail that falls off is the least recently searched, not the least recently added. Terms you actually reuse stay near the front regardless of the cap, and the extra 70 slots buy depth for the long tail. (Selecting an entry from the dropdown does not refresh it — only running the search does.)The constant moves
It goes from an anonymous namespace in
SearchDlg.cpptoSearchHistory.h, beside the function that consumes it. That closes a quiet gap in the tests:CapIsExactlyThirtyByProjectConventionclaimed to lock "the constant CSearchDlg actually passes in", but hardcoded its own copy of the number — so it would have gone on passing while the GUI used something else entirely. It now reads the same symbol the GUI does and cannot drift from it.Dropdown length, checked on all three platforms
The obvious worry is that a longer history makes #755's other complaint worse — the dropdown overlapping other UI. Tried with a staged 100-entry history on each platform:
No platform runs the list off the display, so this does not make that complaint worse. It does not fix it either — capping the visible entries needs a different widget, since wxWidgets exposes no dropdown-height control on plain
wxComboBox(SetPopupMaxHeight()iswxComboCtrl/wxOwnerDrawnComboBoxonly). That is left on the issue, which stays open.33/33 unit tests pass; clang-format 18 and both clang-tidy tiers clean on the diff.