D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

kossebau added a comment. (nullptr used as arguments always make me consider finally working on a patch to add to normal kdevelop the feature "Show argument names at call site" like demoed here among other things: https://kate-editor.org/wp-content/uploads/2018/08/inline-note-anim.gif so

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

dfaure closed this revision. REPOSITORY R288 KJobWidgets REVISION DETAIL https://phabricator.kde.org/D28742 To: dfaure, broulik, davidedmundson, ervin Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

ervin added a comment. Agreed, nullptr is going to be the boolean flag of our time, before it was 0 though, so still an improvement. ;-) More seriously, here I'm not sure how to avoid it, at least it's a case of "if you feel like passing nullptr here you might be doing something wrong".

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

kossebau added a comment. Not into details, so if there is no sane default, forcing developers to pass something sane here make sense Second thought I also had was avoiding code which uses `nullptr` as arguments, which harms humans reading code a bit as in, "nullptr of what?!!" (compare

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

ervin accepted this revision. REPOSITORY R288 KJobWidgets BRANCH master REVISION DETAIL https://phabricator.kde.org/D28742 To: dfaure, broulik, davidedmundson, ervin Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

ervin added a comment. In D28742#646050 , @dfaure wrote: > In D28742#646009 , @kossebau wrote: > > > And perhaps could be defaulted to nullptr, for use-cases which do not have a window at hand and

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

dfaure added a comment. In D28742#646009 , @kossebau wrote: > `window` parameter wants API dox mentioning, though. Oops, I thought I did that. Fixed. > And perhaps could be defaulted to nullptr, for use-cases which do not have a

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

dfaure updated this revision to Diff 79857. dfaure added a comment. Expand docs about the associated window REPOSITORY R288 KJobWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28742?vs=79848=79857 BRANCH master REVISION DETAIL https://phabricator.kde.org/D28742

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

kossebau added a comment. `window` parameter wants API dox mentioning, though. And perhaps could be defaulted to nullptr, for use-cases which do not have a window at hand and are fine with any default? REPOSITORY R288 KJobWidgets BRANCH master REVISION DETAIL

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

ervin accepted this revision. ervin added a comment. Indeed, good point. REPOSITORY R288 KJobWidgets BRANCH master REVISION DETAIL https://phabricator.kde.org/D28742 To: dfaure, broulik, davidedmundson, ervin Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh,

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

dfaure updated this revision to Diff 79848. dfaure added a comment. Add QWidget *window parameter. Even better, no? Needed for dialog boxes to respect stacking order, centering to parent, focus going back to parent after closing... REPOSITORY R288 KJobWidgets CHANGES SINCE LAST

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

ervin accepted this revision. REPOSITORY R288 KJobWidgets BRANCH master REVISION DETAIL https://phabricator.kde.org/D28742 To: dfaure, broulik, davidedmundson, ervin Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

dfaure updated this revision to Diff 79840. dfaure marked an inline comment as done. dfaure added a comment. explicit REPOSITORY R288 KJobWidgets CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28742?vs=79820=79840 BRANCH master REVISION DETAIL

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

dfaure marked an inline comment as done. dfaure added inline comments. INLINE COMMENTS > kossebau wrote in kdialogjobuidelegate.h:51 > Why no explicit? good point REPOSITORY R288 KJobWidgets BRANCH master REVISION DETAIL https://phabricator.kde.org/D28742 To: dfaure, broulik,

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

kossebau added inline comments. INLINE COMMENTS > kdialogjobuidelegate.h:51 > + */ > +KDialogJobUiDelegate(KJobUiDelegate::Flags flags); // KF6 TODO merge > with default constructor, using AutoHandlingDisabled as default value > + Why no explicit? REPOSITORY R288 KJobWidgets BRANCH

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

ervin accepted this revision. This revision is now accepted and ready to land. REPOSITORY R288 KJobWidgets BRANCH master REVISION DETAIL https://phabricator.kde.org/D28742 To: dfaure, broulik, davidedmundson, ervin Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham,

D28742: Add KDialogJobUiDelegate(KJobUiDelegate::Flags) constructor

dfaure created this revision. dfaure added reviewers: broulik, davidedmundson, ervin. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY Requires: D28741 TEST PLAN