[immodule-qt] Introducing pluggable popup menu

[Lars Knoll]

On Friday 06 August 2004 00:56, YamaKen wrote:
> 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.

Ok.

> > 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?

That sounds good and as you said follows our naming conventions :)

> > 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

In Qt 4 the simplest way would be probably to have a method that returns a 
list of QActions. Like that you can add a whole menu tree (as Qt 4 actions 
can have a menu themselves). The other advantage is that you can add the 
actions to other places as menus as well (e.g. to a specialized tool bar).

So how about 

virtual QList<QAction *> actions() const;

for QInputContext in Qt 4?

> > 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. 

I know. But now QMultiInputContext is an IM plugin that loads the other 
plugins and forwards things to them. What's the advantage of this design over 
a design where we move the switching code directly into Qt 4?

Cheers,
Lars