D23989: Fixed interaction with DOS/Windows executables in KRun::runUrl

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Thanks for the quick fix.

INLINE COMMENTS

> krun.cpp:366
> +                // On Windows, run all executables normally
> +                bool runWindowsExecutable = true;
> +#else

const bool ...

> krun.cpp:370
> +                // from being run, so that programs like Wine can handle them instead.
> +                bool runWindowsExecutable = !mime.inherits(QStringLiteral("application/x-ms-dos-executable"));
> +#endif

const bool ...

Also, the name of this bool is very confusing in my opinion.
It's true if the file is NOT a windows executable...

Maybe name it something like supportsRunningExecutable?

> krun.cpp:390
>                      }
> -                } else if (!isFileExecutable && isTextFile) {
> -                    // Don't try to run scripts without execute bit, instead
> -                    // open them with default application
> +                } else if ((!isFileExecutable && isTextFile) || !runWindowsExecutable) {
> +                    // Don't try to run scripts without execute bit or any 

Can you split this into two "else if" conditions? It will be more readable (both the condition, and the comment). This only duplicates a trivial "canRun = false" statement.

And then we could further simplify this by checking the new bool first, so it only needs to be checked once...

  if (!supportsRunningExecutable) {
      canRun = false;
  } else if (!isFileExecutable && !isTextFile) {
      [....]
  } else if (!isFileExecutable && isTextFile) {
      canRun = false;
  }

What do you think?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D23989

To: mdlubakowski, dfaure, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190916/adfdc9ac/attachment-0001.html>