[Panel-devel] Re: PATCH: Dynamic Quicklauchner applet

[Fred Schaettgen]

On Thursday, 2. June 2005 03:37, Aaron J. Seigo wrote:
> On Wednesday 01 June 2005 02:23, you wrote:
> > - The minimum number of items
> > - The maximum number of items
> > - The threshold when an application will be included/removed.
>
> you seem to have forgotten to include the autoadjustdlgbase.ui file which
> is preventing me from compiling this =)

svn add, svn diff, apply diff to clean checkout the be sure that everything is 
in, clean up some debug output in new checkout, svn diff, miss the newly 
added files :P 
I hope it's more complete (and still small enough) this time *sigh*

> hm.. looking at the kmenu patch i really wonder if the DCOP signal
> shouldn't be attached directly to the RecentlyLaunchedApps (RLA) class.
... 

I guess it's fine if the signal is attached to the kmenu and RLA stays a 
rather generic statistics class. We might want to ignore some launch signals 
at a later time (don't know what.. maybe if we also signal recent document 
urls or something), and the decision to ignore the signal should not be done 
by the RLA class then.

> kmenu (which is a PanelServiceMenu (PSM) subclass) item selected ->
> PSM::exec emits the dcop signal -> kmenu gets the signal calls ->
> PSM::updateRecentMenuItems -> calls a bunch of RLA methods
>
> in fact, it seems that PSM::updateRecentMenuItems(service) gets called
> twice, once by KMenu and once by PSM itself.

You're right.. now I remember what the sender argument in the signal was good 
for ;) In the new patch it ignores messages sent by itself. We could also use 
just the DCOP signal, but somehow I don't like the idea of an application 
relying on dcop to communicate with its own modules.

> next time kmenu is opened -> KMenu::initialize() is called -> calls
> PSM::updateRecent() -> PSM messes around with the entries in the KMenu.
> there is no reason for this to be in PSM!

Yeah, I wondered myself why the RLA-related code was put in the base class 
(PSM) and not in the kmenu class. Maybe the author wanted to introduce 
recently used lists not just in the toplevel kmenu?

> so it seems we have some very snaky code here. this problem obviously
> predates your patches. instead, i'd like to see this happen:
>
> dcop signal is emitted (from minicli or PSM) -> RLA::applicationLaunched()
> called -> if it results in a change, emit RLA::recentAppsChanged() ->
> KMenu::updateRecentApplications() (moved from PSM) is called
>
> much nicer, no? what do you think?

Certainly, if the RLA feature isn't supposed to be used in other PSM-based 
classes.

Fred

-- 
Fred Schaettgen
kde.sch at ttgen.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kicker-applets-launcher_dynamicLauncher_schaettgen20050602_2.diff.zip
Type: application/x-zip
Size: 14652 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20050602/528cea78/kicker-applets-launcher_dynamicLauncher_schaettgen20050602_2.diff-0001.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kicker-kicker_execDCOPSignal_schaettgen20050602.diff
Type: text/x-diff
Size: 3464 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/panel-devel/attachments/20050602/528cea78/kicker-kicker_execDCOPSignal_schaettgen20050602-0001.bin