D17617: Display error instead of silently failing when asked to create folder that already exists

[David Faure]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20181216/88736f12/attachment.htm>