dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
BTW if you also want the mkpath case to error-if-already-exists, I guess this
could be implemented in KIO::mkpath with a flag [and then the if()/else() isn't
needed anymore, actually]. Not sure it's worth it though.
INLINE COMMENTS
> knewfilemenu.cpp:906
> + // If the name contains any slashes, use mkpath so that a/b/c works.
> + if (name.contains(QStringLiteral("/"))) {
> + job = KIO::mkpath(url, baseUrl);
QLatin1Char('/') would be enough
> knewfilemenu.cpp:908
> + job = KIO::mkpath(url, baseUrl);
> + job->setProperty("mkpathUrl", url);
> + KJobWidgets::setWindow(job, m_parentWidget);
(could have been renamed to something more generic, in all 3 places)
> knewfilemenu.cpp:910
> + KJobWidgets::setWindow(job, m_parentWidget);
> + job->uiDelegate()->setAutoErrorHandlingEnabled(true);
> +
> KIO::FileUndoManager::self()->recordJob(KIO::FileUndoManager::Mkpath,
> QList<QUrl>(), url, job);
This could be moved out of the if()/else(), no? In fact, same thing for
setWindow and setProperty. No point in duplicating these lines.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D17617
To: ngraham, #dolphin, elvisangelaccio, dfaure
Cc: dfaure, emateli, elvisangelaccio, Codezela, kde-frameworks-devel, michaelh,
ngraham, bruns
