Skip to content

de, el: use $CommandOffset instead of string-length($Prefix) in navigate.yaml - #766

Merged
NSoiffer merged 1 commit into
daisy:mainfrom
michaldziwisz:fix-navigate-command-offset-de-el
Sep 20, 2026
Merged

NSoiffer merged 1 commit into
daisy:mainfrom
michaldziwisz:fix-navigate-command-offset-de-el

Conversation

@michaldziwisz

Copy link
Copy Markdown
Contributor

Closes #740 point 1.

ec36e057 replaced string-length($Prefix)+1 with $CommandOffset, because
$NavCommand is always English (ZoomIn, MoveNext) while $Prefix is the
spoken, translated word. de and el were the last two files still carrying the
old 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/describe as their
Prefix, so string-length("move") happens to equal the length of the Move
stem and all 18 branches match. The first translator who localizes those strings
loses every direction word, silently.

What changed

  • 9 uses of substring($NavCommand, string-length($Prefix)+1) → $CommandOffset
    in each file;
  • the 4 set_variables calls now also set CommandOffset (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 to
en/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 the
    change reverted
    . So the existing German tests do not discriminate this at all,
    which matches the report: there are no navigation tests for de or el.
  • To get a real measurement I wrote four throwaway navigation tests for de on
    the existing init_nav / do_navigate_command harness (the shape used by
    en and nb), with the expected strings read out of the engine rather than
    written by hand. Result: 4/4 with the change, 4/4 without it — the no-op is
    confirmed on live output.
  • Validity control, so those tests are not vacuous: translating the prefixes
    (zoom → zoomen, move → gehe) makes the OLD formula produce
    "zoomen; in zähler; 1" — the direction word is simply gone — while the new one
    produces "zoomen in; in zähler; 1". Same simulation over all 15 navigation
    commands: 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 de navigation pinned.

One thing I noticed while measuring

Greek has no tests running at all. tests/Languages/el/ exists with 11 files,
but el is not declared in tests/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 el this change rests on the structural check only. Say the word and I
will open a separate issue for that — it looked like an oversight rather than a
decision.

Point 2 of the issue (comparing $NavCommand directly, the way ru does) is not
part of this PR; it changes all languages, so it deserves its own decision.

…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.
@moritz-gross moritz-gross added rules Pertains to Rules translation Language translation of math/code labels Sep 16, 2026
@NSoiffer

Copy link
Copy Markdown
Collaborator

This looks good.

I need to look into the el tests. You are correct that it should be part of languages.rs. However, the el.rs file has all the lines commented out. Hopefully they all pass. I'll give them a try.

@NSoiffer
NSoiffer merged commit 5c8117e into daisy:main Sep 20, 2026
10 checks passed
@github-project-automation github-project-automation Bot moved this from Triage to Done in MathCAT Project Board Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rules Pertains to Rules translation Language translation of math/code

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

de and el navigate.yaml still use string-length($Prefix), which breaks as soon as their prefixes are translated

3 participants