Skip to content

feat(text): say so when a text parses into nothing (#278) - #282

Open
HugoFara wants to merge 1 commit into
developfrom
feat/278-warn-when-a-text-parses-to-nothing
Open

feat(text): say so when a text parses into nothing (#278)#282
HugoFara wants to merge 1 commit into
developfrom
feat/278-warn-when-a-text-parses-to-nothing

Conversation

@HugoFara

Copy link
Copy Markdown
Owner

The backstop for #278, independent of #281.

The silent failure

A language whose Word Characters setting doesn't match the script of its texts does not fail. It parses successfully into nothing:

text saved as TxID 27 — stored items:
   (none)
sentences stored: 1

TokenPersistence::save() has if (empty($tokens)) return; and returns without a word. The text saves, opens, and renders every character — and refuses to respond to any click. No exception, no log line, no message. From the outside it is indistinguishable from an ordinary text that has gone inert, which is exactly what the reporter met, and why their issue says "I don't actually know what the expected behavior is".

One rule, three surfaces

ParseCoverage is the single place that decides a parse came out empty, so the reading view, the check-text page and the API agree on when to speak up.

The test is a word-to-character ratio, not a plain zero. A non-Latin text on a Latin language usually matches a few stray tokens — a digit, a Latin fragment — and a zero test lets those through while the text is still unreadable. The floor is one word per fifty characters, well under the ~1-per-6 of Latin prose or the ~1-per-1 of a character-split script, and it only applies to texts long enough to judge (short titles and captions are exempt).

Surfaces, all ones the reader already reaches:

where why
Reading view where the confusion actually happens — a parseWarning on the payload that already feeds it
Check-text page its whole purpose is "did my parsing work?"; it answered a failed parse with an empty list
ParseText::execute() a warning field for anything reading the API

Each says what happened and links straight to /languages/{id}/edit.

Verified against a real database

Chinese on Latin defaults   ParseText.warning=no_words   reader banner: SHOWN
Chinese configured right    ParseText.warning=ok         reader banner: none
ordinary English            ParseText.warning=ok         reader banner: none

The banner reads:

None of this text could be turned into words. The language's Word Characters setting does not match this text, so nothing in it can be clicked or learned.
[Check the language settings]

Notes for review

  • The banner is built through the DOM rather than by concatenation: text_renderer.ts's escapeHtml() uses textContent/innerHTML, which does not escape quotes. That's fine for text nodes but would let an attribute value break out of its attribute, so href goes through setAttribute. A test covers it.
  • While tracing the reading path I found TextReadingService::mainWordLoop() is dead code — no callers. Its query chains select()/selectRaw(), which overwrite rather than accumulate, so it selects only the last five columns and reads six undefined keys. Not touched here; worth a separate look, along with whether the method should exist at all.
  • Four new locale keys across all nine languages.

Checks

Psalm 0 errors · PHPCS 0 errors, 0 warnings · PHPUnit 9109 pass (11 new) · Vitest 4330 pass (4 new) · tsc and ESLint clean · assets rebuilt.

One existing frontend test mocked text_renderer exhaustively and needed the new export added to its mock.

A language whose word characters do not match the script of its texts does not
fail. It parses successfully into nothing: the text saves, opens, and renders
every character, and not one of them can be clicked, looked up or learned.
TokenPersistence::save() takes the empty token list and returns silently, so a
Chinese text on a Latin language stores a row, a sentence, and zero text items.
From the outside that is indistinguishable from an ordinary text that has gone
inert, which is what the reporter met and had no way to diagnose.

ParseCoverage is the one place that decides a parse came out empty, so the
reading view, the check-text page and the API agree on when to speak up. The
test is a word-to-character ratio, not a plain zero: a non-Latin text usually
matches a few stray tokens — a digit, a Latin fragment — and a zero test would
let those through. The floor is one word per fifty characters, well under the
one per six of Latin prose or the one per one of a character-split script, and
it only applies to texts long enough to judge.

Three surfaces, all of them ones the reader already reaches:

- the reading view, where the confusion actually happens, via a parseWarning
  on the payload that already feeds it;
- the check-text page, whose whole purpose is to answer "did my parsing work?"
  and which until now answered a failed parse with an empty list;
- ParseText::execute(), for anything reading the API.

Each says what happened and links to the language's settings.

The banner is built through the DOM rather than by concatenation: the
renderer's escapeHtml() leaves quotes alone, which is fine for text but would
let an attribute value break out of the attribute it sits in.

Verified against a real database:

  Chinese on Latin defaults   warning=no_words   banner shown
  Chinese configured right    warning=ok         no banner
  ordinary English            warning=ok         no banner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant