[immodule-qt] Introducing pluggable popup menu

[YamaKen]

Hi Lars, thank you for good comment.

At Thu, 5 Aug 2004 13:23:07 +0200,
lars at trolltech.com wrote:
> We definitely need a way to add something to the context menu, so this is a 
> very good thing.
> 
> But I do have a few comments/questions about the implementation.
> 
> In Qt 4, you can't use a QPtrList anymore (as it's defined in the compat 
> library), but will have to use a QList. The semantics is is also easier to 
> understand if you return a value and not a pointer. As Qt containers are 
> implicitly shared this is cheaper. 

I'll fix so. Thanks for let me know.

> Is there a reason why you have added a menus() method instead of directly 
> making the exportMenusInto() method virtual?

For flexibility. I'm regarding menus() as main and flexible
function, in contrast with exportMenusInto() as
utility function only for convenience in limited situation.

> I would prefer if you used an enum instead of the "bool insertSeparator" and 
> renamed exportMenusInto to appendMenusTo. The reason to use an enum is that 
> later on the code written is a lot clearer and it can be extended more 
> easily.
> 
> So I would propose
> 
> enum IMMenuAction {
>  InsertSeparator,
>  NoSeparator
> };
> virtual void addIMMenus(QMenu *menu, IMMenuAction = InsertSeparator);

I also prefer your idea in the point of view from
self-explanative code. It will help fast reading of the
sourcecode as following.

qic->exportMenusInto( popup, IMMenuAction::NoSeparator );

But the name 'addIMMenus' is too ambiguous and
confusable. Following code fragment may be recognized as 'add
popup to qic as IMMenu'. So I select the word 'export' and
'into' to indicate operational direction although the method
name like Smalltalk is strange in the Qt world.

qic->addIMMenus( popup );

One alternative is 'addMenusTo' which is familiar with naming
convention of Qt. What do you think about it?

> Also your current implementation limits you to adding submenus. Is this good 
> enough or would one sometimes also need to just add a single item to the 
> menu?

Nobody has experienced enough to determine it yet, I think.

Yes, single QAction could be useful for some input methods. We
should think it deeper. But I think that current implementation
that provides conservative popup menus is sufficient for
existing input methods. So I suggest following plan.

- Use current implementation for Qt3
- Redesign it from scratch for Qt4 based on its new features

> Another question I have is about QMultiInputContext. Why do we need this class 
> at all? Wouldn't it be better to build the support for IM switching directly 
> into Qt?

No. We don't need QMultiInputContext or any other switching
support in any Qt part. So I had removed the QMultiInputContext
dependency from QLineEdit and QTextEdit. See again my previous
patch.

I had already removed any other QMultiInputContext dependency
from Qt in yesterday morning. See following
ChangeLog. QMultiInputContext had reorganized as ordinary IM
plugin. But the trouble of freedesktop.org service prevents me
from diffing or committing the code...

http://freedesktop.org/pipermail/immodule-qt/2004-August/000281.html

> On Wednesday 04 August 2004 16:28, YamaKen wrote:
> > At Wed, 04 Aug 2004 23:14:07 +0900,
> >
> > yamaken at bp.iij4u.or.jp wrote:
> > > [1  <text/plain; US-ASCII (7bit)>]
> > > Hi all, I've implemented pluggable popup menu feature. Please
> > > review the design and implementation of attached patch.
> >
> > Oops, I had sent older patch. Refer new one in this mail.

-------------------------------
YamaKen  yamaken at bp.iij4u.or.jp