> On Oct. 23, 2014, 11:43 a.m., David Faure wrote: > > Thanks for the patch. > > > > Calling init() again surprised me a bit, since this method was never called > > twice before, but OK, anything else would be much more invasive. I would > > suggest to at least add a comment e.g. in the call to showPrompt(), to say > > "this will call init() again once the dialog is closed" or something. > > > > Some small things below.
> Calling init() again surprised me a bit, since this method was never called > twice before Why not splitting up this method? Just renaming the old init method and hide it, then create a new init function which checks if a dialog is needed or not and load the settings. The old init function can then be called in the dialog finished handler, or directly in the new init method when no dialog is needed at all. > anything else would be much more invasive And I don't think this will be too invasive ;) - Emmanuel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120666/#review68968 ----------------------------------------------------------- On Oct. 22, 2014, 6:10 p.m., Arjun AK wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120666/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2014, 6:10 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kio > > > Description > ------- > > This patch makes KIO show a dialog box asking the user what to do (either > open it using a text editor or execute it) when he clicks on a script or a > desktop file. > > See also: https://git.reviewboard.kde.org/r/120171/ > > > Diffs > ----- > > src/widgets/CMakeLists.txt 4060cdf > src/widgets/executablefileopendialog.h PRE-CREATION > src/widgets/executablefileopendialog.cpp PRE-CREATION > src/widgets/krun.cpp 6ac42da > src/widgets/krun_p.h 69e2e98 > > Diff: https://git.reviewboard.kde.org/r/120666/diff/ > > > Testing > ------- > > > Thanks, > > Arjun AK > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
