A Hack for cbKeyBinder Edit Menu Bug

[Biplab]

Hi Pecan,

I've prepared a fix for the annoying Edit-(Cut/Copy/Paste) bug. The fix is as follows.

Code

Index: src/plugins/contrib/keybinder/keybinder.h
===================================================================
--- src/plugins/contrib/keybinder/keybinder.h	(revision 3915)
+++ src/plugins/contrib/keybinder/keybinder.h	(working copy)
@@ -319,6 +319,10 @@
 	//! Adds the given key binding to this command.
 	void AddShortcut(const wxKeyBind &key, bool updateMnu=true) {
 		if (m_nShortcuts >= wxCMD_MAX_SHORTCUTS) return;
+		// Don't add the shortcut if it has been assigned
+		for (int i = 0; i < m_nShortcuts; ++i)
+            if (m_keyShortcut[i].Match(key))
+                return;
 		m_keyShortcut[m_nShortcuts++] = key;
 		if (updateMnu) Update();
 	}

This prevents wxKeyBinder from duplicating shortcut. The patch works. But I'm not sure whether this would break other parts or not. Please have a look at it.

If you find it OK, then please apply it. I would be offline after this post, so I won't be able to reply you.

Patch is also attached with this post.

Best Regards,

Biplab

[attachment deleted by admin]

[Pecan]

Hi Pecan,

I've prepared a fix for the annoying Edit-(Cut/Copy/Paste) bug. The fix is as follows.

Code

Index: src/plugins/contrib/keybinder/keybinder.h
===================================================================
--- src/plugins/contrib/keybinder/keybinder.h	(revision 3915)
+++ src/plugins/contrib/keybinder/keybinder.h	(working copy)
@@ -319,6 +319,10 @@
 	//! Adds the given key binding to this command.
 	void AddShortcut(const wxKeyBind &key, bool updateMnu=true) {
 		if (m_nShortcuts >= wxCMD_MAX_SHORTCUTS) return;
+		// Don't add the shortcut if it has been assigned
+		for (int i = 0; i < m_nShortcuts; ++i)
+            if (m_keyShortcut[i].Match(key))
+                return;
 		m_keyShortcut[m_nShortcuts++] = key;
 		if (updateMnu) Update();
 	}

This prevents wxKeyBinder from duplicating shortcut. The patch works. But I'm not sure whether this would break other parts or not. Please have a look at it.

If you find it OK, then please apply it. I would be offline after this post, so I won't be able to reply you.

Patch is also attached with this post.

Best Regards,

Biplab

Seems to me that all this code is doing is testing if "the key" has already been added to "this" command previously. So, if "Ctrl-A" has already been added to menu item "Select All" then don't add "Ctrl-A" to "Select all" again.

I can't figure out how this solves the miss-assigned menu id/key, but I'll study it more.

What's happening: when the menu structure changes, cbKeybinder.ini has old memorized ids that nolonger match those in the new menu stucture.

So when it fails matching its stale id to the new menu structure, it searches for a matching "label". In this particular case, it unfortunately matches the wrong (duplicate)"cut"(cut marked) label and assigns the old key definition of "cut"(cut line).

The "search for label when ids fails" is necessary, else we couldn't use wxNewId() to get dynamic ids, and the dynamic key assignments of plugins (et.al.) would not work.

I either have to find a way to delete the cbKeybinder.ini file dynamically when the menu structure changes(I don't know how to tell), or find a way to differentiate "cut" from "cut" (they seem the same in this universe).

I've been thinking about adding a table of "unchanging" ids (derived from the xrc) and excluding them from dynamic searchs.

Edit: Searching back in my collection of old keybinder .ini files, it seems that xrc ids also change their value. Damn!

[Biplab]

Thanks for your reply.

I've used that title ("A hack ..") because I couldn't prepare a proper fix.

Right now I've little time to look into it's details as I'm preparing to go on a holiday trip. Will wait for your fix.

Best Regards,

Biplab

[Pecan]

Actually, in the last commit, I removed the code that caused the problem in the first place.

Unfortunately, it removes the ability to assign a shortcut when the menu ids mis-match (whenever the menu structure changes).

But it seems that searching by "text" causes more grief than just not assigning at all. We'll see in the comming months.


Have a nice vacation. Looking forward to seeing you back.

[Biplab]

Have a nice vacation. Looking forward to seeing you back.

Thanks.   Tonight I'm starting my journey.

I've been thinking about adding a table of "unchanging" ids (derived from the xrc) and excluding them from dynamic searchs.

Edit: Searching back in my collection of old keybinder .ini files, it seems that xrc ids also change their value. Damn!

I have a crazy idea. What if you replace id with immediate parents' labels? As keybinder searches for it's children recursively, the label of immediate parent may be attached with the children's label. For root, a word NULL may be added. Let me explain:

File
|
--> New
| |
|  --> Empty file
--> Open

Save it as:
NULL|New|New...|Ctrl-N|
New|Empty file|Create an empty file||
NULL|Open|Open..|||

As the immediate parent's label will not change, this method may be helpful.

So instead of searching for menu ids, let's search for a combination of immediate parent's label+menu label.

But beware, I've not tested it.

Best Regards,

Biplab