feat(gui): amuleGUI remote-to-local path mappings (#843) - #854

Verified the design claims hold: path mappings never reach CEC_Prefs_Packet or the m_exchange_* sets and are read/written straight through wxConfigBase, so they genuinely stay GUI-local; and ResolvePath() really is the single choke point — GetAvailability, CanOpen, CanReveal, Open and Reveal all route through it, so the availability decision and the action use the same translated path. Builds clean here on macOS (amule, amulegui, amuled), no warnings from project sources.

Three things to fix, the first two in the same few lines of ApplyPathMapping().

1. A trailing separator on the remote prefix silently corrupts every path it maps. The substitution is localPrefix.GetRaw() + remotePath.Mid(remotePrefix.length()), and CPath's constructor does not strip trailing separators (StripSeparators is used in JoinPaths/IsSameAs/StartsWith, never in the ctor). So remote /downloads/ with local /Volumes/dl gives /Volumes/dl + incoming/f.avi = /Volumes/dlincoming/f.avi. That is the most likely combination through this UI rather than a corner case: wxDirDialog::GetPath() never returns a trailing separator, so Browse always supplies a local prefix without one, while a user describing a remote directory naturally types one. Because availability is FileExists() on the mapped path, the result is that Open/Show stay greyed — the exact symptom the feature removes — with nothing indicating the mapping is malformed.

Fix: strip trailing separators from both prefixes where they are accepted (OnPathMappingAdd and LoadPathMappings), and let the remainder supply the separator.

2. The prefix match has no boundary check. remotePath.StartsWith(mapping.remotePrefix) means a mapping for /mnt/data also matches /mnt/data-old/f.avi and rewrites it to /Volumes/data-old/f.avi. Usually that path does not exist and the entry just disappears; if it happens to exist, it opens the wrong file.

Fix: require the character after the prefix to be a separator, or the whole string to match.

3. The explanatory paragraph does not use the panel width. itemHint->Wrap( 380 ) in muuli_wdr.cpp is a one-shot text transformation, not a layout constraint: it bakes hard line breaks at 380 pixels measured once at construction, so Expand() stretches the control while the text keeps its original breaks — hence the empty right margin. It is also not FromDIP()-scaled although the font is, so it reads narrower on HiDPI, and it never re-flows even though the dialog has wxRESIZE_BORDER. It is the only Wrap() call in muuli_wdr.cpp.

Fix: drop the Wrap() from muuli_wdr.cpp (that file is layout construction; the other pages keep logic out of it) and wrap from the panel's real client width in PrefsUnifiedDlg, re-applied on size. FromDIP(380) alone fixes only the HiDPI half.

Minor: OnPathMappingAdd returns silently on empty or duplicate input, so the button reads as dead. The fields keep their text, so it is recoverable, but a short message or a disabled Add would be clearer.

The known gap you flagged is the right call — the substitution is separator-agnostic string logic, and (1) above is the part that actually needs a Windows-shaped path to shake out.

#859 has landed on master, and it adds two helpers your path-mappings editor should use for its local-prefix column. Rebasing onto master will bring them in.

Background: writing GetPrintable() into a cell and reading it back as a CPath is not a round trip. On macOS wxConvFileName normalises to NFD, so a precomposed accented character comes back decomposed — same characters, different bytes, and a path a byte-exact filesystem does not have. HarvestSharedDirsList() had the same shape, which is what #859 fixed; the helpers came out of it file-local precisely so this editor could use them too.

They sit in an anonymous namespace above PopulateSharedDirsList(), so your functions see them with no declaration needed. Three changes:

Header, next to m_sharedDirRowPaths:

std::vector<CPath> m_pathMappingRowPaths;

PopulatePathMappingList() — clear the store after DeleteAllItems(), and record the path instead of writing its display form:

list->DeleteAllItems();
m_pathMappingRowPaths.clear();
...
list->InsertItem(row, mapping.remotePrefix);
SetListRowPath(list, row, 1, mapping.localPrefix, m_pathMappingRowPaths);

HarvestPathMappingList() — the whole wxListItem block goes away:

mapping.remotePrefix = list->GetItemText(row);   // unchanged, see below
mapping.localPrefix = GetListRowPath(list, row, m_pathMappingRowPaths);

OnPathMappingAdd():

const long row = list->InsertItem(list->GetItemCount(), remote);
SetListRowPath(list, row, 1, CPath(local), m_pathMappingRowPaths);

remotePrefix needs none of this and should stay exactly as it is: it is the daemon's path as an opaque wxString, never wrapped in a CPath, so it already round-trips byte for byte — and your duplicate check comparing those strings is right for the same reason.

The three points from the earlier review still stand independently of this.

All three, plus the minor, addressed:

1. Trailing separator — stripped at both places prefixes are accepted: OnPathMappingAdd() and LoadPathMappings() (the latter so an already-saved or hand-edited config gets cleaned up too, not just new entries).
2. Prefix boundary — ApplyPathMapping() now requires the character right after the matched prefix to be a separator (either convention, since the daemon's OS isn't known here) or the whole string to match exactly.
3. Wrap() width — dropped the fixed Wrap(380) from muuli_wdr.cpp; PrefsUnifiedDlg now wraps from the control's own real client width, once in PopulatePathMappingList() and again on every resize via a runtime Bind(wxEVT_SIZE) (size events don't route through the static event table by id the way command events do). This also folds in the HiDPI half for free, since it's reading the actual scaled width rather than a hardcoded value.
4. Minor — empty/duplicate Add now tells the user why instead of no-opping silently.

Also rebased onto current master and switched the local-prefix column over to #859's SetListRowPath()/GetListRowPath(), as you suggested.

Rebuilt amule/amuled/amulegui locally, all clean. po/ regenerated (msgid delta +13/-0: the 11 original #843 strings the rebase's po/ conflict resolution reset to upstream, plus 2 new validation-message strings). Local clang-tidy CI replica against upstream/master: Tier-1 only the 13 known pre-existing warnings, Tier-2 clean.

Checked all three fixes against the code and they're right — the strip at load as well as at entry is the part I'd have missed, since it repairs a config written before the fix. I've pushed one commit on top rather than send you round again for it (0341954b).

It reworks the hint re-flow. Bound on the paragraph, the handler fed itself: wxStaticText::SetLabel() resizes the control to fit its label, so restoring the unwrapped text made it briefly as wide as the whole sentence, that width arrived as another size event, and the two alternated until the stack went — EXC_BAD_ACCESS, "excessive recursion", inside SetLabel. It's now bound on the page, whose width the dialog sets and the label can't influence, so there's no feedback path rather than a guard hoping the widths settle.

Two more from the same area. Wrap() only inserts breaks and treats existing ones as hard, so wrapping in place could narrow but never rejoin — widening left the old narrow width, i.e. the margin this started from. The unwrapped text is kept and restored before each wrap. And dropping the construction-time Wrap() made the page's minimum width the whole sentence, since an unwrapped wxStaticText reports its single line as its best width; that's back, at parent->FromDIP(380) so it scales.

Also added the <common/Path.h> include StripSeparators was getting transitively.

Tested on macOS: opens at a sane width, fills it, re-flows both ways under a continuous drag.

One thing left alone: stripping means a prefix of just / collapses to empty and is rejected, so the daemon's root can't be mapped. Mapping /home instead works, and special-casing it seemed worse than the limitation.

One more commit (7c1fd5dc), on the cross-OS half of the mapping — the known gap in your description.

The trailing-separator trim used StripSeparators(), which consults this host's separator set. On a POSIX build that set has no backslash, so a Windows daemon's D:\dl\ kept its trailing separator and the join ran the halves together — the corruption the trim exists to prevent, alive in the mirror direction. It now goes through CPreferences::TrimRemotePrefix(), which accepts either convention, on the same grounds as your boundary test: the daemon's OS is not knowable here.

And the remainder spliced onto the local prefix keeps the daemon's separators, so a POSIX daemon feeds / into a path bound for Win32. Normalised on Windows only — / cannot appear in a Windows filename, whereas a backslash is ordinary in a POSIX one, so the mirror rewrite would corrupt names rather than fix separators.

Checked on a Windows VM with a real SMB share mapped to a drive letter: Test-Path is true for UNC, mapped-drive, and mixed forms alike, so availability and Open were already working there — this is for explorer /select,, which is the fussy one and the reason Reveal exists. UNC is unaffected by the rewrite, since only forward slashes are touched and the leading \\ stays. Built on Windows too, so the new #ifdef __WINDOWS__ branch has actually been through a compiler rather than only CI.