PATCH: Open With...

[Dawit A.]

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 patch
> > 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 need 
to add separators should be done before adding the item that you want to 
separate. A couple of reasons for this.  The first being that the popup menu 
is context based.  As such things are shown based on context.  This means you 
do not want to draw a separator that might otherwise be unnecessary if that 
particular group of items are not supposed to be shown...  The other issue is 
that you are more likely to add a unwanted separator "after" the last item 
than "before" the first item :)  Believe me I run into this problem before 
with my own dirfilter plugins and ever since I adapted the "before" approach 
I never once had that problem anymore. 

> 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 
it) in PLUGINS file which states that no top/bottom serparators should be 
drawn by plugins since it is automatically taken care of.  Plugins should 
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 
selection of directories, files and a combnation of directories and files.  
If you can make my patch fail let me know.  On the contrary, with the current 
approach I get double lines when I select multiple files as well as the issue 
with the plugins where the lines could go missing, hence prompting the 
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 
even looked at it is because the problem started moving around on me and I 
remember dealing with this issue before...

Regards,
Dawit A.