Review Request 120267: Don't run kioexec if we are already running it

[David Faure]

> On Oct. 11, 2014, 9:03 a.m., David Faure wrote:
> > src/core/desktopexecparser.cpp, line 310
> > <https://git.reviewboard.kde.org/r/120267/diff/2/?file=316559#file316559line310>
> >
> >     I would merge this with the if (useKioexec) line. Otherwise this creates a new code path (going directly to result << exec), while surely not entering the main if() would have worked just as well? Less different cases to handle.
> 
> Maarten De Meyer wrote:
>     Actually we need that extra code path, otherwise the url's are not added to the result.
>     ex. kioexec gwenview image.png
>     
>     if(usekioexec)
>       if(name == "kioexec")
>       ...
>     EXEC "gwenview image.png"
>     
>     if(usekioexec && name == "kioexec")
>       ...
>     EXEC "gwenview" (and somehow even deletes image.png)

Ah, I see --- because there's no %u/%U, so the urls don't get substituted.

Hmm, the bit about deleting is scary ... kioexec's whole point is to delete the local file after use. Running kioexec on local files is really not a good idea.

How about we drop all this and just let kioexec exit with an error, when used on local files? There is no use case for this, let's not fix the non-existent use case, let's just catch it as soon as we can.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120267/#review68253
-----------------------------------------------------------


On Oct. 4, 2014, 12:56 p.m., Maarten De Meyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120267/
> -----------------------------------------------------------
> 
> (Updated Oct. 4, 2014, 12:56 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 339123
>     https://bugs.kde.org/show_bug.cgi?id=339123
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> Only add the actual command and url's otherwise we keep spawning instances of kioexec.
> Maybe we should rename the kioexec service from 'dummy' to 'kioexec-dummy' just to be safe.
> 
> 
> Diffs
> -----
> 
>   src/core/desktopexecparser.cpp 9510697 
>   src/kioexec/main.cpp f0e2823 
> 
> Diff: https://git.reviewboard.kde.org/r/120267/diff/
> 
> 
> Testing
> -------
> 
> Run kioexec with local file from command line.
> Remote files don't work (before and after) I'll take a look at that next.
> 
> 
> Thanks,
> 
> Maarten De Meyer
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20141011/73d196a9/attachment.html>