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 fa

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 b

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 a

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 wind

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&id=79857 BRANCH master REVISION DETAIL https://phabricator.kde.org/D28742 AF

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 https://phabricator.kde.

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, ngrah

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 UPDATE

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&id=79840 BRANCH master REVISION DETAIL https://phabricator.kde.org/

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, davidedmu

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, brun

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