sitter added a comment.
some style complaints. Looks great other than that 👍 INLINE COMMENTS > krun.cpp:583 > + const QUrl url = urls[i]; > + if (KIO::DesktopExecParser::isProtocolInSupportedList(url, > appSupportedProtocols)) > + continue; Coding style says we use curly braces even for single-line if statements I think. > sitter wrote in krun.cpp:598 > Is there a reason you are not using a range based for loop here? `for (const > QUrl &url : urls)` What I meant is you should literally iterate using for (QUrl &url : urls) { which is what the code did before but you changed it for some reason. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D23384 To: feverfew, fvogt, davidedmundson, dfaure, ngraham Cc: sitter, davidedmundson, kde-frameworks-devel, ngraham, LeGast00n, GB_2, michaelh, bruns