D17624: KTextEditor::Message: Review documentation
dhaumann closed this revision. REVISION DETAIL https://phabricator.kde.org/D17624 To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars
D17624: KTextEditor::Message: Review documentation
dhaumann added a comment. I integrated this now, see a16d082370a44fcbae3a204bfede1db6e6dffe86 - the change also includes the rename of autoHide to autoHideDelay. The function names cannot be changed of course, since it's public API. REVISION DETAIL https://phabricator.kde.org/D17624 To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars
D17624: KTextEditor::Message: Review documentation
loh.tar added a comment. Sorry for the hassle, will revert the update > Further, I would prefer autoHideDelay instead of just delay. Are you refering here to the header or the code part? The old name in the header was "autoHideTimeR" which is pretty wrong. So have I to change this in the header/docu to "autoHideDelay"? Or perhaps "autoHideTime"? Then is the change minimal. REVISION DETAIL https://phabricator.kde.org/D17624 To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars
D17624: KTextEditor::Message: Review documentation
dhaumann added a comment. Now you are mixing documentation changes with code changes. I would have preferred to have different review requests, especially since the documentation part was already reviewed. Further, I would prefer autoHideDelay instead of just delay. If we use "delay" only, then the question immediately arises "the delay of what"? With this in mind, the naming of "autoHide" as better, since at least the context was clear. I will take care of the documentation now, but will not take care of the renaming for the reason given. REVISION DETAIL https://phabricator.kde.org/D17624 To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars
D17624: KTextEditor::Message: Review documentation
loh.tar updated this revision to Diff 47678. loh.tar added a comment. -Rename member autoHide->delay to fit accepted change in header file just an offer CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17624?vs=47673=47678 REVISION DETAIL https://phabricator.kde.org/D17624 AFFECTED FILES src/include/ktexteditor/message.h src/utils/messageinterface.cpp To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars
D17624: KTextEditor::Message: Review documentation
loh.tar updated this revision to Diff 47673. loh.tar added a comment. - Fix optionally/optional + be I like to suggest that some more eyes take a look at that :-) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17624?vs=47669=47673 REVISION DETAIL https://phabricator.kde.org/D17624 AFFECTED FILES src/include/ktexteditor/message.h To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars
D17624: KTextEditor::Message: Review documentation
dhaumann accepted this revision. dhaumann added a comment. This revision is now accepted and ready to land. Looks almost good, except for "optional" and a missing "be". Could you update again? INLINE COMMENTS > message.h:337 > /** > - * Optionally set an icon for this notification. > - * The icon is shown next to the message text. > + * Add an optionally @p icon for this notification which will shown next > to > + * the message text. If the message was already sent through > Document::postMessage(), Add an optional @p icon ... which will be shown REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17624 To: loh.tar, #ktexteditor, dhaumann Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars
D17624: KTextEditor::Message: Review documentation
loh.tar created this revision. loh.tar added a reviewer: KTextEditor. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. loh.tar requested review of this revision. REVISION SUMMARY - Reduce redundant text - Change some argument names in the hope it's an improvement - Update a code example to fit new style - Try to improve some wording - Add some @see, @p and such, but probably not all will be needed due to "auto reference" - Fix some outdated info REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17624 AFFECTED FILES src/include/ktexteditor/message.h To: loh.tar, #ktexteditor Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
