From kfm-devel Tue Jul 16 01:57:00 2002 From: "Dawit A." Date: Tue, 16 Jul 2002 01:57:00 +0000 To: kfm-devel Subject: Re: PATCH: Open With... X-MARC-Message: https://marc.info/?l=kfm-devel&m=102678517515984 On Monday 15 July 2002 04:04, David Faure wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On Sunday 14 July 2002 20:27, Dawit A. wrote: > > - Make sure that separators are added properly. Currently there were > > cuircumstances where a double/triple separators were being drawn. The > > way to make sure this does not happen is to only add separators "before" > > adding new menu items, never "after". The culprit for the above problem > > were the plugins, specifically the kuick plugin. As such, with the pat= ch > > attached below, a plugin no longer needs to add any separator unless it > > specifically needs to separate its own items... > > I don't see the point of this change. IIRC the logic was simply the other > way round: "always add separators _after_ adding new menu items, never > _before_". Well IMHO that was part of the problem :) . Deciding whether or not you nee= d=20 to add separators should be done before adding the item that you want to=20 separate. A couple of reasons for this. The first being that the popup men= u=20 is context based. As such things are shown based on context. This means y= ou=20 do not want to draw a separator that might otherwise be unnecessary if that= =20 particular group of items are not supposed to be shown... The other issue = is=20 that you are more likely to add a unwanted separator "after" the last item= =20 than "before" the first item :) Believe me I run into this problem before= =20 with my own dirfilter plugins and ever since I adapted the "before" approac= h=20 I never once had that problem anymore.=20 > The kuick plugin did it wrong - isn't it simpler to fix it? IMHO, both need to be fixed. On top of that there should be a note (I can = add=20 it) in PLUGINS file which states that no top/bottom serparators should be=20 drawn by plugins since it is automatically taken care of. Plugins should=20 only draw the separators they need for splitting their own items. > I'm afraid you might have missed some cases, this popupmenu code > (particularly the separators) is a bit touchy (since it's used in so many > different ways), and has been fixed several times already, I'm not too > happy with throwing all this away. Believe me I have tested all the things I can think of including multiple=20 selection of directories, files and a combnation of directories and files. = =20 If you can make my patch fail let me know. On the contrary, with the curre= nt=20 approach I get double lines when I select multiple files as well as the iss= ue=20 with the plugins where the lines could go missing, hence prompting the=20 addition of addSeparator() > Since you investigated the problem - can't this be fixed with the old > logic? Well it can, but believe me it will break at some point. The only reason I= =20 even looked at it is because the problem started moving around on me and I= =20 remember dealing with this issue before... Regards, Dawit A.