Repository navigation
de, el: use $CommandOffset instead of string-length($Prefix) in navigate.yaml - #766
Merged
NSoiffer merged 1 commit intoSep 20, 2026
Merged
Conversation
…ate.yaml Ports ec36e05 to the two remaining languages that still cut the direction word out of $NavCommand with the length of the SPOKEN prefix. $NavCommand is always English (ZoomIn, MoveNext), while $Prefix is the translated word, so string-length($Prefix)+1 only lands on the right offset while the prefix happens to match the English stem. In de and el the prefixes are still the untranslated zoom/move/read/describe, so all 18 branches work today by coincidence; localizing those strings -- the obvious thing for a translator to do -- silently kills every direction word. This is a no-op right now, which is exactly why it is a good time to do it: substring($NavCommand, $CommandOffset) returns the same value as before for every one of the 15 navigation commands with the current prefixes. Verified on the engine, not only on paper: Languages::de is 209/209 both with and without this change. Greek has no tests at all (tests/Languages/el exists but is not declared in tests/languages.rs), so el rests on the structural check: offsets and the list of 9 branches are now identical to en. Refs daisy#740.
Collaborator
|
This looks good. I need to look into the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #740 point 1.
ec36e057replacedstring-length($Prefix)+1with$CommandOffset, because$NavCommandis always English (ZoomIn,MoveNext) while$Prefixis thespoken, translated word.
deandelwere the last two files still carrying theold formula; this ports the change to them and touches nothing else.
This is deliberately a no-op today, which is why it is a good moment to do it.
Both languages still use the untranslated
zoom/move/read/describeas theirPrefix, sostring-length("move")happens to equal the length of theMovestem and all 18 branches match. The first translator who localizes those strings
loses every direction word, silently.
What changed
substring($NavCommand, string-length($Prefix)+1)→$CommandOffsetin each file;
set_variablescalls now also setCommandOffset(5 for Zoom/Move/Read,9 for Describe), same values as
en.Translations are untouched: every line in the diff is one of those two shapes.
After the change the offsets and the list of 9 branches (
In,InAll,Out,OutAll,Next,Previous,Current,LineStart,LineEnd) are identical toen/navigate.yaml.Verification
Checked on the engine rather than only on paper, because the interesting property
here is "nothing changes":
Languages::de→ 209 passed, 0 failed, and the same 209/209 with thechange reverted. So the existing German tests do not discriminate this at all,
which matches the report: there are no navigation tests for
deorel.deonthe existing
init_nav/do_navigate_commandharness (the shape used byenandnb), with the expected strings read out of the engine rather thanwritten by hand. Result: 4/4 with the change, 4/4 without it — the no-op is
confirmed on live output.
(
zoom→zoomen,move→gehe) makes the OLD formula produce"zoomen; in zähler; 1"— the direction word is simply gone — while the new oneproduces
"zoomen in; in zähler; 1". Same simulation over all 15 navigationcommands: the old formula breaks 7/15 for German prefixes and 15/15 for Greek
ones.
I kept those tests out of this PR to hold it to the two files I promised in the
issue. Happy to send them separately if you want
denavigation pinned.One thing I noticed while measuring
Greek has no tests running at all.
tests/Languages/el/exists with 11 files,but
elis not declared intests/languages.rs, so cargo filters everything out(
0 passed; 5139 filtered out). Current spread: pl 625, hu 589, nb 583, en 580,fr 518, ru 513, sv 503, fi 488, zh 290, de 209, pt 174, vi 19, id 16, el 0.
So for
elthis change rests on the structural check only. Say the word and Iwill open a separate issue for that — it looked like an oversight rather than a
decision.
Point 2 of the issue (comparing
$NavCommanddirectly, the wayrudoes) is notpart of this PR; it changes all languages, so it deserves its own decision.