Skip to content

Match locale names longest-first so a name that is a prefix of another parses - #130

Open
spokodev wants to merge 2 commits into
d3:mainfrom
spokodev:fix/locale-name-longest-match
Open

Match locale names longest-first so a name that is a prefix of another parses#130
spokodev wants to merge 2 commits into
d3:mainfrom
spokodev:fix/locale-name-longest-match

Conversation

@spokodev

Copy link
Copy Markdown

Problem

formatRe builds a parser for a locale field (months, weekdays, periods) by
joining the names into an ordered regex alternation:

function formatRe(names) {
  return new RegExp("^(?:" + names.map(requote).join("|") + ")", "i");
}

JavaScript alternation is first-match, not longest-match: (?:a|b|…)
returns the first alternative that matches at the position. When an earlier
name is a prefix of a later name in the same field, the parser matches the
short name first, and the leftover characters then break the parse.

This silently corrupts d3's own formatter output in shipped locales:

const cs = timeFormatLocale(/* locale/cs-CZ.json */);
cs.format("%d %B %Y")(new Date(2020, 6, 15)); // "15 červenec 2020"
cs.parse ("%d %B %Y")("15 červenec 2020");    // null  ← should be 2020-07-15

"červen" (June) is a prefix of "červenec" (July). Same failure for Turkish
"Pazar" (Sun) vs "Pazartesi" (Mon), and "Cuma" (Fri) vs "Cumartesi"
(Sat): a utcParse("%Y %U %A") round-trip over 2020 in tr-TR fails on all
104 Mondays and Saturdays. vi-VN months ("Tháng 1" vs "Tháng 10/11/12")
are affected too.

Fix

Sort the names by descending length before building the alternation, so the
longest matching name wins. formatLookup is keyed by the matched text, so the
reordering does not affect index resolution:

names.slice().sort((a, b) => b.length - a.length).map(requote).join("|")

.slice() keeps the caller's array (shared with formatLookup) unmutated.

Test

Added a test/locale-test.js case that formats and re-parses a date in the two
shipped locales with the prefix collisions (Czech July, Turkish Monday). Fails
on main, passes with the fix. Verified no regression: en-US full/short month
and weekday round-trips over all months/days still pass, and eslint is clean.

…r parses

formatRe joined a field's names into an ordered regex alternation
(?:a|b|…). JavaScript alternation returns the first alternative that
matches at the position, not the longest. When an earlier name is a
prefix of a later name in the same field, the parser matched the short
name first and the leftover characters then broke the parse.

This corrupts d3's own formatter output in shipped locales: Czech
"červen" (June) is a prefix of "červenec" (July), so
timeParse("%d %B %Y") of "15 červenec 2020" returned null; Turkish
"Pazar" (Sun) is a prefix of "Pazartesi" (Mon), so every Monday/Saturday
failed to parse.

Sort the names by descending length before building the alternation, so
the longest matching name wins. formatLookup is keyed by the matched
text, so the reordering does not affect index resolution.

@Fil Fil left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but can you please tighten the comments? They contain redundant explanations.

For example I would strip "; ordered alternation is first-match, not longest-match. formatLookup is keyed by text, so order is irrelevant to it." and " Ordered alternation matched the shorter name first, so these formatted dates failed to round-trip."

@spokodev

Copy link
Copy Markdown
Author

Trimmed both — kept the sort rationale and the example names, dropped the rest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants