Repository navigation
LT-22784: Add Tools > Apply Motif changes; remove the AI export - #1162
johnml1135 wants to merge 10 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1162 +/- ##
==========================================
- Coverage 39.49% 39.49% -0.01%
==========================================
Files 1537 1535 -2
Lines 354392 354562 +170
Branches 41058 41100 +42
==========================================
+ Hits 139974 140018 +44
- Misses 185044 185146 +102
- Partials 29374 29398 +24
🚀 New features to boost your workflow:
|
NUnit Tests 1 files ± 0 1 suites ±0 13m 7s ⏱️ + 2m 45s Results for commit 63e5290. ± Comparison against base commit a90754c. This pull request removes 22 and adds 71 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
johnml1135
changed the title
Comment hygiene (advisory)No comment-style violations in the lines this branch adds since |
jasonleenaylor
left a comment
jasonleenaylor
left a comment
There was a problem hiding this comment.
Looks like a positive next step. Two changes inline (a timeout for Motif, and the Webonary switch the revert undid), plus a question and two nits.
This review was assisted by Claude Opus 5.5.
| { | ||
| var outputTask = process.StandardOutput.ReadToEndAsync(); | ||
| var errorTask = process.StandardError.ReadToEndAsync(); | ||
| process.WaitForExit(); |
There was a problem hiding this comment.
WaitForExit() has no timeout, and this runs on the UI thread (OnApplyMotifChanges runs workflow.Execute() under a WaitCursor) after the project has been unlocked, with Motif's window hidden by CreateNoWindow. If Motif stalls, FLEx sits at "Not Responding" with the project unlocked and nothing the user can close. Send/Receive at least has a visible Bridge window and a watchdog.
SIL.CommandLineProcessing.CommandLineRunner (SIL.Core, which FwUtils already references) takes an encoding and a timeout and reports DidTimeOut; XhtmlDocView.cs:926 uses it under a WaitCursor the same way. On timeout, kill Motif and classify the result as Ambiguous so it reloads. It would also replace the hand-rolled stream draining.
While you're in here, I'd move the command out of FLExBridgeListener. Motif isn't FLEx Bridge; it lives there to reach that listener's private save/unlock/reopen steps (ValidatePendingEditorData, StopParser, GetFullProjectFileName, RefreshCacheWindowAndAll) and its global registration. A MotifListener registered in Main.xml, with those steps pulled into a helper both listeners call, would read better, and it avoids using this listener after the reload has disposed it (the reload lambda sets _parentForm, then ShowMotifApplyMessage runs on the old listener).
Please also make sure nothing between the unlock and the relock-or-reload can throw past them. FindExecutable() runs outside Apply's try, and a quoted MOTIF_DIR makes Path.Combine throw on .NET Framework; a failed relock followed by a failed reload does the same.
There was a problem hiding this comment.
Fixed in 1f1b25e, c27dbf3 and 63e5290.
- Timeout.
RunProcesswaits ten minutes (FW_MOTIF_APPLY_TIMEOUT_SECONDS, 60 to 7200), then kills Motif and waits a few seconds to confirm it exited. The result isTimedOut, which reloads. If Motif can't be stopped, FieldWorks neither relocks nor reopens, so the two programs never write the file at the same time. It also stops waiting for output a child process keeps open. CommandLineRunner. I kept the existing runner. In the SIL.Core version we pin (18.0.0-beta0030),CommandLineRunneralso blocks the calling thread. It sets the encoding on stdout only, can miss the last of the output after the process exits, and can't tell us whether the kill worked.- Listener. The command is in
MotifListener, registered inMain.xml. The steps it shares withFLExBridgeListener(committing edits, stopping the parser, reopening) are inProjectLifecycle. The handler captures the window, cache and app before it starts, and shows its result on the reopened window. - Throw paths. Motif is resolved before the unlock, and a quoted
MOTIF_DIRcounts as not installed. If a save fails after the lock was released, the project is locked again. If waiting fails after Motif started, FieldWorks stops it before deciding anything.MotifApplyWorkflownow returns a failed save, a Motif failure, a missing project file and a failed reopen as results instead of throwing them. Tests cover each case, and they replaceReloadFailurePropagates.
FieldWorks still waits on the UI thread, as Send/Receive does, but the wait is now bounded. A responsive dialog with Cancel is safe only once Motif applies to a copy and FieldWorks keeps its lock. That needs a Motif CLI change, and it's planned as a follow-up; I'll link it here once it's filed.
FwUtilsTests (including real-process timeout tests) and LexEdDllTests pass. I also ran it by hand in a debug FieldWorks:
- Against the real Motif CLI, it reported "no Motif changes waiting", relocked, and closed cleanly.
- Against a stand-in
motif.exethat hangs, with a 60-second limit, Motif was stopped after about 70 seconds, the project reopened, and the time-out message showed.
|
|
||
| private bool IsMotifAvailable() | ||
| { | ||
| if (!_motifAvailable.HasValue) |
There was a problem hiding this comment.
Question: since this is cached for the window's lifetime, someone who installs Motif while FieldWorks is open won't see the command enabled until they restart. Probably fine, but it may be worth having the Motif installer tell users to restart FieldWorks.
There was a problem hiding this comment.
Rather than asking for a restart, I dropped the cache in 66a51c6. XCore asks for this item only when it builds the Tools menu and each time Tools opens, not on idle. Send/Receive already checks for FLEx Bridge on every call the same way. The click checks again before confirming, so an uninstalled Motif can't unlock the project for nothing. CommandFollowsMotifBeingInstalledAndRemovedWhileFieldWorksIsOpen runs one listener through absent, installed and removed. I also checked it in a running FieldWorks: adding and then removing the HKCU\Software\SIL\Motif CliPath value enabled and then disabled the command, without a restart. The cost is one registry read and a file check each time Tools opens, so a MOTIF_DIR on a slow network drive would delay the menu.
Co-Authored-By: GPT-6 Luna <noreply@openai.com>
Run Motif at the save boundary and restore the cache according to its result. Co-Authored-By: GPT-6 Luna <noreply@openai.com>
This reverts commit 11a2396 (#1070). It removes the experimental "Export Grammar and Texts for AI Analysis" option behind FLEX_AI_EXPORT, its text picker and instructions file, the bundled Docs/ai-parser-help guide, and the EnvironmentVariables helper that only this feature used. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reverting #1070 restored doc comments wider than the 98-column comment limit. Rewrap them; the wording is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Motif installs per user, and its install hook records the CLI at HKCU\Software\SIL\Motif, value CliPath. The resolver read HKLM value InstallationDir, which no Motif package writes, so an installed Motif was reported missing unless MOTIF_DIR was set. MOTIF_DIR still wins; a recorded path whose file is gone counts as not installed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reverting #1070 also undid changes that were not part of the export. Restore them as they are on main: - EnvironmentVariables and its tests, shared by the interaction-trace flags, the Avalonia opt-in and Webonary. WEBONARY_API=0, false or off had turned the JSON API back on. - UIModeGates and its tests, back on the shared helper. - CONTEXT.md: the "Texts & Words" area name and the Grammar and Word occurrence glossary entries. - The /.review/ ignore rule and the UTF-8 BOM on Main.xml. Add a Webonary test that reads WEBONARY_API through UseJsonApi. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Motif is not FLEx Bridge, so its command gets its own listener, registered in Main.xml. The steps both listeners take around an external edit -- committing pending edits, stopping the parser, and reopening the project -- move to ProjectLifecycle. Reopening disposes the listener that started it, so the handler now captures what it needs first and shows its result on the reopened window instead of reusing the old listener. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Motif ran on the UI thread with no time limit while the project was unlocked, so a stalled Motif froze FieldWorks with nothing to close. - Wait at most ten minutes (FW_MOTIF_APPLY_TIMEOUT_SECONDS, 60 to 7200), then kill Motif and reopen the project, since it may have changed. If Motif cannot be stopped, neither relock nor reopen. - Stop waiting for output a child process still holds open. - Resolve Motif before unlocking. A malformed MOTIF_DIR or registry path counts as not installed instead of throwing. - A save failure stops before Motif starts. A Motif failure is Ambiguous. A failed reopen, or a missing project file, is reported instead of thrown, and a missing file is not reopened. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The listener cached whether Motif was installed for the window's lifetime. Installing Motif while FieldWorks was open left the command disabled until a restart, and uninstalling it left the command enabled. XCore asks for this item only when it builds or opens the Tools menu, so check each time, as Send/Receive does for FLEx Bridge. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- If waiting for Motif fails after it started, kill it and confirm it exited. If it is still running, neither relock nor reopen. - Check that the project file exists before relocking as well as before reopening, so a missing file is reported either way. - If saving fails after the lock was released, lock the project again. - Log the exceptions that become an Ambiguous result. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ae0e20d to
63e5290
Compare
jasonleenaylor
left a comment
jasonleenaylor
left a comment
There was a problem hiding this comment.
Thanks, both required changes are fixed. One question and one nit inline, neither blocking.
| using (new WaitCursor()) | ||
| workflowResult = workflow.Execute(); | ||
|
|
||
| ShowMotifApplyMessage(workflowResult.ReopenedWindow ?? owner, workflowResult.Message, |
There was a problem hiding this comment.
Non-blocking question: when Motif can't be stopped, the project file is missing, or the reopen fails, the project stays unlocked and this shows the message on the original window, which is still live and editable. Until the user closes FieldWorks, an edit can still be written to the .fwdata, and with Motif still running that's the two-writers case again. Would closing the window, or making it read-only, on those outcomes be safer than relying on the message?
| <command id="CmdChooseLangProject" label="_Open..." message="ChooseLangProject" icon="OpenProject" shortcut="Ctrl+O"/> | ||
| <!-- Send/Receive commands. See "ContextHelp.xml" file for corresponding tooltips. --> | ||
| <command id="CmdFLExBridge" label="Send/Receive _Project (with other FLEx users)..." icon="syncSendReceive" message="FLExBridge" /> | ||
| <command id="CmdApplyMotifChanges" label="_Apply Motif changes..." message="ApplyMotifChanges"/> |
There was a problem hiding this comment.
Nit: this sits in the Send/Receive commands block, whose comment points to ContextHelp.xml for tooltips, but it's a Tools menu item and has no tooltip there.
3 participants
Linguists can review parser results in Motif, a separate tool for checking a project's analyses. Motif lets them approve or disapprove analyses and add new ones. Until now, the only way to get those decisions back into the FieldWorks project was to close FieldWorks and run Motif against the project.
This adds Tools > Apply Motif changes…. It is available in every area, like Send/Receive. FieldWorks asks for confirmation, saves the project, and releases its lock. It then runs Motif once, the same way it runs FLEx Bridge:
What happens next depends on Motif's result:
If relocking fails, FieldWorks reloads instead, so it never keeps editing a project that Motif may have written to. A wait cursor shows while Motif runs.
Time limit and failures. Motif gets ten minutes, which
FW_MOTIF_APPLY_TIMEOUT_SECONDScan change to anything from 60 to 7200 seconds. After that FieldWorks kills Motif, waits a few seconds to confirm it stopped, and reloads. Nothing between the unlock and the relock or reload can throw past them:Motif is found before the project is unlocked.
Finding Motif.
MOTIF_DIRenvironment variable, a folder containingmotif.exe.CliPathunderHKCU\Software\SIL\Motif, the full path tomotif.exe. Motif installs per user, and its install hook writes this value and its uninstall removes it. A recorded path whose file is gone counts as not installed.FieldWorks never searches
PATHand writes nothing to the registry. If Motif is not installed, the command is disabled. The lookup isn't cached: FieldWorks checks each time the Tools menu opens and again on click, like Send/Receive checks for FLEx Bridge, so installing or removing Motif shows without a restart. A malformedMOTIF_DIRor registry path counts as not installed.FieldWorks references nothing of Motif's. It reads the exit code and a few stable JSON fields,
ok,applied,summaryandcode, and ignores anything else, so Motif can extend its output without breaking FieldWorks. There is no Undo for applied changes. The confirmation doesn't promise one.Code.
MotifExecutableResolverandMotifApplyRunnerare in FwUtils. Discovery and the process are injected.MotifApplyWorkflowholds the save, run, and reload-or-relock decisions, plus message selection. It returns failures instead of throwing them.MotifListenerhandles the command and is registered inMain.xml. The steps it shares withFLExBridgeListener(committing pending edits, stopping the parser, reopening the project) are inProjectLifecycle. Reopening disposes the listener, so the handler captures what it needs first and shows its result on the new window.LexEdStrings.resx, with translator comments.Tests. They need no Motif installation.
MOTIF_DIR, and not found.MotifListenerhandles the command.motif.exethat never exits, withFW_MOTIF_APPLY_TIMEOUT_SECONDS=60, Motif was stopped and the project reopened, with the time-out message. Nomotif.exewas left running.CliPathregistry value while FieldWorks was open enabled and then disabled the command.The matching Motif change, which releases
apply --all-pendingand gives the result an exit code, is in Motif's repository. It will be available before this is used outside development.Also removes the AI grammar and texts export. A separate commit reverts #1070 (11a2396). It removes the experimental "Export Grammar and Texts for AI Analysis" option behind
FLEX_AI_EXPORT, its text picker and instructions file, and the bundledDocs/ai-parser-helpguide. #1070 also carried changes unrelated to the export. A later commit puts those back as they are on main:EnvironmentVariableshelper, used byWEBONARY_API, the Avalonia opt-in and the interaction-trace flagsCONTEXT.mdglossary entries/.review/ignore ruleMain.xmlNext step. A follow-up will have Motif apply to a copy of the project, which FieldWorks then publishes only on success. The original then never leaves FieldWorks' lock, and Cancel becomes safe to offer. That needs a Motif CLI change, and it will be linked here once it's filed.
Jira: LT-22784.
🤖 Generated with Claude Code
This change is