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

2020-05-29 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I have modified DesktopExecParser in kio commit d1e9325fba37eddb9cb44173edfc1fee122a0c57 to return an

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

2020-05-29 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > kopenwithdialog.cpp:1009 > +if (QDir::isAbsolutePath(binaryName)) { > +QFileInfo fi(binaryName); > +if (fi.exists() && !fi.isExecutable()) { It's created on the next line. Anyway my initial comment

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

2020-05-29 Thread Kai Uwe Broulik
broulik added inline comments. INLINE COMMENTS > ahmadsamir wrote in kopenwithdialog.cpp:1008 > https://doc.qt.io/qt-5.15/qfileinfo.html#isAbsolute ? But I don't want to create a `QFIleInfo` just because REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29445 To: broulik,

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

2020-05-29 Thread Ahmad Samir
ahmadsamir added inline comments. INLINE COMMENTS > broulik wrote in kopenwithdialog.cpp:1008 > Actually, that doesn't exist https://doc.qt.io/qt-5.15/qfileinfo.html#isAbsolute ? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29445 To: broulik, #frameworks, #vdg Cc:

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

2020-05-29 Thread Kai Uwe Broulik
broulik added inline comments. INLINE COMMENTS > broulik wrote in kopenwithdialog.cpp:1008 > Forgot that also existed, will update Actually, that doesn't exist REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29445 To: broulik, #frameworks, #vdg Cc: ahmadsamir, dfaure,

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

2020-05-06 Thread Kai Uwe Broulik
broulik added inline comments. INLINE COMMENTS > ahmadsamir wrote in kopenwithdialog.cpp:1008 > Why not QFileInfo::isAbsolute()? Forgot that also existed, will update REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D29445 To: broulik, #frameworks, #vdg Cc: ahmadsamir,

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

2020-05-05 Thread Ahmad Samir
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

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

2020-05-05 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > apol wrote in kopenwithdialog.cpp:1006 > This patch D29170 suggests that > findExecutable won't find non-executables. > > Something's wrong. Right, hence the isEmpty() here. This check passes if 1) it doesn't

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

2020-05-05 Thread Aleix Pol Gonzalez
apol added inline comments. INLINE COMMENTS > kopenwithdialog.cpp:1006 > // Ensure that the typed binary name actually exists (#81190) > if (QStandardPaths::findExecutable(binaryName).isEmpty()) { > +// Maybe it exists but isn't executable This patch D29170

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

2020-05-05 Thread Kai Uwe Broulik
broulik created this revision. broulik added reviewers: Frameworks, VDG. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. broulik requested review of this revision. REVISION SUMMARY `QStandardPaths::findExecutable` only finds executable binaries, so when