ahmadsamir added a comment.

  In D29170#657334 <https://phabricator.kde.org/D29170#657334>, @dfaure wrote:
  
  > Resolve relative executables using the directory of the .desktop file 
referring to them
  >
  > Not useful for /usr/share/applications stuff, but useful for custom setups
  >  where a custom desktop file refers to a local executable.
  >
  > Re-reading the thread 
https://lists.freedesktop.org/archives/xdg/2011-April/011883.html
  >  I'm wondering if this is the right thing to do though.
  >
  > After all, on the command line "foo" doesn't start a local executable 
called foo,
  >  only ./foo does that. I was always working under that assumption for Exec= 
as well
  >  (the fact that the current dir isn't part of the search path). Undecided.
  
  
  I don't think you need to go out of your way to support custom setups, after 
all it's quite simple for the user to edit the .desktop file and specify the 
path to the executable. Even from the xdg thread, they were talking about 
installing a virtual machine guest stuff, which is something the user only has 
to do once, whereas, from my POV at least, a .desktop file is more for things 
your run frequently/on a regular basis.
  
  It would simplify the code, and would be consistent with how a shell would 
run a program; indeed a binary in the current dir has to be explicitly prefixed 
with  ./, (I can't remember exactly but it I think I read somewhere that's for 
security reasons).
  
  That also means that the original code trying to find the executable in the 
current dir never really worked, because "current dir" is most likely where the 
_KIO executable_ exists (I tested with qt-creator and QDir::current() is indeed 
where the compiled binary exists). So not that useful.

INLINE COMMENTS

> desktopexecparser.cpp:465
> +                }
> +                if (QFile::exists(exePath)) {
> +                    execlist[0] = exePath;

I think we should check for isExecutable() here too (this matches the behaviour 
of QStandardPaths::findExecutable()).

> kprocessrunner.cpp:53
> +    const QFileInfo fi(executable);
> +    if (fi.exists() && !fi.isExecutable())
> +        return executable;

Coding style: braces around if block.

REPOSITORY
  R241 KIO

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

To: dfaure, ahmadsamir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

Reply via email to