Requires: D28741
Details
Ported kruntest to ApplicationLauncherJob, used this constructor, works.
Diff Detail
- Repository
- R288 KJobWidgets
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 25071 Build 25089: arc lint + arc unit
src/kdialogjobuidelegate.h | ||
---|---|---|
51 | Why no explicit? |
src/kdialogjobuidelegate.h | ||
---|---|---|
51 | good point |
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...
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?
Oops, I thought I did that. Fixed.
And perhaps could be defaulted to nullptr, for use-cases which do not have a window at hand and are fine with any default?
I've been wondering. But people tend to forget to do so, and in most cases, if we choose the dialog delegate, then there's a QWidget based window somewhere.
Plasma uses KNotificationJobUiDelegate so it's not a problem here.
My thinking is that I'd rather force people to think about it, and possibly pass a nullptr in case there's really no window around.
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 boolean flags) ;)
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".
(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 far hoped someone else would do it, but seems it is time to help myself possibly)