Adds a new option to the KTextEdit control in KTextWidgets to show
spelling suggestions and options, provided by Sonnet, in the standard context
menu.
BUG: 400647
Details
Details
- Reviewers
cullmann cfeck - Group Reviewers
VDG Frameworks
Existing users of this widget should be unaffected.
When opting-in to the new behavior, right-clicking near a misspelled word will no longer show the Sonnet menu, but, rather, the usual context menu. The top 5 spelling suggestions appear as the top-most options of the context menu, followed by a sub-menu providing access to all spelling suggestions. The sub-menu also contains options to "Ignore word" or "Add to dictionary". If the number of available spelling suggestions is less than five, then all of them will appear both at the top level context menu and in the sub-menu.
Selecting any spelling suggestion should make the desired substitution, leaving the cursor at the end of the newly entered word.
Diff Detail
Diff Detail
- Repository
- R310 KTextWidgets
- Branch
- bug_400647 (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 18241 Build 18259: arc lint + arc unit
Comment Actions
Very nice! Could you attach a screenshot or a screen recording in the Test Plan section that shows what this is and how it works? it's nice to do this when you add VDG as a reviewer since not all VDG people can test out patches themselves
| src/widgets/spellingmenu.h | ||
|---|---|---|
| 44 | Reviewing, I noticed that these slots are actually protected. I will need to either update the documentation or the visibility of these slots. My inclination is toward the latter, but I will wait for any feedback before making changes. | |
Comment Actions
I am not sure if changing the visibility of members is ABI compatible. This might need to wait for KF6.
Comment Actions
@cfeck - I was referring to SpellingMenu::addWordToDictionary and SpellingMenu::ignoreWord being inconsistent with the documentation of that (brand new) class. I don't think there should be any ABI considerations there, since it is a new class, but perhaps I am misunderstanding your comment or there are factors of which I am ignorant. I think it would be potentially useful to users of this class to be able to manually call this functionality. It isn't necessary, though, and I could just change the documentation to be consistent. But we probably don't want to merge-in this change if we might have a change of heart on the visibility of these slots later.
If there's something I'm missing (or you have any other feedback), let me know, thanks!
Comment Actions
Oh, I didn't read the code, only replied to the comment. If this is new code, then changing the visibility is no ABI problem.