Repository navigation
OPENNLP-1933: Regex removal (5a/10): Simplify dictionary and filename parsing - #1320
Merged
Merged
Conversation
This was referenced Sep 22, 2026
Merged
rzo1
requested changes
Sep 22, 2026
rzo1
left a comment
Contributor
There was a problem hiding this comment.
Blocking
- spellcheck.xml:317-321. The manual now says counts may use Arabic-Indic and fullwidth digits and an optional sign. That's only what
Long.parseLonghappens to accept, and once it's in the manual, users will rely on it and we'll have to keep supporting it. SymSpell dictionaries use ASCII digits. Either restrict the count to ASCII digits (StringUtil.isAsciiDigitexists) or drop the sentence and just say "a nonnegative integer".
Minor
- FrequencyDictionaryLoader.java:254 vs :240, :273, :287.
isSkippablebecame non-static, whilestripBom,parseCountandsaturatedAddstay static. Either drop that change or align all. - FrequencyDictionaryLoader.java:274.
raw.trim()isn't needed any more. A column can no longer contain a space or tab, sotrim()only strips control characters, andthe<TAB>\u00015is accepted with count 5. Drop thetrim(). - FrequencyDictionaryLoader.java:214-238.
splitColumnsis the same "split and skip empty fields" loop assplitClassPath(#1278) andEventFields.split(#1321). Please add one helper toStringUtiland use it in all three.COLUMN_TAB/COLUMN_SPACE(:73-74) then become arguments to that helper; right now they're the only constants without Javadoc. - FrequencyDictionaryLoader.java:240.
BOM/stripBomduplicates the byte-order-mark handling in #1277 (AbstractDL,JsonScan) andHunspellDictionary. A sharedStringUtil.stripByteOrderMarkwould cover all of them. - spellcheck.xml:306-313. Too much detail for the manual:
String.strip(), vertical tab, form feed, em space, no-break space. Say "columns are separated by TABs or runs of spaces; blank lines and lines starting with#are skipped", keep the SymSpell link, and drop the rest. - langdetect.xml:181-184. Same here: "The language is taken from the first three characters of the file name, the lowercase ISO 639-3 code, e.g.
eng_news_2005_10K-sentences.txt." is enough. The list of rejected prefixes can go. - Description. "Hosted CI has not run for this split" is outdated, since CI is green. Please also remove "We had a complicated ticket…" and the "AI assisted text" block, and keep only what changes for users.
krickert
added a commit
to ai-pipestream/opennlp
that referenced
this pull request
Sep 29, 2026
…ache#1320) (#22) parseCount scans the column by code point with Character.digit, so a count in decimal digits of any script loads, a sign is rejected with "count must not be negative" after the digits are checked, and an overflow is "count is not an integer". Squash of #22, reviewed by Kristian Rickert.
This removes regex from Leipzig file name checks, Morfologik suffix handling and frequency dictionary parsing. It is part 5a of the regex removal series. AI assisted summary - AI edited for accuracy and I edited for clarity: > Leipzig file name checks scan for a three-letter lowercase ISO 639-3 > prefix. Morfologik replaces ".info" only at the end of a file name. > Frequency dictionary columns are split on tabs or runs of spaces. > Blank lines and lines starting with "#" are skipped, and Unicode terms > are kept as written. > Counts must be nonnegative integers. Character.digit reads decimal > digits from any script by code point. Signs, decimal points, exponents > and overflow are rejected, with the line number in the error message. > Tests cover file name boundaries, separators, Unicode terms, counts > in several scripts and malformed input. The manual explains the formats > and links to the SymSpell dictionary specification. Assisted-by: Claude Code 2.1.281 Co-authored-by: Kristian Rickert <kristian@apache.org> Co-authored-by: Claude <noreply@anthropic.com> Reviewed-by: Kristian Rickert <kristian@apache.org> Signed-off-by: Kristian Rickert <kristian@apache.org>
krickert
force-pushed
the
OPENNLP-1933-dictionary-regex
branch
from
September 29, 2026 02:01
c41339c to
0026ca1
Compare
Contributor
Author
|
I replaced regex in Leipzig file name checks, Morfologik suffix handling and frequency dictionary parsing with direct character checks. It matches only a trailing .info suffix. Split frequency columns on tabs or spaces, preserve Unicode terms and digits, and report line numbers for invalid counts. I added tests for file names, separators, Unicode input and invalid counts. Documented the dictionary formats in the manual. LMK if I need to do anything else. |
rzo1
approved these changes
Sep 29, 2026
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.
Removes regex operations from Leipzig filename checks, Morfologik dictionary suffix handling, and frequency-dictionary column parsing. We had a complicated ticket on the fifth of ten regex removal, so this is part of that effort per @rzo1's guidance
Details:
.infosuffix.Long.parseLong, including Arabic-Indic and fullwidth digits.AI assisted text: