You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/01/21 14:25:06 UTC
[GitHub] [lucene-solr] donnerpeter opened a new pull request #2229: LUCENE-9688: Hunspell: consider prefix's continuation flags when applying suffix
donnerpeter opened a new pull request #2229:
URL: https://github.com/apache/lucene-solr/pull/2229
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Lucene or Solr:
* https://issues.apache.org/jira/projects/LUCENE
* https://issues.apache.org/jira/projects/SOLR
You will need to create an account in Jira in order to create an issue.
The title of the PR should reference the Jira issue number in the form:
* LUCENE-####: <short description of problem or changes>
* SOLR-####: <short description of problem or changes>
LUCENE and SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
aff:
SFX s Y 1
SFX s 0 s .
PFX p Y 1
PFX p 0 0/s .
dic:
calorie/p
"calories" should be accepted
# Solution
When applying suffix "s" (which expects "s" flag to be set), check not only the word itself (which only has "p") flag, but the continuation flags of the just applied prefix "p" which has "s" as its continuation.
# Tests
`TestDependencies` updated
# Checklist
Please review the following and check all that apply:
- [ ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [ ] I have developed this patch against the `master` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2229: LUCENE-9688: Hunspell: consider prefix's continuation flags when applying suffix
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2229:
URL: https://github.com/apache/lucene-solr/pull/2229#discussion_r562463263
##########
File path: lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/Stemmer.java
##########
@@ -472,9 +472,9 @@ private boolean checkCondition(
* @param strippedWord Word the affix has been removed and the strip added
* @param length valid length of stripped word
* @param affix HunspellAffix representing the affix rule itself
- * @param prefixFlag when we already stripped a prefix, we cant simply recurse and check the
- * suffix, unless both are compatible so we must check dictionary form against both to add it
- * as a stem!
+ * @param prefixId when we already stripped a prefix, we cant simply recurse and check the suffix,
Review comment:
cant -> can't (I realize it's not you, but maybe correct while we're here).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2229: LUCENE-9688: Hunspell: consider prefix's continuation flags when applying suffix
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2229:
URL: https://github.com/apache/lucene-solr/pull/2229#discussion_r562486256
##########
File path: lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/Stemmer.java
##########
@@ -588,6 +585,12 @@ private boolean checkCondition(
return stems;
}
+ private boolean isFlagAppendedByAffix(int affixId, char flag) {
+ if (affixId < 0) return false;
Review comment:
The discussion of whether braces in single-line statements in if/while/for loops are needed or not
is like religion - you can't end it, once started... I'm just relaying what has been said
on the mailing list after the formatter folded code like this:
```
if (foo)
doSomething()
else
doSomethingElse()
```
Privately, I don't have a strong opinion. Depends on the context where it's at, I guess.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2229: LUCENE-9688: Hunspell: consider prefix's continuation flags when applying suffix
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2229:
URL: https://github.com/apache/lucene-solr/pull/2229#discussion_r562555997
##########
File path: lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/Stemmer.java
##########
@@ -588,6 +585,12 @@ private boolean checkCondition(
return stems;
}
+ private boolean isFlagAppendedByAffix(int affixId, char flag) {
+ if (affixId < 0) return false;
Review comment:
I think consistency in those braces (over verbosity) is a good rule in general, but ok.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2229: LUCENE-9688: Hunspell: consider prefix's continuation flags when applying suffix
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2229:
URL: https://github.com/apache/lucene-solr/pull/2229#discussion_r562524364
##########
File path: lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/Stemmer.java
##########
@@ -588,6 +585,12 @@ private boolean checkCondition(
return stems;
}
+ private boolean isFlagAppendedByAffix(int affixId, char flag) {
+ if (affixId < 0) return false;
Review comment:
Would it be acceptable to reformat the code when such a demand appears, or after I'm finished with these improvements? These "some people" might never need to look into Hunspell code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2229: LUCENE-9688: Hunspell: consider prefix's continuation flags when applying suffix
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2229:
URL: https://github.com/apache/lucene-solr/pull/2229#discussion_r562483637
##########
File path: lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/Stemmer.java
##########
@@ -588,6 +585,12 @@ private boolean checkCondition(
return stems;
}
+ private boolean isFlagAppendedByAffix(int affixId, char flag) {
+ if (affixId < 0) return false;
Review comment:
Curly braces and 2 more lines? Oh pity, I love conciseness so much :(
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2229: LUCENE-9688: Hunspell: consider prefix's continuation flags when applying suffix
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2229:
URL: https://github.com/apache/lucene-solr/pull/2229#discussion_r562462937
##########
File path: lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/Stemmer.java
##########
@@ -588,6 +585,12 @@ private boolean checkCondition(
return stems;
}
+ private boolean isFlagAppendedByAffix(int affixId, char flag) {
+ if (affixId < 0) return false;
Review comment:
Add curly braces. I know, it's verbose but some folks on this list even went as far as to require it (the formatter doesn't care).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss merged pull request #2229: LUCENE-9688: Hunspell: consider prefix's continuation flags when applying suffix
Posted by GitBox <gi...@apache.org>.
dweiss merged pull request #2229:
URL: https://github.com/apache/lucene-solr/pull/2229
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2229: LUCENE-9688: Hunspell: consider prefix's continuation flags when applying suffix
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2229:
URL: https://github.com/apache/lucene-solr/pull/2229#discussion_r562493566
##########
File path: lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/Stemmer.java
##########
@@ -588,6 +585,12 @@ private boolean checkCondition(
return stems;
}
+ private boolean isFlagAppendedByAffix(int affixId, char flag) {
+ if (affixId < 0) return false;
Review comment:
In that case I'd also put braces (although I've seen people who don't). But my case is "then-branch is a simple `return` on the same line". Are braces also required in this case? And separate lines, like this?
```
if (affixId < 0) {
return;
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2229: LUCENE-9688: Hunspell: consider prefix's continuation flags when applying suffix
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2229:
URL: https://github.com/apache/lucene-solr/pull/2229#discussion_r562496757
##########
File path: lucene/analysis/common/src/java/org/apache/lucene/analysis/hunspell/Stemmer.java
##########
@@ -588,6 +585,12 @@ private boolean checkCondition(
return stems;
}
+ private boolean isFlagAppendedByAffix(int affixId, char flag) {
+ if (affixId < 0) return false;
Review comment:
Required is not the right word... Perhaps... advised. :) You are entitled to your own opinion (at least as long as this passes precommit checks!) - like I said, I relayed what's been mentioned on the mailing list. Some people just put braces everywhere and they are used to this consistency, even if it adds verbosity.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org