Review Request: associate an external tool to an applet

Aaron Seigo aseigo at kde.org
Tue Sep 15 16:49:26 CEST 2009



> On 2009-09-15 04:32:47, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/applet.h, lines 631-637
> > <http://reviewboard.kde.org/r/1606/diff/2/?file=11440#file11440line631>
> >
> >     external -> associated?
> >     tool -> application?
> >     
> >     we will need to have a non-KService version of the tool definer method since not all apps will be registered in that manner. i think we could probably just take a string and figure out ourselves internally if it's a service or an executable.
> >     
> >     as for arguments ... hm. use cases could be:
> >     
> >     * command line arguments (e.g. konsole -e ls -lR)
> >     * URLs
> >     * ...?
> >     
> >     probably needs more use case thinking. 
> >     
> >     if it does remain limited to URLs (and i think an API that makes launching apps with urls would be a good idea), then that might be a more generic sort of thing: associatedUrls()? 
> >     
> >     it will need to support url lists.
> >     
> >     a "does this applet have a valid associated app?" convenience method would be nice
> >     
> >
> 
>  wrote:
>     ok, so it should take a string and then try to obtain a service out of it.
>     if it fails it gets executed as a command... i wasn't really sure about that since i wanted to avoid people passing absolute urls and then complainig it's broken, but well, their bad..
>     
>     for the list of urls i'm wondering if the app is not present would make sense to try to run all of them, it could be massive...

yes, the heuristic is probably pretty simple: if it's an absolute path, treat it as a command. otherwise, try and find it as a service. if that fails, use findExe. if that fails, they failed ;)


> On 2009-09-15 04:32:47, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/applet.cpp, line 1848
> > <http://reviewboard.kde.org/r/1606/diff/2/?file=11441#file11441line1848>
> >
> >     you should stop the job here, no? or perhaps put it on hold and publish it so that the app can then use it?
> 
>  wrote:
>     hmm, not sure it would be useful published?

it allows the application that will be launched in a few moments to (magically!) reuse that IOSlave so if it's accessing a remote file it doesn't need to establish a connection with the remote system again. 

       job->putOnHold();
        KIO::Scheduler::publishSlaveOnHold();
 
is pretty magical :) but yes, krun can do it all for us.


> On 2009-09-15 04:32:47, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/applet.cpp, line 1980
> > <http://reviewboard.kde.org/r/1606/diff/2/?file=11441#file11441line1980>
> >
> >     you know what would be dreadfully cool? if there was an external tool manager that, when an app was triggered, it would see if that resulted in a window popping up. if so, then it could track that window and as long as that window existed, it could become associated with the widget?
> >     
> >     might not work so hot, i suppose, if you open an image in a viewer, navigate to another image in the viewer, and then click the widget's "run" icon again. hmm...
> >     
> >     another benefit of having a manager for these would be that instead of having an extra set of members in the applet's dptr regardless of whether or not it uses this feature, there would only be one manager and only as many objects in memory as there are widgets using this feature.
> 
>  wrote:
>     a singleton? accessible in the public api? and it should avoid to launch the app two times?

i think it can be private for the time being. Applet will use it internally.


> On 2009-09-15 04:32:47, Aaron Seigo wrote:
> > /trunk/KDE/kdelibs/plasma/applet.cpp, lines 1985-1987
> > <http://reviewboard.kde.org/r/1606/diff/2/?file=11441#file11441line1985>
> >
> >     this is actually unecessary; KRun does all of this for you, including re-using the mimetype fetching job.
> 
>  wrote:
>     hmm, but i actuall need to fetch the mimetype no? runurl wants it as arameter, so it doesn't try to understand it by itself no?

what i meant is that you won't be using runUrl anymore and can just let KRun do it for you.


- Aaron


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1606/#review2367
-----------------------------------------------------------


On 2009-09-14 15:04:43, Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1606/
> -----------------------------------------------------------
> 
> (Updated 2009-09-14 15:04:43)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> first implementation of an idea discussed @tokamak: add api to assign the execution of something to an applet. the rationale is to launch an app that is the "full version" of that applet, where the applet serves as little preview.
> it is possible to assign a kservice (usually used with kservice::servicefromdesktopfile("app name")) and an optional argument.
> if the service is missing and the argument url is present the url will be opened based on its mimetype.
> an applethandle icon and a context menu entry will be added to launch that external tool
> now there are roe rough edges:
> -api: pass kservice and url or just strings to make bindings easier?
> -urls: right now there is only one parameter: maybe having support for a list?
> -naming: now all the names are talking about an "external tool", the applet handle uses the "maximize" icon, calling it maximize everywhere? i would leave external tool in the api and call maximize the action and the action text, that is what is presented to the user..
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/plasma/applet.h 1023331 
>   /trunk/KDE/kdelibs/plasma/applet.cpp 1023331 
>   /trunk/KDE/kdelibs/plasma/containment.cpp 1023331 
>   /trunk/KDE/kdelibs/plasma/private/applet_p.h 1023331 
>   /trunk/KDE/kdelibs/plasma/private/applethandle.cpp 1023331 
>   /trunk/KDE/kdelibs/plasma/private/applethandle_p.h 1023331 
> 
> Diff: http://reviewboard.kde.org/r/1606/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco
> 
>



More information about the Plasma-devel mailing list