D22510: Added dialog to set execute permission for executable file when trying to run it.

[David Faure]

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

INLINE COMMENTS

> krun.cpp:186
> +protected:
> +    void showEvent(QShowEvent *e) override
> +    {

_pre-existing) this code will also be triggered when switching virtual desktops, and we don't want a resize then. So this should test for `e->spontaneous()` and return early if true.

> krun.cpp:218
> +        m_textEdit->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Minimum);
> +        updateGeometry();
> +    }

Is this line necessary? (I wouldn't think so)

> krun.cpp:288
> +{
> +    QFile desktopFile(fileName);
> +

rename to `file` now that it's used for both

> krun.cpp:337
> +                bool canRun = true;
> +                bool isFileExecutable = isExecutableFile(u, _mimetype);
> +

At this point checking the mimetype again is redundant, it was done on line 328. I guess we need a simple QFileInfo::isExecutable call or wrapper.

> mdlubakowski wrote in krun.cpp:1105
> isExecutableFile will return false if the file doesn't have +x bit, which will result in prompt not being show, so I dont think this should be removed.

Ah. Hmm. I see. This code is confusing.
isFileExecutable will then be true for a non-+x desktop file.

But other than the naming, it makes sense, this code is actually about the edit-or-execute prompt for +x scripts and (any) desktop files.

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190902/c07ea86f/attachment.html>