Whether a whole-sentence candidate is produced depends on which
dictionaries and local models are installed, so the CI product data
yields none and the presence requirement failed there. Check the
canonical reading of whatever whole-sentence candidates do appear, and
assert the shuangpin-to-quanpin conversion unconditionally instead --
that conversion is the value the fix attaches to those candidates.
The Google-pinyin whole-sentence candidate is inserted at index 0 in both
quanpin and shuangpin, so it is what Space commits. All four construction
sites built it without a canonical quanpin reading, so when it finished a
word-creation session update_creating_word_progress() had no reading to
join and can_store stayed false: the prefix plus the sentence went to the
host app but were never learned.
Carry the converted quanpin segmentation on those candidates, and extract
the early-return storage condition into a testable predicate.
Most of them come from logging that is compiled out. The disabled DIAG_LOGF /
FANY_IPC_LOG* macros expanded to `((void)0)`, so every HRESULT and parameter
that exists only to be logged looked unreferenced (C4189/C4100). They now
expand to `((void)sizeof(DiscardLogArgs(__VA_ARGS__)))`: the arguments stay in
an unevaluated context, so nothing is computed and no side effect runs, but the
names count as used.
The rest:
- C4100: commented out the parameter names that are genuinely unused
(WinMain's hPrevInstance/nCmdShow, AdjustWndPosition's hwnd/height, the
WebView2 callbacks' sender, and the no-op logging sinks).
- C4189/C4101: removed the dead locals and the unused catch bindings left over
with the log statements.
- C4244: made the DIP-to-pixel and int-to-float conversions explicit in
globals.h, ime_windows.cpp and the menu placement.
- C4456: renamed the shadowing `hwnd` and `lock` locals.
- C4005: guarded the tests' `#define NOMINMAX` with #ifndef; the test targets
already define it on the command line.
Release builds clean and ctest passes for the server, the tests and msimeui.
Warnings under vendor/ are untouched: those are submodules with their own
repositories.
Conflict resolutions:
* windows/src/Utils/FanyUtils.h -- develop deleted GetIMEDataDirPath too
(via #273), so both sides had identical content and the whole-file
conflict was only develop's .gitattributes LF normalisation. Took
develop's version; the symbol is now gone repo-wide.
* server/src/skin/candidate_skin_catalog.cpp -- #273 landed the same
wide-path manifest read on develop. Took develop's code verbatim so
this hunk is a no-op against develop, keeping only the comment that
explains why toml::parse_file(manifest.string()) is unusable here.
* server/tests/src/test_quanpin_scheme.cpp (x2) -- union: this branch's
test::Utf8(db_path) plus develop's `both` autocorrect-flag local.
Verified: server Release build clean, 282 tests pass.
The portable test target builds on a bare Linux runner with no vcpkg tree, so
the UTF-8 path helper cannot live in test_framework.h -- including the engine's
data_path.h there broke the sanitizer build. Move it to test_utf8_path.h, which
only the Windows tests that need it include.
Narrow strings in this repo are UTF-8, but std::filesystem converts narrow
strings through the system ANSI code page. The two agree only on ASCII, so any
path crossing that boundary breaks for users whose profile directory is not
ASCII -- and on a code page that cannot represent the characters at all,
path::string() throws rather than mangling.
The user dictionary replay tool hit exactly that. It handed the journal API
paths converted with path::string(); on a Chinese user name the conversion
threw, the exception escaped main(), and MSVC fastfailed the process with
0xC0000409. The installer treats any non-zero exit from replay as fatal, by
design, so those users could not upgrade at all. Build the paths from the wide
form via path_from_utf8/path_to_utf8 instead, and wrap main() so an unexpected
failure reports a diagnosable message and exit 1 rather than a fastfail code.
The skin catalog had the same defect: toml::parse_file(manifest.string())
routed the manifest path through the ANSI code page, and skins live under the
user profile. The throw escaped past the toml handlers around it. Read the file
through the wide path and parse the text instead.
Also drop FanyUtils::GetIMEDataDirPath, which built a path from the ANSI
environment block and dereferenced getenv without a null check. It had no
callers.
Tests cover the three layers this spans: the conversion contract itself, the
journal API in-process, the replay tool as the installer actually launches it,
and the skin catalog. Each non-ASCII case is paired with an ASCII control so a
failure says whether the path encoding or the data caused it. The replay and
skin tests were both confirmed to fail with their fix reverted.
Test fixtures pass paths through the new test::Utf8 helper rather than
path::string(), since the temp directory sits inside the user profile too.
Elevated setup left high-IL config.toml that medium-IL Settings never
rewrote on machines hit by the ANSI path bug. Make the data directory
writable, migrate ACP-mangled leftovers, rebuild unparseable files, and
keep remaining asset paths on wchar_t.
Fixes#242
std::filesystem::path(std::string) and path::string() convert through the
system ANSI code page, so a non-ASCII (e.g. Chinese) user profile path on a
non-UTF-8 (GBK 936) system was corrupted and every config read/write failed
with "设置保存失败"; on a code page that cannot represent the characters
path::string() threw, crashing the Server at startup in a watchdog restart loop.
Never route config paths through the ANSI narrow encoding:
- build g_config_path and the legacy voice path from the wide accessors;
- read config by opening the wide path + toml::parse(text) instead of
toml::parse_file(path.string()) (LoadImeConfig, MigrateLegacyVoiceInputConfig);
- build and open the TSF shared config path as a wide std::filesystem::path.
Add a regression test that round-trips config read/write under a Chinese
profile path.
* fix(server): publish candidate pages as immutable snapshots
Global::candidate_ui is a plain global with no lock. The IPC worker rebuilds and destroys its vectors while the UI thread paints from them, and the handoff is an asynchronous PostMessage, so the worker returns to its loop and pops the next task without waiting. A second keystroke, or an async english.db candidate arrival, could free the page_views buffer and its strings while FillItemsFromUi held a reference into it. ShowItemContextMenu reads the same vectors from a mouse callback with no message handoff at all.
The worker now copies each finished page into an immutable snapshot and publishes it at the single point where the rebuild completes; every UI-thread reader loads that snapshot once and reads only its own copy. PostMessage is deliberately left alone: the worker also serves UI-originated tasks, so a synchronous handoff would invite a cross-thread deadlock.
* fix(server): bound the aux and diagnostic pipe reads
Both listeners serve a single pipe instance inline and polled a NOWAIT handle for as long as the process ran, breaking only on a read that succeeded or failed with something other than ERROR_NO_DATA. A client that connected and never wrote held the only instance forever, so the loop never returned to ConnectNamedPipe and no later client was accepted, permanently losing the langbar right-click channel and the terminal-deactivation teardown fallback.
Give both loops the same deadline the hello reads already use and go back to accepting when it expires.
* fix(server): snapshot the voice config and replace config.toml atomically
GetConfiguredVoiceInput handed out a reference to a global that LoadImeConfig rewrites from the IPC worker thread, clearing and refilling two std::map members and around twenty strings. The voice control thread and the low-level keyboard hook, which copies the struct on every keystroke, walked those nodes as they were freed. LoadImeConfig now parses into a local and publishes it in one step under a process-local shared_mutex, and the getter returns a snapshot by value. The file lock is deliberately not reused for readers: it is a named cross-process mutex with a five second wait, and taking it in a keyboard hook would stall typing system-wide.
WriteConfiguredValue also reopened the live config.toml with ios::trunc, so a crash mid-write destroyed the user settings and could leave the file unparseable for good. It now goes through WriteFileTextAtomically, which this file already uses elsewhere. That makes the directory entry swap atomic; it does not flush, so it is not a durability guarantee.
* fix(ipc): restrict the server pipes to the owning account
The pipe DACL granted Everyone FILE_ALL_ACCESS. That bit set includes FILE_CREATE_PIPE_INSTANCE, so any local account could add an instance to a name this server already owned and answer part of the connections in its place. The names are machine-global with no per-user component, and the TSF client opened them with no SQOS, so a squatter could also impersonate the connecting thread.
Build the descriptor from the process owner SID and admit only LocalSystem, that account, and app containers with connect-only rights so UWP text input still works; fail pipe creation outright when the SID cannot be resolved rather than falling back to something permissive. Pass FILE_FLAG_FIRST_PIPE_INSTANCE on the first instance of each name so a squatted name fails loudly, and request SECURITY_IDENTIFICATION on the client side.
Authenticating the peer process after connect is a larger change to the handshake and is not attempted here.
* fix(tsf): guard the null focus paths and unadvise the compartment sink
_CandidateChangeNotification treated ITfThreadMgr::GetFocus and ITfDocumentMgr::GetTop as if they could only fail through a FAILED HRESULT. Both return S_OK with a null out-pointer when nothing has focus or the document stack is empty, and the results were dereferenced unconditionally, so an external UILess host calling Finalize while focus moved away crashed the host application. The sibling _RequestCancelComposition in the same class already checks both.
CLangBarItemButton::CleanUp only unregistered the compartment inside the SUCCEEDED branch of a CoCreateInstance, then deleted the sink regardless. This TIP registers COM-less activation support, where that call does fail, and the sink was freed while TSF still held it advised. Unadvising never needed the thread manager, so it now runs unconditionally before the deletes.
* fix(ui): drop the cached visuals when the scene is replaced
Window caches focused, hovered and captured Visual pointers into the scene tree, and SetScene destroyed that tree without clearing them. Toggling the input mode rebuilds the floating toolbar scene, so the next mouse move dispatched a virtual call through a freed object.
Clear all three before the assignment, and release the capture only when this window actually holds it: an adopted HWND may never have called SetCapture, and capturedVisual_ is set even when it did not.
* fix(settings): scope external skin CSS through the CSS parser
injectScopedSkinCss concatenated a skin toolbar.css into an @scope block as text. Block nesting is textual, so a leading close brace in the skin ended the block early and the rest styled the whole settings window. The host validates only the skin file name, never the body, and candidatePreviewCss interpolated unvalidated colour strings into rule bodies the same way.
Create the scope rule empty and re-insert every rule parsed out of the skin as a child of it, so nothing lands outside however the braces are balanced, and accept a colour only in the plain notations we actually render.
* fix(release): gate the dictionary source commit on its default branch
verify_published iterated only data["repositories"], but the dictionary block pins a source_commit exactly like they do. The manifest check merely proves the release agrees with the lock about that commit, which a branch commit satisfies just as happily, so a dictionary built from an unmerged branch shipped with the provenance the gate exists to refuse.
Run the dictionary through the same loop under its own name, resolving its repository from the lock. A retired dictionary repository that no longer answers is reported rather than failing the release: its published artifacts are immutable history, and being unable to re-check them is not the same as finding them unmerged.
The engine pin moved 55839c8 -> 127019f in the previous commit, and that range contains 86c5fa7 "fix(quanpin): enumerate alternative segmentations for four-syllable input". QuanpinDictionaryOnlyTriesMultipleSegmentationsForAtMostThreeSyllables asserted the behaviour that fix deliberately changed, so it failed here and, through the product lock, in MSIME-Windows' locked-Server integration job as well.
The engine is the authoritative layer: quanpin/quanpin_dictionary.cpp declares kMaxSyllablesForMultipleSegmentations = 4 and gates enumeration on segments.size() <= that. Four syllables now enumerate alternatives; five still do not. The test says exactly that, with a tbl_6_x fixture row so the five-syllable case would have something to find if the bound ever moved silently.
The engine absorbed the six local mode queries during the cross-platform work,
and MSIME-Linux has been driving them in production since. This repository kept
its own implementations of the same six features, so date-time, emoji, kaomoji,
jianpin, quick-phrase and unicode each existed twice and were free to drift.
Deletes this repository's copies and points the eight call sites in
event_listener.cpp plus the two async workers at metasequoia::local_modes.
The shuangpin profile is the part to look at. The old modules resolved the
*configured* scheme themselves:
GetShuangpinProfile(shuangpin_schema.empty() ? GetConfiguredShuangpinSchema()
: shuangpin_schema)
The engine takes a resolved ShuangpinProfile and defaults it to Xiaohe. Taking
that default would have decoded J mode, emoji and kaomoji as Xiaohe for every
user on Ziranma, Shoudao or Microsoft shuangpin, so every call site passes the
configured profile explicitly through a ConfiguredShuangpinProfile() helper.
The database path needs the same care in the other direction. The old modules
took a UTF-8 std::string and handed it straight to sqlite3_open_v2; the engine
takes a std::filesystem::path and converts back with path_to_utf8 before
opening. The two async workers therefore bridge with path_from_utf8 rather than
constructing a path from the narrow string, which on Windows would interpret a
non-ASCII data directory in the active code page.
The four existing test files are kept and re-pointed at the engine functions
with every expectation byte-identical, including the seventeen date-time format
strings and the four shuangpin schemes jianpin covers. They are the equivalence
check: if the engine's port diverged from what this repository shipped, those
assertions fail rather than the difference reaching users.
Jia Sui
