I see that the command line explicitly limits replacements to 5 (hardcoded) and I see that web-UI also has a limit (although I didn’t find the code).
Would it be reasonable to allow this limit to be configurable? Maybe allow to override per language?
I do have quite a bit of cases in Ukrainian where 5 suggestions can’t cover them all. Then either suggestions are dropped or I have to combine them together, which makes it ugly for the user (and impossible to inflect the related words properly).
What also does not help is that there’s no warning (e.g. in the unit tests) that the rule has too many suggestions and they will be dropped.
I’m not sure… I think the UI becomes very overloaded with just 5 suggestions already. It would be great if we could limit the suggestions to those that make most sense.
Understood. But can we add a warning in our unit tests if the rule has exceeded suggestion limit?
I could try to add it myself, but I could not find a common place/constant for the cut-off.
As the suggestions from Java rules can be generated dynamically, I think you’re only referring to XML rules? I guess it could be added somewhere in PatternRuleTest.runTestForLanguage().
Well, definitely for the xml rules.
But also I see that we have a “sanity check” for AbstractSimpleReplaceRule2 that prints “WARNING: replacement … isn’t known to spell checker” - that could be a good place too (it’ll cover about 9000 words with replacements for Ukrainian).
But also even in the Rule itself (or its satellite classes) - if it can print a warning, then if you have good enough unit test coverage then you’ll be able to catch most of the cases where your replacements are ignored. We could also use logger.warn/info to allow the user to show or hide such warnings (although I’d rather always see them in the unit tests like the message for AbstractSimpleReplaceRule2).
But that would print the warning either way, running as a unit test or not, wouldn’t it? Not that this is necessarily an issue.
There are some ways to control that, e.g. we can add (static) ‘showLengthWarning’ field to the rule class and set it from unit tests only.
But on the other hand, if we’re discarding suggestions (at least for non-speller rules) it may be not a bad thing to print a warning in the log (especially if we can control it with the logger config).
I’ve created a branch that prints warnings for > 5 replacements for simple replace rule and xml rules.
For xml rules it’s a bit tricky - if there are more than 5 replacements in the message, the user will see them as text, but there won’t be buttons to click for automatic replacement. But the suggestions exceed the limit out of the message then they are not visible in UI at all. I’ve tried to show messages to cover those scenarios.
I’ve fixed the ‘if’ formatting and rebased the branch on top of latest master.