dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> krun.cpp:1105
> const bool isFileExecutable = (isExecutableFile(m_strURL, mime.name()) ||
>
> mime.inherits(QStringLiteral("application/x-desktop")));
> const bool isTextFile = mime.inherits(QStringLiteral("text/plain"));
This can be removed now that isExecutableFile returns true for desktop files.
> krun.cpp:1389
> runFlags |= KRun::RunExecutables;
> + //If file is executable but doesn't have +x permission, ask user if
> he wants to set +x
> + if (!isExecutableFile(d->m_strURL, type) && isExecutable(type)) {
is *an* executable
(otherwise this sentence is very confusing)
> krun.cpp:1390
> + //If file is executable but doesn't have +x permission, ask user if
> he wants to set +x
> + if (!isExecutableFile(d->m_strURL, type) && isExecutable(type)) {
> + KMessageBox::ButtonCode makeExecutable =
> KMessageBox::questionYesNo(
What happens for desktop files? Won't this go into this code, before going into
the existing code to make desktop files executable?
> krun.cpp:1404
> + mode_t newPermissions = binaryFile.permissions() | S_IXUSR;
> + KIO::Job* chmodJob = KIO::chmod(d->m_strURL, newPermissions);
> + connect(chmodJob, &KJob::result,
Only local files can be executed anyway, so you could use
file.setPermissions(QFile::ExeUser | file.permissions()) like
makeFileExecutable does for desktop files.
Actually maybe you could reuse more of that code?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D22510
To: mdlubakowski, #frameworks, dfaure, cfeck, pino
Cc: broulik, ngraham, probono, kde-frameworks-devel, LeGast00n, GB_2, michaelh,
bruns
