Author Topic: Keybinder v2 notes/problems  (Read 9793 times)

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12625
    • Travis build status
Re: Keybinder v2 notes/problems
« Reply #60 on: April 09, 2020, 12:05:06 am »
You most probably have to change the eol property. I'll do it for you in a minute.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12625
    • Travis build status
Re: Keybinder v2 notes/problems
« Reply #61 on: April 09, 2020, 12:13:32 am »
Code: [Select]
         {
+            // FIXME (ph#):This call crashes KeyBinder, I'll fix soon.
+            if (lb->GetPageText(selection) == wxT("Keyboard shortcuts")) break;

What is this? If you don't have this method implemented it should not crash! But you have to rebuild properly!!!
This doesn't probably work on translated C::Bs...
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline Pecan

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 2235
Re: Keybinder v2 notes/problems
« Reply #62 on: April 09, 2020, 12:52:36 am »
Code: [Select]
         {
+            // FIXME (ph#):This call crashes KeyBinder, I'll fix soon.
+            if (lb->GetPageText(selection) == wxT("Keyboard shortcuts")) break;

What is this? If you don't have this method implemented it should not crash! But you have to rebuild properly!!!
This doesn't probably work on translated C::Bs...

What's a translated C::B ?
KeyBinder does not implement the call.

Are you actually able to configure your keys with KeyBinder after the addition of the OnPageChanging() call?

For me, Setting/Editor/KeyboardShortcuts crashes consistenly .
Even after a full rebuild of CB and it's plugins (on windows).

I've spent approximately 12 hours trying to find its' cause.
GDB only shows one frame with no meaningful information.

I'll eventually find the problem, but for now this is a temporary fix to allow users to set their key bindings.
« Last Edit: April 09, 2020, 12:56:10 am by Pecan »

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12625
    • Travis build status
Re: Keybinder v2 notes/problems
« Reply #63 on: April 09, 2020, 10:20:52 am »
This happens because you have a duplicate configurationpanel.h file which doesn't have this method. I don't know why you do this but I doubt it is a good idea.
Next time when you have a problem with a change I've made, you can tell me. :)
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline Pecan

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 2235
Re: Keybinder v2 notes/problems
« Reply #64 on: April 09, 2020, 06:29:51 pm »
This happens because you have a duplicate configurationpanel.h file which doesn't have this method. I don't know why you do this but I doubt it is a good idea.
Next time when you have a problem with a change I've made, you can tell me. :)

I had no problem with your change. In fact I like it.
I only made that temporary fix to give me time to find the source of the crash. And I had all intentions of removing it.
Please don't feel offended.

Thanks for your work.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 12625
    • Travis build status
Re: Keybinder v2 notes/problems
« Reply #65 on: April 09, 2020, 07:16:26 pm »
I don't like it because there is not need to put workarounds in the code. It is fine if trunk/master is broken for some time. If someone is affected he/she can revert the commit that is causing the problem.

BTW: Connecting events to the parent is not a good idea in my opinion. It is too fragile if someday we decide to change the UI of the config dialogs.
I'm talking about the page-changed and destroyed events. For the latter - why do you needed? Can't you use the cancel/apply methods implementations?

If there is something missing from the API it is better to add it. Applying workarounds leads to more work in the future.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline Pecan

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 2235
Re: Keybinder v2 notes/problems
« Reply #66 on: April 09, 2020, 07:55:21 pm »
I don't like it because there is not need to put workarounds in the code. It is fine if trunk/master is broken for some time. If someone is affected he/she can revert the commit that is causing the problem.

BTW: Connecting events to the parent is not a good idea in my opinion. It is too fragile if someday we decide to change the UI of the config dialogs.
I'm talking about the page-changed and destroyed events. For the latter - why do you needed? Can't you use the cancel/apply methods implementations?

If there is something missing from the API it is better to add it. Applying workarounds leads to more work in the future.

I will no longer need the events now that OnPageChanging() is provided.

I used the events to avoid the large overhead caused by scanning the menu structure and building the voluminously stacked arrays used by the original keybinder code from 2005.

The overhead occurred when the user chose Settinging/Editor. There was no need for that overhead until the user actually chose "Keyboard shortcuts."  Using the events cut that overhead in half.

The event method served the same purpose that is now provided by OnPageChanging(). I had already planned to remove the events and use that UI instead.

The duplicate configurationpanel.h was a fix for disappearing wxMessageBoxes on linux from years and years ago. It also will be removed as it's no longer needed.
 

Offline gd_on

  • Lives here!
  • ****
  • Posts: 559
Re: Keybinder v2 notes/problems
« Reply #67 on: April 21, 2020, 10:58:40 am »
A new problem, mix of languages :
Usually, I use a translated version of C::B, in french. Sometimes, I need to switch the interface to English, at least to verify the exact word used in the original interface.
After my last switch, I have seen this dialog (see attachment). The sentence, describing the conflict in English and french are the same, as if there where a mix of languages.
(svn 12069, but may be appeared before).
And in Settings/Environmement/Keyboard Shortcuts Ctrl-W and Ctrl-Shift-W appears twice.
I tried to delete one of them in the right part, but it's not sufficient. I have to delete All to avoid this warning.
gd_on

« Last Edit: April 21, 2020, 11:01:12 am by gd_on »
Windows 10, svn C::B (last version or almost!), WxWidgets 3.1.4 (git), Compilers 8.1.0, 64 bits (seh, posix : gcc, g++ and gfortran in C:\MinGW64) or 32 bits (sjlj, posix in C:\MinGW32). Tests and switch to msys2 and gcc 9.3 compilers.

Offline Pecan

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 2235
Re: Keybinder v2 notes/problems
« Reply #68 on: April 22, 2020, 11:35:01 pm »
A new problem, mix of languages :
Usually, I use a translated version of C::B, in french. Sometimes, I need to switch the interface to English, at least to verify the exact word used in the original interface.
After my last switch, I have seen this dialog (see attachment). The sentence, describing the conflict in English and french are the same, as if there where a mix of languages.
(svn 12069, but may be appeared before).
And in Settings/Environmement/Keyboard Shortcuts Ctrl-W and Ctrl-Shift-W appears twice.
I tried to delete one of them in the right part, but it's not sufficient. I have to delete All to avoid this warning.
gd_on

Until I can re-create and find a fix for this, using separate personalities for differenct languages will solve the problem.

See the CB /p parameter.