Skip to content

feat(search): persist search history across restarts - #643

Merged
got3nks merged 8 commits into
amule-org:masterfrom
LSalami:add-search-history
Jul 28, 2026
Merged

feat(search): persist search history across restarts#643
got3nks merged 8 commits into
amule-org:masterfrom
LSalami:add-search-history

Conversation

@LSalami

@LSalami LSalami commented Jul 27, 2026

Copy link
Copy Markdown

Summary

aMule's search field had no memory of past searches (#641).

Implementation

  • Swapped the plain text control for a wxComboBox in the search
    panel. It's a wxTextEntry just like wxTextCtrl, so the existing
    GetValue()/Clear() call sites keep working unchanged via that
    shared base — no widget-specific special-casing needed elsewhere.
  • Submitted search terms are persisted to wxConfig
    (/eMule/SearchHistory/*), most-recent-first, deduplicated
    case-insensitively, capped at 20 entries.
  • Right-clicking the field (or invoking the context-menu key —
    wxEVT_CONTEXT_MENU fires from both) opens a small menu:
    • Remember search history (checkbox, default on) — lets a user
      pause recording without losing what's already saved.
    • a separator, then Clear search history — a one-shot
      destructive action, kept visually apart from the toggle above so
      it doesn't read as another state in the same group.

Test plan

  • Built locally on macOS (arm64)
  • Verified via real UI interaction (not just compile): submitted several searches, confirmed each is written to amule.conf under [eMule/SearchHistory], most-recent-first
  • Verified deduplication: re-submitting an existing term moves it to the front instead of creating a duplicate entry
  • Verified the combo box is picked up as a native accessible control (1 AXComboBox found via macOS accessibility inspection) rather than an opaque custom widget
  • Verified the right-click menu, the Remember toggle, and Clear all work as expected
  • scripts/update-po.sh run to register the two new menu strings

@got3nks got3nks left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @LSalami — genuinely useful first step, and the core is clean (case-insensitive dedup, cap, most-recent-first; Enter-to-search preserved; the wxTextEntry cross-cast is the right way to keep the existing call sites working). A few things to tighten before merge, one sizing tweak, and a bigger-picture note on where this ideally heads.

Before merge

  1. Don't drop the field's edit context menu. The Name field was a CMuleTextCtrl, whose entire purpose is providing the Cut/Copy/Paste/Select-All right-click menu; it's now a plain wxComboBox whose only context menu is the new Remember/Clear one, so right-clicking the search box no longer offers paste/copy. Please augment rather than replace — the standard edit items + a separator + the history actions.

  2. Persist to a dedicated file, not amule.conf. Search terms are arguably private, and putting them in the main config both bloats it and makes them travel with the file (config backups, and the --amule-config-file push to amuleweb/amuleapi). eMule keeps them in a dedicated AC_SearchStrings.dat in the config dir; a small searchhistory.dat mirrors that and keeps the prefs clean. (The enable flag is a real setting, so that one can stay in prefs — see #3.)

  3. Move the "Remember search history" toggle into Preferences. Right now it's reachable only by right-clicking the field, so it's effectively undiscoverable. It's a persistent setting, so Preferences is where users expect it — put it on a page present in both the monolithic and amulegui builds, since this feature works in both. Keep Clear as the context-menu action (a one-shot action, not a setting).

  4. Bump the cap. 20 is a bit tight; eMule keeps 30 (CCustomAutoComplete's default). These are just strings, so matching 30 (or a little more) is free.

Minor / optional

  • It's a plain dropdown with no type-ahead — consider wxComboBox::AutoComplete(...) so the history surfaces as you type (eMule uses ACO_AUTOSUGGEST); it also helps discoverability.
  • Worth stating explicitly that this is query history, not result persistence — see below.

Bigger picture (not asking for it in this PR)
The original #641 also wanted the results persisted, not just the queries. The fuller solution is real search persistence — parameters and the result hits — and eMule already has the blueprint: CSearchList::StoreSearches()/LoadSearches() serialize each open search plus its CSearchFile results to StoredSearches.met on shutdown and restore them on startup. The key detail for us: doing that in amuled (daemon-side) reloads the results back into the searchlist, so a restored hit stays downloadable by hash — meaning searches survive an amuled restart for every client (amulegui and amuleapi), not just the monolithic GUI, with no client-side changes. So this PR is a fine first increment; I'd just frame it as query-history and leave #641 open for the amuled-side result persistence as the follow-up.

LSalami added a commit to LSalami/amule that referenced this pull request Jul 27, 2026
…refs toggle, edit menu

Addresses all four blocking points from got3nks's review, plus both
optional suggestions:

1. Edit context menu restored. Overriding the field's context menu no
   longer drops Cut/Copy/Paste/Select All -- same custom-Paste-ID idiom
   as CMuleTextCtrl::OnRightDown (wxMenu over-permits wxID_PASTE, so it
   gets a manual clipboard-content check), plus a separator, plus the
   history's own Clear action.

2. Persisted to a dedicated searchhistory.dat (via CTextFile, one term
   per line) instead of amule.conf -- mirrors eMule's AC_SearchStrings.dat.
   Query terms no longer bloat the main config or travel with config
   backups / the --amule-config-file push to amuleweb/amuleapi.

3. "Remember search history" moved into Preferences > General as an
   ordinary Cfg_Bool checkbox (IDC_SEARCHHISTORYENABLED), same
   NewCfgItem wiring as every other GUI-behavior toggle on that page.
   It's a client-side-only setting, so the one checkbox works unchanged
   in both amule and amuleGUI. The context menu keeps only Clear (a
   one-shot action, not a setting to toggle from a hidden menu).

4. Cap raised from 20 to 30, matching eMule's CCustomAutoComplete default.

Optional, done anyway:
- wxComboBox::AutoComplete() wired up (eMule's ACO_AUTOSUGGEST
  equivalent), re-armed after every load/record/clear so it always
  reflects the current entry set.
- Explicitly framed as *query* history throughout (comments, tooltip)
  to distinguish it from the separate, not-yet-implemented result
  persistence that's the rest of amule-org#641.

The dedup/move-to-front/cap logic that used to live inline in
CSearchDlg::RecordSearchHistory is extracted into a pure function,
ApplySearchHistoryEntry() (SearchHistory.h/.cpp), decoupled from
wxComboBox/wxConfig specifically so it's unit-testable without a wx
event loop -- new SearchHistoryTest covers empty-list insert, reorder
of an existing term, case-insensitive dedup, empty-term no-op, and
capping (including locking in the actual 30-entry constant).

Rebased onto current master; po/ catalogs regenerated as the final
step so the diff is just the new/changed strings in sync with the tree.

Verified for real: built and ran both amule and amulegui, plus the
full unit test suite (27/27 passing, including the new
SearchHistoryTest's 7 cases).
@LSalami
LSalami force-pushed the add-search-history branch from 18a1fb9 to b7a500a Compare July 27, 2026 20:23
@LSalami

LSalami commented Jul 27, 2026

Copy link
Copy Markdown
Author

Done in b7a500a — addressed all four blocking points, plus both optional suggestions:

  1. Edit context menu restored. Overriding the field's context menu no longer drops Cut/Copy/Paste/Select All — reused the same custom-Paste-ID idiom CMuleTextCtrl::OnRightDown already uses (wxMenu auto-manages Cut/Copy off the stock IDs, but is too permissive about wxID_PASTE, so Paste gets a manual clipboard-content check).
  2. Persisted to a dedicated searchhistory.dat (via CTextFile, one term per line) instead of amule.conf — mirrors eMule's AC_SearchStrings.dat.
  3. "Remember search history" moved into Preferences > General as an ordinary Cfg_Bool checkbox, same NewCfgItem wiring as the other GUI-behavior toggles on that page. Client-side-only setting, so it works unchanged in both amule and amuleGUI. Context menu now only has Clear.
  4. Cap raised 20 → 30, matching eMule's CCustomAutoComplete default.

Optional, done anyway:

  • wxComboBox::AutoComplete() wired up, re-armed after every load/record/clear.
  • Framed explicitly as query history throughout (comments + tooltip) to distinguish from the separate result-persistence half of [Request] Search history #641.

Also extracted the dedup/move-to-front/cap logic into a pure ApplySearchHistoryEntry() function (SearchHistory.h/.cpp), decoupled from wxComboBox/wxConfig so it's actually unit-testable — new SearchHistoryTest covers empty-list insert, reordering an existing term, case-insensitive dedup, empty-term no-op, and capping (including locking in the real 30-entry constant), 7 cases all passing.

Rebased onto current master; po/ regenerated as the final step per your note on #278.

Verified for real: built and ran both amule and amulegui locally, plus the full unit test suite (27/27 passing).

Agreed on leaving #641's result-persistence (StoredSearches.met-equivalent, done daemon-side) as a separate follow-up — happy to pick that up next once this lands.

@got3nks

got3nks commented Jul 27, 2026

Copy link
Copy Markdown

Only thing between this and merge is a mechanical one: the po/ catalogs conflict with the regen from a few PRs that landed today (#648/#650/#651) — the source itself applies cleanly. Could you rebase on current master and re-run scripts/update-po.sh? That resolves the catalog conflict deterministically, and I'll merge as soon as it's green.

Everything else looks great. One note (not a blocker): the as-you-type autocomplete works as intended on a Linux/GTK build, but on macOS (wx 3.3.3) it doesn't fire — that's a wxWidgets limitation for text/combo-box autocomplete on the macOS port, not anything in this PR. The dropdown surfaces the history on every platform, so there's nothing to change here.

LSalami added a commit to LSalami/amule that referenced this pull request Jul 27, 2026
…refs toggle, edit menu

Addresses all four blocking points from got3nks's review, plus both
optional suggestions:

1. Edit context menu restored. Overriding the field's context menu no
   longer drops Cut/Copy/Paste/Select All -- same custom-Paste-ID idiom
   as CMuleTextCtrl::OnRightDown (wxMenu over-permits wxID_PASTE, so it
   gets a manual clipboard-content check), plus a separator, plus the
   history's own Clear action.

2. Persisted to a dedicated searchhistory.dat (via CTextFile, one term
   per line) instead of amule.conf -- mirrors eMule's AC_SearchStrings.dat.
   Query terms no longer bloat the main config or travel with config
   backups / the --amule-config-file push to amuleweb/amuleapi.

3. "Remember search history" moved into Preferences > General as an
   ordinary Cfg_Bool checkbox (IDC_SEARCHHISTORYENABLED), same
   NewCfgItem wiring as every other GUI-behavior toggle on that page.
   It's a client-side-only setting, so the one checkbox works unchanged
   in both amule and amuleGUI. The context menu keeps only Clear (a
   one-shot action, not a setting to toggle from a hidden menu).

4. Cap raised from 20 to 30, matching eMule's CCustomAutoComplete default.

Optional, done anyway:
- wxComboBox::AutoComplete() wired up (eMule's ACO_AUTOSUGGEST
  equivalent), re-armed after every load/record/clear so it always
  reflects the current entry set.
- Explicitly framed as *query* history throughout (comments, tooltip)
  to distinguish it from the separate, not-yet-implemented result
  persistence that's the rest of amule-org#641.

The dedup/move-to-front/cap logic that used to live inline in
CSearchDlg::RecordSearchHistory is extracted into a pure function,
ApplySearchHistoryEntry() (SearchHistory.h/.cpp), decoupled from
wxComboBox/wxConfig specifically so it's unit-testable without a wx
event loop -- new SearchHistoryTest covers empty-list insert, reorder
of an existing term, case-insensitive dedup, empty-term no-op, and
capping (including locking in the actual 30-entry constant).

Rebased onto current master; po/ catalogs regenerated as the final
step so the diff is just the new/changed strings in sync with the tree.

Verified for real: built and ran both amule and amulegui, plus the
full unit test suite (27/27 passing, including the new
SearchHistoryTest's 7 cases).
@LSalami
LSalami force-pushed the add-search-history branch from 5edeb4b to 0ec00b3 Compare July 27, 2026 20:49
LSalami added a commit to LSalami/amule that referenced this pull request Jul 27, 2026
Mechanical rebase to resolve the po/ catalog conflicts against master's
amule-org#648/amule-org#650/amule-org#651 (per got3nks's note on amule-org#643) -- no source changes here,
scripts/update-po.sh output only, so the diff is just the tree back in
sync with the current strings.
@LSalami

LSalami commented Jul 27, 2026

Copy link
Copy Markdown
Author

Rebased onto current master and re-ran scripts/update-po.sh in 0ec00b3 — that resolves the po/ catalog conflicts against #648/#650/#651 deterministically, no source changes in that commit. Also fixed a CI-only link failure in the same push (5edeb4b): SearchHistoryTest needed Format.cpp + strerror_r.c linked in for muleunit's glibc backtrace path, same as MagnetURITest/CMuleCollectionTest already do — passed locally on macOS but failed on Ubuntu/mingw.

Thanks for flagging the macOS autocomplete behavior — good to know it's a wx port limitation rather than something to chase down here, and reassuring that the dropdown itself is solid on every platform either way.

LSalami added a commit to LSalami/amule that referenced this pull request Jul 28, 2026
Mechanical rebase to resolve the po/ conflicts against master's
amule-org#643/amule-org#646/amule-org#648/amule-org#650/amule-org#651 -- no source changes here. Confirms got3nks's
prediction on the Alt+<letter>-outside-_() fix: the only genuinely new
msgid in the diff is "Navigate" (the new macOS menu itself); every
other hunk is line-number-comment churn from the rebase, not orphaned
or duplicated translations.
@got3nks

got3nks commented Jul 28, 2026

Copy link
Copy Markdown

@LSalami this'll need a fresh po regen (not on you), the catalogs drift every time another merge touches them.

LSalami added 8 commits July 28, 2026 09:27
The search field was a plain text control with no memory of past
searches. Swap it for a wxComboBox (a wxTextEntry, like wxTextCtrl,
so the existing GetValue()/Clear() call sites keep working via that
shared base) and persist submitted terms to wxConfig, most-recent
first, capped at 20 entries and deduplicated case-insensitively.

Right-clicking the field opens a small menu ("Remember search
history" checkbox + "Clear search history") so history can be paused
or wiped without a dedicated Preferences page.

i18n: ran scripts/update-po.sh to register the two new menu strings.
A separator between the checkbox and "Clear search history" keeps the
one-shot destructive action visually distinct from the persistent
on/off state, instead of reading as a second toggle in the same group.
Fixes the modernize-use-nullptr clang-tidy finding on the changed
line; matches the nullptr convention already used a couple of
wxComboBox constructions below in the same file.
…refs toggle, edit menu

Addresses all four blocking points from got3nks's review, plus both
optional suggestions:

1. Edit context menu restored. Overriding the field's context menu no
   longer drops Cut/Copy/Paste/Select All -- same custom-Paste-ID idiom
   as CMuleTextCtrl::OnRightDown (wxMenu over-permits wxID_PASTE, so it
   gets a manual clipboard-content check), plus a separator, plus the
   history's own Clear action.

2. Persisted to a dedicated searchhistory.dat (via CTextFile, one term
   per line) instead of amule.conf -- mirrors eMule's AC_SearchStrings.dat.
   Query terms no longer bloat the main config or travel with config
   backups / the --amule-config-file push to amuleweb/amuleapi.

3. "Remember search history" moved into Preferences > General as an
   ordinary Cfg_Bool checkbox (IDC_SEARCHHISTORYENABLED), same
   NewCfgItem wiring as every other GUI-behavior toggle on that page.
   It's a client-side-only setting, so the one checkbox works unchanged
   in both amule and amuleGUI. The context menu keeps only Clear (a
   one-shot action, not a setting to toggle from a hidden menu).

4. Cap raised from 20 to 30, matching eMule's CCustomAutoComplete default.

Optional, done anyway:
- wxComboBox::AutoComplete() wired up (eMule's ACO_AUTOSUGGEST
  equivalent), re-armed after every load/record/clear so it always
  reflects the current entry set.
- Explicitly framed as *query* history throughout (comments, tooltip)
  to distinguish it from the separate, not-yet-implemented result
  persistence that's the rest of amule-org#641.

The dedup/move-to-front/cap logic that used to live inline in
CSearchDlg::RecordSearchHistory is extracted into a pure function,
ApplySearchHistoryEntry() (SearchHistory.h/.cpp), decoupled from
wxComboBox/wxConfig specifically so it's unit-testable without a wx
event loop -- new SearchHistoryTest covers empty-list insert, reorder
of an existing term, case-insensitive dedup, empty-term no-op, and
capping (including locking in the actual 30-entry constant).

Rebased onto current master; po/ catalogs regenerated as the final
step so the diff is just the new/changed strings in sync with the tree.

Verified for real: built and ran both amule and amulegui, plus the
full unit test suite (27/27 passing, including the new
SearchHistoryTest's 7 cases).
Same Linux/mingw-only link failure as CMuleCollectionTest/MagnetURITest
already work around: muleunit's MuleDebug.cpp needs CFormat for its
glibc backtrace path, which isn't compiled in on macOS -- so the
target linked fine locally but failed on the Ubuntu/mingw CI builds
(and the clang-tidy jobs, which build the tree first).
Mechanical rebase to resolve the po/ catalog conflicts against master's
amule-org#648/amule-org#650/amule-org#651 (per got3nks's note on amule-org#643) -- no source changes here,
scripts/update-po.sh output only, so the diff is just the tree back in
sync with the current strings.
txtIgnoreEmptyLines|txtStripWhitespace has no single named enumerator
to cast to -- clang-tidy Tier-1 flagged it (clang-analyzer-optin.core.
EnumCastOutOfRange), correctly: EReadTextFile isn't a flag enum, so a
synthesized OR'd value is genuinely out of its declared range even
though CTextFile::ReadLines treats it as bitflags at runtime.

txtReadDefault would dodge the cast but also drops '#'-led lines,
silently eating a legitimate search term that happens to start with
one. Read unfiltered (txtReadAll, a real enumerator) and do the
trim/empty-line-drop by hand instead -- same behavior, no cast, no
lost terms.
Mechanical rebase to resolve the po/ conflicts against master's amule-org#642
(just merged) -- no source changes here.
@LSalami
LSalami force-pushed the add-search-history branch from 126864e to 406a9de Compare July 28, 2026 07:30
@LSalami

LSalami commented Jul 28, 2026

Copy link
Copy Markdown
Author

Rebased onto current master and regenerated po/ again — clean now (406a9de).

@got3nks
got3nks merged commit c655e7b into amule-org:master Jul 28, 2026
13 checks passed
@got3nks got3nks mentioned this pull request Jul 28, 2026
LSalami added a commit to LSalami/amule that referenced this pull request Jul 28, 2026
Mechanical rebase to resolve po/ conflicts against master's amule-org#642/amule-org#643
-- no source changes here. Also resolved a real conflict in
muuli_wdr.h: amule-org#643's IDC_SEARCHHISTORYENABLED (10488) landed on master,
which this branch had already anticipated and reserved 10489 for.
LSalami added a commit to LSalami/amule that referenced this pull request Jul 28, 2026
Mechanical rebase to resolve conflicts against master's amule-org#642/amule-org#643 --
no source changes here. Resolved a real conflict in amuleDlg.cpp's
event table: amule-org#642 added the Alt+<letter> EVT_MENU block right where
this branch removed the EVT_TOOL(ID_BUTTONCONNECT, ...) line; kept
amule-org#642's block, dropped the connect-button line as intended.
LSalami added a commit to LSalami/amule that referenced this pull request Jul 28, 2026
Mechanical rebase to resolve po/ conflicts against master's amule-org#642/amule-org#643
-- no source changes here. Also resolved a real conflict in
muuli_wdr.h: amule-org#643's IDC_SEARCHHISTORYENABLED (10488) landed on master,
which this branch had already anticipated and reserved 10489 for.
LSalami added a commit to LSalami/amule that referenced this pull request Jul 28, 2026
Mechanical rebase to resolve po/ conflicts against master's amule-org#642/amule-org#643
-- no source changes here. Also resolved a real conflict in
muuli_wdr.h: amule-org#643's IDC_SEARCHHISTORYENABLED (10488) landed on master,
which this branch had already anticipated and reserved 10489 for.
LSalami added a commit to LSalami/amule that referenced this pull request Jul 28, 2026
Mechanical rebase to resolve conflicts against master's amule-org#642/amule-org#643 --
no source changes here. Resolved a real conflict in amuleDlg.cpp's
event table: amule-org#642 added the Alt+<letter> EVT_MENU block right where
this branch removed the EVT_TOOL(ID_BUTTONCONNECT, ...) line; kept
amule-org#642's block, dropped the connect-button line as intended.

Regenerated again via scripts/update-po.sh after later rebasing onto
master's amule-org#671 (Preferences sidebar) and amule-org#665/amule-org#662 (amuleapi
credentials, sparse part-file setting), which had added strings this
branch's catalogs didn't have yet.
LSalami added a commit to LSalami/amule that referenced this pull request Jul 28, 2026
Mechanical rebase to resolve conflicts against master's amule-org#642/amule-org#643 --
no source changes here. Resolved a real conflict in amuleDlg.cpp's
event table: amule-org#642 added the Alt+<letter> EVT_MENU block right where
this branch removed the EVT_TOOL(ID_BUTTONCONNECT, ...) line; kept
amule-org#642's block, dropped the connect-button line as intended.

Regenerated again via scripts/update-po.sh after later rebasing onto
master's amule-org#671 (Preferences sidebar) and amule-org#665/amule-org#662 (amuleapi
credentials, sparse part-file setting), which had added strings this
branch's catalogs didn't have yet.
got3nks pushed a commit that referenced this pull request Jul 28, 2026
* feat(gui): remove the global Connect/Disconnect toolbar button

Closes #402. Per-network controls already exist (Kad pane's Start/Stop,
Servers pane's ED2K connect/disconnect), the connection state is
already shown in the status bar, and after discussing relocating it
into the Networks view the agreed call (got3nks + ngosang) was simply
to remove it -- mirrors what #585 already did on the Web UI side.

Removed the toolbar tool, its three skin icons (Toolbar_Connect/
Disconnect/Connecting) and their bitmap wiring, the button-update block
in ShowConnectionState(), and the two EnableTool(ID_BUTTONCONNECT, ...)
call sites.

CamuleDlg::OnBnConnect() itself stays -- CMuleTrayIcon::DoConnectDisconnect()
still calls it for the tray icon's own connect/disconnect action, which
is unaffected by this change.

ShowConnectionState()'s skinChanged parameter was only ever read by the
now-removed block, so it's gone too, along with the one call site that
passed true and the forceUpdate plumbing that reached it through
GuiEvents.cpp's ShowConnState() free function.

Left ID_BUTTONCONNECT and muuli_wdr.cpp's muleToolbar() (which still
calls it) alone -- that function predates Apply_Toolbar_Skin, is never
called anywhere, and touching pre-existing dead code is out of scope
for this fix.

Verified: built and ran both amule and amulegui cleanly, full unit
test suite passing (26/26). Confirmed via the po/ diff that "Connect"/
"Disconnect"/"Cancel" remain in the catalog (still used by other
per-network buttons) -- only their now-orphaned toolbar-specific
tooltip strings were dropped. Could not get a real on-screen visual
confirmation of the toolbar layout -- same macOS Accessibility
automation limitations as #180 got in the way of screenshotting the
actual app window.

* i18n: regenerate po/ catalogs after rebasing onto current master

Mechanical rebase to resolve conflicts against master's #642/#643 --
no source changes here. Resolved a real conflict in amuleDlg.cpp's
event table: #642 added the Alt+<letter> EVT_MENU block right where
this branch removed the EVT_TOOL(ID_BUTTONCONNECT, ...) line; kept
#642's block, dropped the connect-button line as intended.

Regenerated again via scripts/update-po.sh after later rebasing onto
master's #671 (Preferences sidebar) and #665/#662 (amuleapi
credentials, sparse part-file setting), which had added strings this
branch's catalogs didn't have yet.

* feat(gui): turn the per-network Disconnect buttons into Connect/Cancel/Disconnect toggles

Relocates the removed global toolbar button's functionality into the
ED2K (IDC_ED2KDISCONNECT) and Kad (ID_KADDISCONNECT) panes instead of
just dropping it, per got3nks's proposal on #663: each button now
mirrors ed2kState/kadState (off/connecting/connected) using the
existing connButImg() bitmaps, and can independently connect or
cancel/disconnect its own network -- something the old OR-toggled
global button never allowed. The ED2K pane also gains a Connect
button it never had (previously Disconnect-only).

CServerWnd::UpdateED2KConnectButton() / CKadDlg::UpdateConnectButton()
are called from CamuleDlg::ShowConnectionState() alongside the
existing UpdateED2KInfo()/UpdateKadInfo() calls, and once from each
pane's own ctor/Init() for the initial paint. OnBnClickedED2KDisconnect
and OnBnClickedDisconnectKad gained the missing "connect when off"
branch; OnBnConnect and the tray icon are untouched.

Verified interactively on Windows: both buttons show the correct
label/icon for their live state, ED2K disconnect/reconnect works from
its own button, and disabling ED2K in Preferences while Kad stays
connected confirms the two toggle independently.

po/ regenerated via scripts/update-po.sh to match the new button
labels ("Disconnect Kad" is now unused and moves to the obsolete
section; no new translatable strings).

* fix(gui): drop the per-network connect-button state cache

got3nks flagged two bugs in review (PR #663): the static cache's
early return also skipped Enable(), so the button could get stuck
disabled once IsReady()/network-pref state changed without the
Connect/Cancel/Disconnect state itself changing (master had two now-
deleted escape hatches -- the skinChanged force-path and an
unconditional EnableTool call -- that used to paper over this). Worse,
the cache was function-scope static, surviving widget recreation, so
a fresh pane's Init() call could early-return and leave the wxDesigner
default label ("Disconnect Kad") on screen.

Dropping the cache removes both: SetLabel/SetBitmap/Enable are cheap
on a plain wxButton (unlike the old toolbar tool, which had real
native-image-list reasons to memoize, see amuleDlg.cpp:1054-ish), and
master's own second call site already did an unconditional EnableTool
every tick without issue.

Second review round (macOS + Ubuntu ARM64, monolithic + amuleGUI)
confirmed the toggle works functionally but flagged three cosmetic
issues, all fixed here:

- No gap between the connect-button icon and its label. The obvious
  fix, wxButton::SetBitmapMargins(), is NOT portable (wxOSX overrides
  it, wxGTK inherits the base no-op) -- prefixing the label with a
  literal space outside the _() call behaves identically on both and
  introduces no new msgid.
- The Kad button was stretched full-width by its sizer's .Expand()
  flag, while the ED2K one sizes naturally; GTK and macOS then filled
  the slack differently (centered vs. icon-left). Dropped .Expand()
  so the two buttons size the same way -- trades away lining up with
  the "Bootstrap from known clients" button above it, but the two
  connect buttons reading as the same control matters more.
- CServerWnd::UpdateED2KConnectButton() and CKadDlg::UpdateConnectButton()
  were near-identical (same 3-state enum, same switch, same three
  bitmaps). Factored into a shared SetConnectButtonState(button, state,
  enabled) helper in muuli_wdr.cpp/.h, next to connButImg() which it
  reuses -- so the icon-spacing fix above lives in one place instead
  of two.

Verified interactively on Windows: re-tested full disconnect/reconnect
cycle on both panes, toggling ED2K off and back on via Preferences
(existing behavior: needs an app restart to re-enable) with correct
state on the fresh post-restart paint, and confirmed by screenshot
that both buttons now show a visible icon/label gap and size the same
way (Kad no longer stretched). Also confirmed amulegui (CLIENT_GUI)
builds clean with these changes; did not perform full interactive
remote-daemon testing.

* fix(gui): first-paint layout bug + connect-button placement, per review

Two more issues from got3nks's Linux/GTK pass on #663.

1. Bug: the ED2K connect button painted with the icon overlapping the
   label on first show (Kad was fine, only by luck of timing).
   CServerWnd::UpdateED2KConnectButton() runs after sizer->Show(this,
   TRUE) in the ctor, so SetBitmap() grows the button's best size
   after the row was already measured against the text-only label,
   and nothing re-lays it out until a window resize. Fixed by calling
   Layout() on the button's parent at the end of the shared
   SetConnectButtonState() helper -- covers both panes, and stops Kad
   from relying on being laid out late (its notebook page is only
   measured once shown, after Init() already set its bitmap).

2. Layout: moved both connect/disconnect toggles to lead their pane
   instead of sitting among secondary controls, per got3nks's request:
   - ED2K (serverListDlgUp): toggle now on its own top row, right-
     aligned via a stretch spacer, directly above the server list.
     The "Add server manually" form (name/IP/port/Add) moved below
     the table instead of sharing a row with the toggle; the vertical
     separator that used to sit between them is gone (no longer
     needed once they're not adjacent).
   - Kad (KadDlg): toggle moved out of the "Bootstrap" static box
     (where it sat under "Bootstrap from known clients") to its own
     top row above the box, right-aligned the same way as ED2K's.
     item0 changed from a 1x1 FlexGridSizer (which only ever held the
     two-column area) to a plain vertical wxBoxSizer so it can hold
     the new top row plus the existing two-column layout.

Both panes now read "primary control on top, secondary/manual
controls below" -- matching shape on both tabs.

Verified interactively on Windows: screenshotted both panes, confirmed
no icon/label overlap on first paint (no window resize needed), and
re-ran the disconnect/reconnect toggle cycle on both after the layout
change.
got3nks added a commit that referenced this pull request Jul 30, 2026
…ry UI (#709)

Four things, all around the IDC_SEARCHNAME field.

"Reset Fields" wiped the whole search history instead of just the Name field.
The call was already casting to wxTextEntry to avoid exactly that, with a
comment saying so, but the cast does not help: wxTextEntry::Clear() is
virtual and wxComboBoxBase overrides it as { wxItemContainer::Clear();
wxTextEntry::Clear(); }, so the call dispatched to the override and emptied
the dropdown's item list too. Use SetValue("") instead, which only touches
the text. This also explains the second half of the report -- the last term
still being offered as a completion after the list looked empty -- since the
item list was cleared while the autocomplete set was left armed.

Fixes a null dereference in SearchListCtrl's "search related files" action.
The field became a wxComboBox in #643, and wxComboBox does not derive from
wxTextCtrl -- it is wxWindowWithItems<wxControl, wxComboBoxBase> -- so the
CastByID dynamic_cast to wxTextCtrl there had been yielding nullptr, which
the following SetValue() dereferenced. Cast to wxTextEntry, the common base
of both control types.

Adds a visible "Clear search history" button after the search-type choice.
The action already existed in the field's right-click menu, but that menu is
unreachable on Windows: wxComboBox's editable part is a native child EDIT
window, and wx forwards only key, focus and clipboard messages from it
(ShouldForwardFromEditToCombo in src/msw/combobox.cpp) -- WM_CONTEXTMENU is
not among them, so the native menu appears instead and our handler never
runs. Built in SearchDlg.cpp rather than muuli_wdr so the existing msgid
keeps its catalog position; adding the same string to muuli_wdr.cpp would
move the pot entry, since xgettext orders entries by file scan order.

Gates the whole history UI on "Remember search history", which previously
only stopped new terms being recorded: existing terms stayed visible in the
dropdown and kept being offered as completions, and the field kept its
dropdown either way. Now the Name field is a plain wxTextCtrl when the
preference is off, the Clear button is hidden, and no stored terms are
loaded. searchhistory.dat is deliberately left on disk, so re-enabling
restores the previous history rather than starting over. Applied live from
PrefsUnifiedDlg::OnOk, so no restart is needed.
@LSalami
LSalami deleted the add-search-history branch August 5, 2026 14:12
got3nks added a commit that referenced this pull request Aug 6, 2026
The Search tab's Name-field history held 30 entries, eMule's CCustomAutoComplete default, 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. Raised to 100 on request (#755).

Eviction is LRU and stays that way: ApplySearchHistoryEntry moves a searched term to the front and drops any earlier case-insensitive copy, so what falls off the tail is the least recently searched rather than the least recently added. Terms that get reused stay near the front whatever the cap is, which is what makes the extra slots worth having -- they go to the long tail instead of hoarding stale one-offs.

The constant moves from an anonymous namespace in SearchDlg.cpp to SearchHistory.h, beside the function that consumes it. That closes a gap in the tests: the one 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.

A longer history could have worsened the other half of #755 -- the dropdown overlapping other UI -- so the list was checked with a full 100 entries on Windows 11, Ubuntu and macOS first. None of the three runs it off the screen. Bounding the visible entries is a separate matter and not addressed here: wxWidgets exposes no dropdown-height control on plain wxComboBox (SetPopupMaxHeight is wxComboCtrl/wxOwnerDrawnComboBox only), so it needs a different widget rather than a setting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants