From kfm-devel  Tue Jul 16 05:10:13 2002
From: "Dawit A." <adawit () kde ! org>
Date: Tue, 16 Jul 2002 05:10:13 +0000
To: kfm-devel
Subject: Re: PATCH: Open With...
X-MARC-Message: https://marc.info/?l=kfm-devel&m=102679668822533

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.=
=2E.
> >
> > I don't see the point of this change. IIRC the logic was simply the oth=
er
> > 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 sin=
ce
> 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=
=20
working under all the circumstances I tested :) It really does not matter=20
whether the approach followed is "before-items" or "after-items" so long as=
=20
one mode is used consistently throughout.  It just happens that this is not=
=20
currently done consistently in konq_popupmenu.  Specially, the way addPlugi=
ns=20
are handled.=20

Just to make sure we are on the same page by "before-items", I mean the=20
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"=20
instead of item B.  Thus, for example if items C & D are only used in certa=
in=20
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=20
separator at the end.  However, the same is true for the other approach as=
=20
well where you do not want to accidentally start with a separator ; so it=20
does not really matter how it is done so long as the same approach is used=
=20
throughout...  For me it is much much easier to follow the "before-items"=20
approach, hence the implementation in my patch... YMMV

Regards,
Dawit A.