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
