LUCENE-9776: Hunspell: allow to inflect the last part of COMPOUNDRULE compounds #2397
Conversation
This file contains 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
donnerpeter
commented
Feb 18, 2021
| @@ -397,8 +398,7 @@ private boolean checkCompoundRules( | |||
| if (forms != null) { | |||
| words.add(forms); | |||
|
|
|||
| if (dictionary.compoundRules != null | |||
There was a problem hiding this comment.
null check is done by the caller of this method
donnerpeter
commented
Feb 18, 2021
| IntsRef forms = dictionary.lookupWord(wordChars, start, length); | ||
| if (forms == null) return false; | ||
| IntsRef ref = new IntsRef(new int[1], 0, 1); | ||
| words.add(ref); |
There was a problem hiding this comment.
reuse the "forms" object at the stack top, change its contents for each root candidate
donnerpeter
commented
Feb 18, 2021
| // we can't add this form, it only belongs inside a compound word | ||
| if (!context.isCompound() && dictionary.hasFlag(entryId, dictionary.onlyincompound)) { | ||
| continue; | ||
| if ((context == WordContext.COMPOUND_BEGIN || context == WordContext.COMPOUND_MIDDLE) |
There was a problem hiding this comment.
The "compoundforbid" check extracted (and the contexts enumerated), everything else went into an extracted method, since the same logic was needed in applyAffix
donnerpeter
commented
Feb 18, 2021
| @@ -540,8 +533,8 @@ private boolean isAffixCompatible( | |||
| if (!isPrefix && dictionary.hasFlag(append, dictionary.compoundForbid)) { | |||
| return false; | |||
| } | |||
| WordContext allowed = isPrefix ? WordContext.COMPOUND_BEGIN : WordContext.COMPOUND_END; | |||
| if (context != allowed && !dictionary.hasFlag(append, dictionary.compoundPermit)) { | |||
| if (!context.isAffixAllowedWithoutSpecialPermit(isPrefix) | |||
There was a problem hiding this comment.
This check becomes a bit more complicated with the addition of a new context, so it's extracted to a method
donnerpeter
commented
Feb 18, 2021
| @@ -550,18 +543,17 @@ private boolean isAffixCompatible( | |||
| && dictionary.hasFlag(append, dictionary.onlyincompound)) { | |||
| return false; | |||
| } | |||
| } else if (dictionary.hasFlag(append, dictionary.onlyincompound)) { | |||
There was a problem hiding this comment.
Two similar checks from below are now united here
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.
Description
To support Dutch "15-urige" (inflected "15-urig")
Solution
To allow affixes and honor ONLYINCOMPOUND there but not COMPOUNDFLAG/COMPOUNDEND, introduce yet another compound word context COMPOUND_RULE_END, use it in the last part of a compound rule.
Tests
compoundrule4expandedChecklist
Please review the following and check all that apply:
masterbranch../gradlew check.