Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect if logic in Speller.replaceRunOnWords()? #96

Closed
danielnaber opened this issue Sep 9, 2017 · 3 comments
Closed

incorrect if logic in Speller.replaceRunOnWords()? #96

danielnaber opened this issue Sep 9, 2017 · 3 comments

Comments

@danielnaber
Copy link
Collaborator

This looks wrong, it uses getOutputConversionPairs() if it's empty, shouldn't it be the other way round?

if (!dictionaryMetadata.getOutputConversionPairs().isEmpty()) {
candidates.add(firstCh + " " + original.subSequence(i, original.length()));
} else {
candidates.add(DictionaryLookup.applyReplacements(firstCh + " " + original.subSequence(i, original.length()),
dictionaryMetadata.getOutputConversionPairs()).toString());
}

I'm not sure if applyReplacements() makes sense at all. We've just checked that both parts are in the dictionary, should replacements still be applied? The if isn't tested, remove it and testRunonWords() still works.

Ping @milekpl, seems this was originally your code.

@danielnaber
Copy link
Collaborator Author

@jaumeortola Maybe you can comment on this?

@jaumeortola
Copy link
Contributor

I think you are right. The 'if' can be removed and just do candidates.add( ). But I haven't done any test.
Is this used anywhere? Does it cause any error?

@danielnaber
Copy link
Collaborator Author

I found this when trying to develop a fix for languagetool-org/languagetool#731 (comment) and I just thought it's probably better to clean it up first.

@dweiss dweiss closed this as completed in b3d223d Sep 25, 2017
dweiss added a commit that referenced this issue Sep 25, 2017
fix #96: incorrect logic in runOnWords
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants