"Bo Peng" <[EMAIL PROTECTED]> writes:

> On 10/16/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:
>> Author: bpeng
>> Date: Tue Oct 16 19:48:37 2007
>> New Revision: 20991
>>
>> URL: http://www.lyx.org/trac/changeset/20991
>> Log:
>> List currently used shortcuts in the shortcuts panel of the preference dialog
>
> Lfun and shortcuts can be edited in place so shortcut modification can
> be done in this way. If I add an empty item, new shortcut can be
> inserted. I guess I can also categorize these shortcuts.

A few comments on the code itself:

        KeyMap::Table::const_iterator it = km.begin();
        KeyMap::Table::const_iterator it_end = km.end();
        for (; it != it_end; ++it) {
                // a LFUN_COMMAND_PREFIX
                if (it->table.get()) {
                        addKeyMap(mathItem, editItem, bufferItem, fontItem, 
layoutItem, miscItem,
                                *it->table.get(), prefix + " " + 
km.printKeySym(it->code, it->mod.first));
                        continue;
                }

This is definitely _not_ the way to go. If you need some code to get a
vector of functions and keysequences, just add that to KeyMap.h. The
GUI code shall not have to know about the structure of key sequences.

Similarly, the printing of keysyms is not a good idea. If you have a
key sequence, just use keyseq.print(). it would be better to change
display the shortcuts using the GUI view, not the LyX internal
representation. This is especially important on the mac. I understand
however that it would defeat the simple editing of bindings that you
have put in place. However the long term solution has to be a proper
UI (people are not supposed to know that the codes for page up/down
are prior/next, for example).

                QTreeWidgetItem * newItem = NULL;
                string const action = lyxaction.getActionName(it->func.action);
                if (action == "self-insert")
                        continue;
                else if (contains(action, "math"))
                        newItem = new QTreeWidgetItem(mathItem);

This is super-ugly (but  guess that you know it...). If you want to do
categorization, please do it in LyXAction: add an enum with values
Math, Buffer, Hidden (for self-insert)... We should not make the
selection based on names.

I do not like much to see such ugly code committed to svn, to be
frank. It has a big potential of just remaining like that.

For the future, when it comes to saving the bindings, we shall find a
way to save only the differential wrt the main bind files (cua, for
ex.). Otherwise, if the all bindings are just dumped, we'll have
difficulties with upgrades.

JMarc

Reply via email to