D24993: Add integrated inline spelling menu to KTextEdit
pajamapants3000 added a comment. @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! REPOSITORY R310 KTextWidgets REVISION DETAIL https://phabricator.kde.org/D24993 To: pajamapants3000, #vdg, #frameworks, cullmann, cfeck Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns
D24993: Add integrated inline spelling menu to KTextEdit
pajamapants3000 added inline comments. INLINE COMMENTS > spellingmenu.h:44 > + * are handled internally, without any extra effort required from the parent. > + * Slots are available to facilitate these actions on-demand if needed. > + * 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. REPOSITORY R310 KTextWidgets REVISION DETAIL https://phabricator.kde.org/D24993 To: pajamapants3000, #vdg, #frameworks, cullmann, cfeck Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns
D24993: Add integrated inline spelling menu to KTextEdit
pajamapants3000 edited the test plan for this revision. REPOSITORY R310 KTextWidgets REVISION DETAIL https://phabricator.kde.org/D24993 To: pajamapants3000, #vdg, #frameworks, cullmann, cfeck Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns
D24993: Add integrated inline spelling menu to KTextEdit
pajamapants3000 created this revision. pajamapants3000 added a reviewer: VDG. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. pajamapants3000 requested review of this revision. REVISION SUMMARY 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 REPOSITORY R310 KTextWidgets BRANCH bug_400647 (branched from master) REVISION DETAIL https://phabricator.kde.org/D24993 AFFECTED FILES src/CMakeLists.txt src/widgets/ktextedit.cpp src/widgets/ktextedit.h src/widgets/spellingmenu.cpp src/widgets/spellingmenu.h To: pajamapants3000, #vdg Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D21897: Address some issues reported by Krazy analysis
pajamapants3000 added inline comments. INLINE COMMENTS > elvisangelaccio wrote in krichtextwidget.cpp:638 > Ah of course. I didn't notice it was `exec()` that was being called. Coming back after two months, even I thought this looked wrong at first. I will endeavor to make my intent clearer, thank you :) > broulik wrote in ktextedit.cpp:373 > You can do `dialog->setAttribute(Qt::WA_DeleteOnClose);` when creating the > dialog instead of an explicit delete call Thank you broulik! Could we use a `QScopedPointer` here? I see them throughout the KDE source, but not often for dialogs. The krazy analysis looked for `QPointer`, referencing blog https://blogs.kde.org/node/3919, which appears to have predated the existence of `QScopedPointer`. REPOSITORY R310 KTextWidgets BRANCH krazy-50b2564 (branched from master) REVISION DETAIL https://phabricator.kde.org/D21897 To: pajamapants3000, elvisangelaccio Cc: broulik, vkrause, elvisangelaccio, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D21897: Address some issues reported by Krazy analysis
pajamapants3000 added a comment. Thank you for the feedback and approval! I don't have the access needed to land a change, what should I do next? I would certainly appreciate someone else landing it for me. Let me know, thanks! REPOSITORY R310 KTextWidgets BRANCH krazy-50b2564 (branched from master) REVISION DETAIL https://phabricator.kde.org/D21897 To: pajamapants3000, elvisangelaccio Cc: vkrause, elvisangelaccio, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D21897: Address some issues reported by Krazy analysis
pajamapants3000 created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. pajamapants3000 requested review of this revision. REVISION SUMMARY Resolved all issues discovered by the krazy plugins: crashy, explicit. Other issues remain; I left them alone because I either wasn't sure they should be addressed or I wasn't sure how to do so. TEST PLAN Should build and pass existing tests - confirmed. REPOSITORY R310 KTextWidgets BRANCH krazy-50b2564 (branched from master) REVISION DETAIL https://phabricator.kde.org/D21897 AFFECTED FILES autotests/kfindtest.h src/kregexpeditor/kregexpeditorinterface.h src/widgets/krichtextwidget.cpp src/widgets/ktextedit.cpp To: pajamapants3000 Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
