D26259: Fixes svn plugin issuing "mkfifo" on Windows.

Elvis Angelaccio noreply at phabricator.kde.org
Sun Jan 12 18:32:35 GMT 2020


elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> fileviewsvnplugin.cpp:311
> +    QProcess *process = new QProcess(this);
> +    const QString tmpFileNameTemplate = QDir::tempPath() + QDir::separator() + QString("%1.XXXXXX").arg( QDir(m_contextDir).dirName() );
> +    QTemporaryFile *file = new QTemporaryFile(tmpFileNameTemplate, process);

Please use `QStringLiteral("%1/%2.XXXXXX").arg(...)` instead. No need to use QDir::separator().

> fileviewsvnplugin.cpp:315
> +    if (!file->open()) {
> +        emit errorMessage( i18nc("@info:status", "Show changes error: can't create temporary file.") );
> +        process->deleteLater();

No space before/after ()

> fileviewsvnplugin.cpp:315
> +    if (!file->open()) {
> +        emit errorMessage( i18nc("@info:status", "Show changes error: can't create temporary file.") );
> +        process->deleteLater();

I'd just show a generic "Could not show local SVN changes.". Maybe we could print a qWarning() saying that the temporary file could not be created (which is unlikely though...).

> fileviewsvnplugin.cpp:330
> +    if (!process->waitForFinished() || process->exitCode() != 0) {
> +        emit errorMessage( i18nc("@info:status", "Show changes error: svn diff failed.") );
> +        process->deleteLater();

"Could not show local SVN changes: svn diff failed".

> fileviewsvnplugin.cpp:343
> +    if ( !process->waitForStarted() ) {
> +        emit errorMessage( i18nc("@info:status", "Show changes error: start of kompare failed.") );
> +        process->deleteLater();

"Could not show local SVN changes: could not start kompare". This would also help in the case kompare is not installed.

REVISION DETAIL
  https://phabricator.kde.org/D26259

To: nikolaik, #dolphin, elvisangelaccio, meven, ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200112/e8543cee/attachment.htm>


More information about the kfm-devel mailing list