[
https://issues.apache.org/jira/browse/OPENNLP-1929?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18119510#comment-18119510
]
ASF GitHub Bot commented on OPENNLP-1929:
-----------------------------------------
rzo1 commented on PR #1280:
URL: https://github.com/apache/opennlp/pull/1280#issuecomment-5838121260
Thanks, the earlier points are addressed. I'm fine with the hard break
instead of keeping a `Pattern`. Maintainer edits don't work on the
`ai-pipestream` fork, so here is a patch with the small fixes:
- `README.md`: the "Migrating from 2.x to 3.x" section claimed there are no
breaking changes. It now lists this one under "Breaking changes". #1278 adds
the same section, so whichever PR merges second only adds its bullet.
- `BasicContextGenerator`: the `@param sep` doc is reduced to its
constraints, since the rationale is already covered by `@throws`.
- `BasicContextGeneratorTest`: dropped the 2.x history comment.
Apply it with `git am 1280.patch`, and please don't force-push over it
without applying it first.
<details>
<summary>1280.patch</summary>
```diff
From 9f1049aabcb8819d1633c1b21959efacf7e61a70 Mon Sep 17 00:00:00 2001
From: Richard Zowalla <[email protected]>
Date: Fri, 25 Sep 2026 21:09:29 +0200
Subject: [PATCH] OPENNLP-1929: Document the breaking change and trim
commentary
- README: list the literal separator and the backslash rejection under
breaking changes in the 2.x to 3.x migration section.
- BasicContextGenerator: reduce the sep parameter doc to its constraints.
- BasicContextGeneratorTest: drop the 2.x history comment.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---
README.md | 6 +++++-
.../java/opennlp/tools/ml/maxent/BasicContextGenerator.java | 6 ++----
.../opennlp/tools/ml/maxent/BasicContextGeneratorTest.java | 2 --
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/README.md b/README.md
index 5a40f38e4..e02bd2598 100644
-
> RegEx removal part 7: BasicContextGenerator splits on the separator as a
> regular expression
> -------------------------------------------------------------------------------------------
>
> Key: OPENNLP-1929
> URL: https://issues.apache.org/jira/browse/OPENNLP-1929
> Project: OpenNLP
> Issue Type: Bug
> Components: Machine Learning
> Reporter: Kristian Rickert
> Assignee: Kristian Rickert
> Priority: Major
> Fix For: 3.0.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> {{BasicContextGenerator(String sep)}} documents its argument as a "separator
> character", but {{getContext}} calls {{String.split(separator)}}, which
> compiles the argument as a regular expression on every call.
> Consequences for a caller who follows the Javadoc:
> * {{"|"}}, {{"."}}, {{"+"}}, {{"*"}}, {{"?"}} split on every character or on
> nothing.
> * {{"("}} or {{"["}} throw {{PatternSyntaxException}} at prediction time, not
> at construction.
> * An empty separator splits the input into single characters.
> * If an application takes the separator from configuration, a crafted pattern
> is re-evaluated on every context string, which is a backtracking exposure.
> No code in the repository calls the separator constructor; it arrived with
> the maxent modularization (OPENNLP-1740) and has no test.
> Fix: split on the literal separator with a cursor scan, keeping
> {{String.split}} trailing-empty semantics; reject an empty separator with
> {{IllegalArgumentException}} at construction; state in the Javadoc that the
> separator is literal. Tests: default space, a metacharacter separator, a
> multi-character separator, leading and trailing separators, empty and null
> separator.
> Compatibility: external code that intentionally passes a pattern such as
> {{\s+}} relied on undocumented behavior; 3.0 is the release to correct it.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
