PATCH: Open With...

[Dawit A.]

On Monday 15 July 2002 21:57, Dawit A. wrote:
> 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.

On second though... I take back almost all that I said except that my patch 
working under all the circumstances I tested :) It really does not matter 
whether the approach followed is "before-items" or "after-items" so long as 
one mode is used consistently throughout.  It just happens that this is not 
currently done consistently in konq_popupmenu.  Specially, the way addPlugins 
are handled. 

Just to make sure we are on the same page by "before-items", I mean the 
following.  If we have

item A
item B
item C
item D

and want a separator b/n items B and C, then the separator is "tied to item C" 
instead of item B.  Thus, for example if items C & D are only used in certain 
contexts:

item A
item B

if ( context )
{
  addSeparator();
 item C;
 item D;
}

By comparison the "after-items" will tie the separator to item B:

item A
item B
addSeparator ()

if (context)
{
  item C
  item D
  addSeparator ()
}

The only down side is that you have to be careful not to be left with a 
separator at the end.  However, the same is true for the other approach as 
well where you do not want to accidentally start with a separator ; so it 
does not really matter how it is done so long as the same approach is used 
throughout...  For me it is much much easier to follow the "before-items" 
approach, hence the implementation in my patch... YMMV

Regards,
Dawit A.