QStandardPaths::findExecutable only finds executable binaries, so when explicitly pointing it at an existing file, it would complain about it not existing, rather than telling you it's not executable.
Details
- Reviewers
dfaure - Group Reviewers
Frameworks VDG
- Tried to associate a non-executable shell script with a file: it now told me what was up
Before it would tell me file doesn't exist
Diff Detail
- Repository
- R241 KIO
- Lint
Lint Skipped - Unit
Unit Tests Skipped
src/widgets/kopenwithdialog.cpp | ||
---|---|---|
1006 | Right, hence the isEmpty() here. This check passes if 1) it doesn't exist, or 2) it's not executable. |
src/widgets/kopenwithdialog.cpp | ||
---|---|---|
1006 | 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:
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". | |
1008 | Why not QFileInfo::isAbsolute()? |
src/widgets/kopenwithdialog.cpp | ||
---|---|---|
1008 | Forgot that also existed, will update |
src/widgets/kopenwithdialog.cpp | ||
---|---|---|
1008 | Actually, that doesn't exist |
src/widgets/kopenwithdialog.cpp | ||
---|---|---|
1008 |
src/widgets/kopenwithdialog.cpp | ||
---|---|---|
1008 | But I don't want to create a QFIleInfo just because |
src/widgets/kopenwithdialog.cpp | ||
---|---|---|
1009 | It's created on the next line. Anyway my initial comment was a nit-pick... it's a micro-optimisation for when the code goes through the if block, technically DesktopExecParser should get the absolute path in most cases. |
I have modified DesktopExecParser in kio commit d1e9325fba37eddb9cb44173edfc1fee122a0c57 to return an error string so that its users don't need to do the work again of figuring out what went wrong (see DesktopExecParser::errorMessage()).
I just realized that this would be very useful here as well. AFAICS the current patch only works if the user types a full path, not with a relative path.