D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

Ahmad Samir noreply at phabricator.kde.org
Tue May 5 18:24:53 BST 2020


ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in kopenwithdialog.cpp:1006
> Right, hence the isEmpty() here. This check passes if 1) it doesn't exist, or 2) it's not executable.

I was wondering how the QString returned by KIO::DesktopExecParser::executablePath() would work if it returned /usr/bin/foo (as it doesn't strip the path), so how does findExecutable() work in that case? ... so I tested and it turns out, findExecutable() will work with:

- foo, if it can find it in $PATH and it's executable
- /usr/bin/foo, if it's executable (though I have to wonder how that qualifies as "executableName"?)
- /some/path/foo, as long as "foo" is executable, the docs don't say anything about this behaviour, however the implementation does indeed support this

This change covers the use case of an absolute path to a file that _exists_ but isn't _executable_.

And again, findExecutable() would be more useful if it reported some sort of error saying "I found it, but it's not executable".

> kopenwithdialog.cpp:1008
> +            // Maybe it exists but isn't executable
> +            if (QDir::isAbsolutePath(binaryName)) {
> +                QFileInfo fi(binaryName);

Why not QFileInfo::isAbsolute()?

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, #vdg
Cc: ahmadsamir, dfaure, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200505/29375d9a/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list