Multiple linux bug fixes - #15
Conversation
Update to latest upstream
Removed incomplete bullet point from requirements section.
…hown language when the dropdown box was selected to something other than russian. (It used to stay russian despite the selection.) Added a function to find the users locale and use that as the shown language first, fallsback to russian if the locale can't be read or doesn't map the three supported languages.
Fixed an issue where the interface language would not update in real time to what the user just saved, no longer requires the app to be restarted to see changes.
|
Rechecking for the lint and test error |
Guessing the interface language from the locale was done inside the block that opens the setup wizard, and that block runs on more than a first run: it also runs whenever no provider is configured, which an expired key or a config written before the provider registry will do. A config file that old still names a language its owner chose. The welcome page seeds its dropdown from whatever is on screen and finishing writes that back, so a user who picked Spanish and clicked through would have found it saved as German. The rule is now one function, startup_ui_language, asked by both entry points: a config file that exists decides, and only a machine that has never run the app is asked what language it speaks. That also gives Windows the first-run guess, which it did not have. The guess itself moved to app/i18n.py, next to UI_LANGUAGES — the list of supported languages it had been carrying a fourth copy of, and where it can be tested at all: it was in main_gtk, which imports gi at module level and therefore cannot be imported on CI. It reads the environment in gettext's order now, LANGUAGE first rather than last, and treats that variable as the colon-separated preference list it is. Smaller things from the same change: the resize grip is relabelled along with the rest of the overlay; three widget references nothing reads are gone; and a provider validation that finishes after a language switch no longer pokes the button the rebuild destroyed. Version 3.5.0, and the CI test floor rises with the 25 tests added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks — this is a real gap and three of the four things you fixed had been broken since the GTK frontend landed. I've reviewed it and pushed a commit onto this branch (09dd423) rather than sending you round again for something I could do in one pass. Everything below is either a defect I found or a reason for a change I made; nothing here is a request for more work from you. The one defect
if not os.path.exists(CONFIG_FILE) or not any_configured(config.providers):The second half is not a first run. It's true whenever no provider is configured — an expired key does it, and so does a config file written before providers became a registry in 3.4.0. Such a file exists, and it names a language its owner chose. Guessing there isn't just a mislabelled window: the welcome page now seeds its dropdown from The condition is now split, and the rule lives in one function both entry points call: def startup_ui_language(*, config_exists: bool, saved: str) -> str:
return saved if config_exists else guess_ui_language()What else changed, and whyThe guess moved to Environment variable order. You had Windows gets the first-run guess too. Three small ones in your diff. Two things I left alone
The Qt overlay still doesn't relabel itself. Released as 3.5.0New behaviour (locale detection), so a minor bump: |
Two defects a review of the previous commit found, both in code added there
and neither shipped.
locale.getlocale() cannot answer this on Windows. It reports the C runtime's
name for the locale — measured here, ('Russian_Russia', '1252') — which is
not an ISO code, so the guess fell through to the default and every Windows
first run opened in Russian, which is the thing the change was written to
stop. GetUserDefaultUILanguage through locale.windows_locale answers with
ru_RU, and it is also the better question: Windows keeps the language the
interface is in separate from the locale dates and numbers are formatted by,
and the first of those is what we are matching. The existing test passed
only because it mocked the POSIX shape Windows never produces.
The other: startup_ui_language was handed os.path.exists(CONFIG_FILE), and
AppConfig.load reads config.json.bak as well. With only the backup on disk,
load hands back a real saved ui_language that the caller one line later
discards as a first run — and on GTK the wizard opens too, seeds its
dropdown from the guess and writes it back on finish, so the recovered
choice is gone for good. That is exactly what the docstring claims the
function prevents. It is config.py's question now: saved_config_exists
walks the same candidate list load does, so the two cannot come to look at
different files, and it parses rather than stats, which also gets the
corrupt-config case the right way round.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Reviewed my own commit above and found two defects in it. Both are fixed in 53a5aa0; neither ever shipped, so 3.5.0 stays 3.5.0. The locale guess didn't work on Windows at all — including for the case that motivated it. So
It's def saved_config_exists(path: str = CONFIG_FILE) -> bool: ...sharing Twelve tests added for the two, the fixture now neutralises the Windows lookup as well (CI runs on an English Windows runner, so leaving it live would have had the runner answering the "no locale anywhere" tests), and there's an AST guard so neither entry point can go back to statting the config path. 1126 passing, 3 skipped; ruff clean. |
The version was wrong. Nothing here is new: the app has offered three
interface languages since long before this branch and opened in one of them
regardless, on both platforms. Fixing that is a patch, so the changelog
entry loses its Added section along with the number, and the two bullets
describing defects that only ever existed inside this branch go with it —
a changelog is for what changed for the people running it.
Three things a review of the finished branch turned up:
tr.set_language validated against a hardcoded ("RU","EN","ES") while the
locale guess resolved against UI_LANGUAGES. A fourth translation added to
the table would be found by the guess and then silently clamped back to
Russian on the way in — this branch's own bug, wearing a new hat. One table
now, and a test that walks it.
_start_pipeline_if_ready is gone. It could not fire (control only reaches it
after the wizard, and _finish always writes the keyless provider, so
any_configured is true by then) and it would have been wrong if it could:
TranslationPipeline builds its TranslatorService in __init__ and
update_config never rebuilds it, so the very case it existed for — a first
provider configured while the app runs — would have started a pipeline that
cannot translate. pipeline.start() is unconditional again, as on Qt.
The settings window now closes only when the language changed. That is the
one save that needs the rebuild; closing on every save meant the "Saved"
written two lines earlier was never on screen long enough to read.
Test floor tightened to the number CI actually collects, rather than left
ninety short of it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The live language change was fixed on GTK and described in the changelog without qualification, which left every Windows user reading about a repair they had not been given: open_settings applied opacity and nothing else, so the ON/OFF badge, the Settings button, the opacity label, the reply placeholder, the Copy button and the filter tabs all stayed in the language they were built in until the app was restarted. ChatOverlay.apply_language does for Qt what its GTK counterpart already did. Two of the widgets it needs were locals in the chrome builder and are kept on the overlay now. The badge is relabelled from its own state rather than from the string table alone, so a running translation cannot be shown as stopped. The reply panel's status line is deliberately left out: it is rewritten on every action, so it arrives in the new language by itself, and touching it here would overwrite whatever it is currently saying. Also the last hardcoded copy of the language table, in the GTK wizard. A fourth translation added to UI_LANGUAGES and missed there would leave the dropdown falling back to its first entry and _finish persisting that over the language the locale guess had got right — the same silent clamp removed from tr.set_language in the previous commit, one file over. The tests read the source rather than driving the widgets, which is weaker and is written down as a choice: constructing ChatOverlay under pytest kills the interpreter outright, which is why nothing else in the suite instantiates it. What they do hold is the part that rots — a label added to the chrome and forgotten in the refresh fails the run. The behaviour itself was verified by hand, off-screen, against a live overlay: five chrome labels and thirteen filter tabs followed the switch, and the badge kept its state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two windows the previous commit did not reach, found by reviewing it. The clipboard reply dialog is a separate window, created on demand by the hotkey and then kept, so it outlives the setting changed after it — being separate is exactly why it was missed. Its placeholder and Copy button relabel now; the status line and output field are written on every action and arrive in the new language by themselves. The tray menu is worse, because it is the one window a user cannot close and reopen to get the new language: built once at startup and never touched again. Its five items were locals except the two that already needed keeping. The first item is written from state, not from the table alone — it says Hide or Show depending on where the overlay is, and relabelling it unconditionally would offer to hide a window that is already hidden. Verified by hand against live widgets, off-screen, the same way as the overlay: both dialog labels and all five tray items follow the switch, and the Hide/Show item keeps its state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main.py crossed the five-hundred-line limit on the last commit, and the hundred and thirty lines it was carrying for this had nothing to do with the job the rest of the file does. Wiring the application together is one thing; deciding whether another copy of it is already running is another, and the second is the half with the sharp edge, because it terminates a process. Nothing about it changed. Its tests were already a file of their own and already named after it — they point at the module now instead of reaching through main, which as a side effect lets them run where PyQt6 is not installed, since the guard never needed Qt. main.py: 504 lines to 370. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…typed Two asymmetries a final review found, both of the same kind as the rest of this release: one frontend was fixed and the other was described as fixed. Changing the language in the Qt wizard restarts it, and the new one builds its fields from the config — so anything not written to the config first is simply gone. An API key pasted on page two, the WoW path browsed for on page three: both discarded, silently, by the one control on page one. The GTK wizard got a snapshot and restore in this branch and the changelog describes the carry-across without naming a platform. The restart writes the two fields first now, the same two calls _finish makes. The GTK tray menu was five English literals. The keys have existed as long as the Qt tray has been using them; the menu simply never went through tr(), so a Russian player got a Russian overlay above an English tray whatever they picked. It is translated now and relabelled on save, because the tray is built once at startup and there is no reopening it to get the new language. Its first item is written from state rather than the table alone, so it cannot offer to hide a window that is already hidden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The entry was written when the live language change was fixed on GTK only, and three more repairs landed after it: the Qt overlay and the clipboard window relabel themselves, the tray menu is translated and refreshed on both platforms — on Linux it had never gone through tr() at all — and neither setup wizard drops what was typed into it any more. Five bullets instead of two, and none of them describes as general something done on one platform. That was the failure this whole release kept repeating; it would be a poor place to repeat it one last time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Merged and released as v3.4.1. Thank you — this started as four Linux fixes and turned out to be the thread that unravelled the same defect on four different windows. First, a correction to my two earlier comments: the version is 3.4.1, not 3.5.0. I bumped the minor number on the grounds that locale detection was a new feature. It is not: the app has offered three interface languages since long before this branch and opened in Russian regardless, on both platforms. Fixing that is a patch. The changelog entry lost its Added section along with the number, and lost two bullets describing defects that only ever existed inside this branch — a changelog is for what changed for the people running it. What your PR turned upThe defect you reported on Linux — the wizard not following its own dropdown — had three siblings, all of the same shape: one frontend fixed, the other described as fixed.
The one that would have hurt most was in the first version of my own fix: Notes on your diff specificallyEverything you wrote is in the release. Three things I changed and one I removed:
Four review passes, 1015 → 1146 tests, and the pattern is now written into the suite: anything both frontends do is asserted against both modules, so the next half-fix fails the build instead of the changelog. Released to GitHub, CurseForge and Wago. Nothing further needed from you — thanks again for finding the loose thread. |
Fixed an undeclared variable
Fixed the interface language not updating to what the user saved in settings in real time
Fixed an issue where the setup wizard on linux would not update the shown language when the dropdown box was selected to something other than russian. (It used to stay russian despite the selection.)
Added a function to find the users locale and use that as the shown language first, fallsback to russian if the locale can't be read or doesn't map the three supported languages.