bugGNU Octave - Bugs: bug #41217, GUI Editor could support...

 
 

bug #41217: GUI Editor could support customizable keybindings

Submitter:  Felipe G. Nievinski <fgnievinski>
Submitted:  Mon 13 Jan 2014 03:50:05 PM UTC
   
 
Category:  GUI Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 24 Jul 2014 12:54:47 PM UTC, comment #30: 

Thanks Torsten, maybe all those new features deserve an announcement on the maintainers ML?

I won't be able to test further during the coming weeks (vacancies), but I am sure some will be happy to do so.

Pantxo Diribarne <pantxo>
Group Member
Wed 23 Jul 2014 07:15:13 PM UTC, comment #29: 

Pantxo, this really seems to be a good solution, thank you. I have pushed the patch with changeset
http://hg.savannah.gnu.org/hgweb/octave/rev/c6b89c4a9e63

I have changed the status to "Ready for test".

Torsten Lilge <ttl>
Group Member
Wed 23 Jul 2014 07:08:38 AM UTC, comment #28: 

Hi Torsten,

I come with a totally different approach than discussed below. I simply manually swapped Meta and Ctrl in Qscintilla's shortcut list. Everything works as expected using the attached patch.

If you prefer to stick to what we have discussed below, I also can provide a patch, but the code is much harder to maintain and it reinvents the wheel: qscintilla does a good job as long as the right modifier key is set.

(file #31758)

Pantxo Diribarne <pantxo>
Group Member
Sun 20 Jul 2014 12:01:52 PM UTC, comment #27: 

According to his page http://www.danrodney.com/mac/ (see at the bottom), those emacs-like key bindings are enabled in cocoa applications like safari or textedit (the very basic mac text/rtf editor).

About replacements for non-alternate USING_OSX_KEYS :

  • META + K (DeleteLineRight): this action is handled by octave so "setKey (0)" is fine as is.
  • META (+SHIFT) +E/A => CMD (+SHIFT) +right/left arrow
  • META + L (VerticalCentreCaret): I don't know, I'd "setKey (0)".
Pantxo Diribarne <pantxo>
Group Member
Sat 19 Jul 2014 04:48:30 PM UTC, comment #26: 

One possibility would be to remove the alternate keys that are only set when USING_OSX_KEYS is defined (see file qscicommandset.cpp in the QScintilla sources). For example,  Qt::Key_V | Qt::META is the alternate key for page down.

Not alternate but primary keys for USING_OSX_KEYS:


Qt::Key_A | Qt::META                       ==> Home
Qt::Key_A | Qt::META | Qt::SHIFT           ==> HomeExtend
Qt::Key_A | Qt::META | Qt::ALT | Qt::SHIFT ==> HomeRectExtend
Qt::Key_E | Qt::META                       ==> LineEnd
Qt::Key_E | Qt::META | Qt::SHIFT           ==> LineEndExtend
Qt::Key_E | Qt::META | Qt::ALT | Qt::SHIFT ==> LineEndRectExtend
Qt::Key_K | Qt::META                       ==> DeleteLineRight


Are these the common keys used on a mac for these actions? Can they be replaced by a combination of special keys (end,home etc.)?

Torsten Lilge <ttl>
Group Member
Sat 19 Jul 2014 02:43:29 PM UTC, comment #25: 

I don't know what Qt::Key_C | Qt::META is the shortcut of? I'll try to print the "description" attribute when I have access to my mac machine (at work next week).

From what I could see, most (not all) mac specific keys that involve META are "alternateKey", we should thus at least "setAlternateKey (0)" also.

If you think it's worth I can produce a patch that disables all mac specific shortcuts involving META (as in the excerpt from comment #23) or replace META by CTRL.

Pantxo Diribarne <pantxo>
Group Member
Sat 19 Jul 2014 11:28:33 AM UTC, comment #24: 

Is, e.g. Qt::Key_C | Qt::META the shortcut for SelectionCopy? I am wondering why


cmd_set->find (QsciCommand::SelectionCopy)->setKey (0);


is not sufficient for disabling this. What version of QScintilla are you using?

Concerning the possible duplicates: The editor takes control of some of the Qscintilla hotkeys (those that are disabled  by setKey (0)). The other hotkeys are those to navigate the cursor (cursor keys, home, end etc. with or without selection). IMHO it is rather unlikely that one wants to use this key sequences for other actions. There are some exceptions like "/", "[", "]", "\\" + modifier but these keys can't be used on many keyboard layouts.

Torsten Lilge <ttl>
Group Member
Fri 18 Jul 2014 04:15:24 PM UTC, comment #23: 

AFAICS QScintilla has a hard coded value for CMD so I don't think it is possible to instruct it to do the CTRL/META swap.

I think I have found a clue on how to solve the problem. I have made the following change and all offending shortcuts now work:


diff -r a311cf821fb0 libgui/src/m-editor/octave-qscintilla.cc
--- a/libgui/src/m-editor/octave-qscintilla.cc        Thu Jul 17 19:27:42 2014 +0200
+++ b/libgui/src/m-editor/octave-qscintilla.cc        Fri Jul 18 18:01:24 2014 +0200
@@ -65,6 +65,34 @@
   cmd_set->find (QsciCommand::LineDelete)->setKey (0);
   cmd_set->find (QsciCommand::LineCut)->setKey (0);
   cmd_set->find (QsciCommand::LineCopy)->setKey (0);
+  QList< QsciCommand * > cmd_list = cmd_set->commands ();
+  for (int i = 0; i < cmd_list.length (); i++)
+    {
+      int cmd_key = cmd_list.at (i)->key ();
+      switch (cmd_key)
+        {
+          case Qt::Key_C | Qt::META :
+          case Qt::Key_A | Qt::META :
+          case Qt::Key_N | Qt::META :
+          case Qt::Key_P | Qt::META :
+          case Qt::Key_F | Qt::META :
+          case Qt::Key_B | Qt::META :
+          case Qt::Key_V | Qt::META :
+            cmd_list.at (i)->setKey (0);
+        }
+      cmd_key = cmd_list.at (i)->alternateKey ();
+      switch (cmd_key)
+        {
+          case Qt::Key_C | Qt::META :
+          case Qt::Key_A | Qt::META :
+          case Qt::Key_N | Qt::META :
+          case Qt::Key_P | Qt::META :
+          case Qt::Key_F | Qt::META :
+          case Qt::Key_B | Qt::META :
+          case Qt::Key_V | Qt::META :
+            cmd_list.at (i)->setAlternateKey (0);
+        }
+    }
 #else


This is clearly not the right approach. Qscintilla's key/alternateKey should be deleted only when needed, i.e. when octave tries to set a shortcut that is already defined.
Could the shortcut manager check for duplicates not only among octave's but also among qscintilla default keys and alternate keys? 


Pantxo Diribarne <pantxo>
Group Member
Fri 18 Jul 2014 04:57:04 AM UTC, comment #22: 

So how can we fix this?
Can the ctrl/meta swap in QScintilla be disabled?

Torsten Lilge <ttl>
Group Member
Thu 17 Jul 2014 06:58:14 PM UTC, comment #21: 

Sorry for the misinterpretation, the non working shortcuts used to work before my patch. The problem is that preventing Qt to interpret CMD as CTRL I also introduced conflicting shortcuts between Octave an QScintilla standardCommands. If I change "editor paste" to "META(CMD)+ALT+V" it works fine.

QScintilla assumes the CTRL/META swap is on, and defines default shortcuts that involve META key (see qscicommandset.cpp in qscintilla sources). For example META+V triggers "PageDown" which is what I observe when I try to paste.

Pantxo Diribarne <pantxo>
Group Member
Tue 15 Jul 2014 06:15:10 PM UTC, comment #20: 

I just see that "New Script", "New Function" and "Open" are just copied into the editor menus as well (like the debug actions) whereas Copy and Paste are doubly defined. Could you please test which of these actions don't work in the editor?

Torsten Lilge <ttl>
Group Member
Tue 15 Jul 2014 05:42:34 PM UTC, comment #19: 

I guess you mean that triggering the doubly defined actions via shortcuts does not work? Have you chosen same or different shortcuts in main and editor window for these actions?

Torsten Lilge <ttl>
Group Member
Tue 15 Jul 2014 04:31:00 PM UTC, comment #18: 

Thanks, I saw on the mailing list that you had trouble applying the patch because it was written against default. I'll take care to switch to the gui-release branch before writing GUI patches in the future.

I just noticed that some doubly defined actions (at least "copy", "paste", and "new"), i.e. those that can be triggered from the editor or from the main menu, don't work in the editor but do work elsewhere. OTOH, the debug actions (that trigger dbstep dbcont ...), which are defined once and copied on both menus, do work both in the editor and in the command window. Should I file a bug report or should we discuss this issue here?

I could check that it used to work in 3.8.0 and that the problem is not linked to this changeset http://hg.savannah.gnu.org/hgweb/octave/rev/bfb735b70978

Do you have an idea I could test?
 
About shortcuts for the dock-widget I have no precise idea, I'll think about it.


Pantxo Diribarne <pantxo>
Group Member
Mon 14 Jul 2014 07:11:43 PM UTC, comment #17: 

Pantxo, thanks for the patch. I have pushed it with changeset
http://hg.savannah.gnu.org/hgweb/octave/rev/58f1178f49ef
into the gui-release branch.

Do you have a suggestion concerning the shortcuts for the dock widgets?

Torsten Lilge <ttl>
Group Member
Thu 10 Jul 2014 04:29:17 PM UTC, comment #16: 

Hi Torsten,

Sorry for the delay. I attached a patch in which I tried to stick as much as possible to the shortcuts that have been chosen for linux and windows (for most of them ctrl->CMD).

As function keys have predefined, system wide, behavior on Mac I prepended any shgortcut involving F* by option modifier.

Also, at least French keyboards need Shift to type numbers on laptops, so that Ctrl+2 or Ctrl+Shift+2 is not a good choice ...

Thank you for the possibility to import export shortcuts, it will be very handy.



(file #31697)

Pantxo Diribarne <pantxo>
Group Member
Thu 12 Jun 2014 05:33:12 PM UTC, comment #15: 

Changeset
http://hg.savannah.gnu.org/hgweb/octave/rev/3f6280d0a36b
introduces the possibility to export and import shortcut sets.

Torsten Lilge <ttl>
Group Member
Thu 05 Jun 2014 08:21:24 PM UTC, comment #14: 

For me, your proof of concept is acceptable but I am not a MAC user.

I will think about a possibility to store/load shortcuts in/from a separate file.

Torsten Lilge <ttl>
Group Member
Sat 17 May 2014 01:59:21 PM UTC, comment #13: 

First thanks again for this beautiful tool.

For the first default set, if you think the proof of concept from bug #41337 (see comment 17) is acceptable, I can provide a patch for mac. I will have to change many default shortcuts as many of the current ones are already defined as system wide shortcuts on mac (e.g. F9 -> show all applications windows) and thus don't reach octave.

Now about the second set, I am not sure it should be hard coded: it could be read from a shrotcut_settings file. If the shortcut_manager was able to import/export shortcut_settings files (I don't know how hard it is to have such feature), we could have as much base shortcut sets as we want. Those sets (platform specific) could also be produced and shared by octave users.

Pantxo Diribarne <pantxo>
Group Member
Sat 10 May 2014 09:41:15 AM UTC, comment #12: 

I have added a second set of shortcuts to the shortcut manager (changeset http://hg.savannah.gnu.org/hgweb/octave/rev/95249367d6fa). Up to now, the default shortcuts of both sets are the same. Suggestions on the defaults for the second set are welcome.

For platform depending configurations I suggest to use conditional compilation in shortcut_manager::do_init_data (). Can anyone provide the related shortcuts suitable for mac os?

Torsten Lilge <ttl>
Group Member
Sat 10 May 2014 09:31:16 AM UTC, comment #11: 

The user preference for disabling global shortcuts when the terminal has focus (comment #10) was added with changeset
http://hg.savannah.gnu.org/hgweb/octave/rev/f017240310fb

Torsten Lilge <ttl>
Group Member
Sun 06 Apr 2014 09:24:56 PM UTC, comment #10: 

Thanks for the clarifications. You have my vote for this being a preference.

Pantxo Diribarne <pantxo>
Group Member
Sun 06 Apr 2014 09:14:37 PM UTC, comment #9: 

When the editor has focus and is docked, all shortcuts are active.
Disabling the shortcuts when the focus changes to the terminal could become a user preference if desired.

Torsten Lilge <ttl>
Group Member
Sun 06 Apr 2014 08:33:10 PM UTC, comment #8: 

Right, I see now.
There is also a specific bug dedicated to readline default settings (bug #40156). You emphasize it's probably not two completely distinct features.
I fear shortcuts that work only if neither the editor nor the terminal has focus (i.e. probably less than 1% of the time), won't be very useful.

Pantxo Diribarne <pantxo>
Group Member
Sun 06 Apr 2014 07:06:21 PM UTC, comment #7: 

When the terminal gets the focus, almost all shortcuts of the gui are disabled because they could conflict with the freely configurable shortcuts for gnu-readline which is used by the command line interface. Even the default Ctrl+Shift+F does not work when the terminal has focus.

Torsten Lilge <ttl>
Group Member
Sun 06 Apr 2014 06:43:10 PM UTC, comment #6: 

I can confirm the conflicting shortcut issue but there was no conflict in my test:
actually I had set "Main:Edit:Find in Files" (Ctrl+X is not defined in the Main menu) not "Editor:Edit:Find File" (which can't currently be set) and was trying the shortcut while the terminal had focus.
I assume "Ctrl+X,Ctrl+F" should work but it doesn't when the terminal has focus (works great e.g. in the command history window).

Pantxo Diribarne <pantxo>
Group Member
Sun 06 Apr 2014 06:15:54 PM UTC, comment #5: 

I can confirm this issue with Ctrl+X,Ctrl+F.

If you try, e.g. Ctrl+B,Ctrl+F, everything should work fine. The Ctrl+X is used by "Cut" (you should be able to confirm that when having something selected in the editor). I will extend the check for duplicates to take this case into consideration.

Torsten Lilge <ttl>
Group Member
Sun 06 Apr 2014 03:35:31 PM UTC, comment #4: 

I tried to set emacs-like shortcuts (actually I typed "Ctrl+X, Ctrl+F" in the shortcut dialog for "Edit:Find file"). The shortcut appears in the the menu but don't trigger anything when typed.
According to Qt doc [1] this should be possible.

Now about platform specific keybindings, I'll wait for the next step :-)

[1] http://qt-project.org/doc/qt-5.0/qtgui/qkeysequence.html#gnu-emacs-style-key-sequences

Pantxo Diribarne <pantxo>
Group Member
Sun 06 Apr 2014 02:49:02 PM UTC, comment #3: 

Yes, please post your comments here.

I see platform specific defaults (as they are not covered by the platform specific shortcuts provided by Qt itself) as the next step. The list that is used for storing the shortcuts ans its defaults can easily be initialized depending on the target platform.

At the moment only view menus are already managed by the shortcut-manager. At the moment I am working on the scintilla-internal shortcuts for basic edit functions like duplication of the current etc..


Torsten Lilge <ttl>
Group Member
Sun 06 Apr 2014 02:23:47 PM UTC, comment #2: 

Thanks very very much Torsten for this new feature. This will lets users adopt the editor more easily now they can mimic some of their favorite editor's shortcuts.

I tested a bit and have a few comments. Should I post theim here?
It's mainly about being able to load platform specific defaults, and being able to use emacs-like shortcuts.


Pantxo Diribarne <pantxo>
Group Member
Tue 01 Apr 2014 07:38:17 PM UTC, comment #1: 

I have pushed changeset
http://hg.savannah.gnu.org/hgweb/octave/rev/086093fbdc1a
which implements a shortcut manager.

The manager includes changing shortcuts via the settings dialog and updating the action's shortcuts. For now, not all actions are using the shortcut manager. If the structure proves to be robust in everyday use I will all the other actions.
This bug report is left open because of those still missing actions.


Torsten Lilge <ttl>
Group Member
Mon 13 Jan 2014 03:50:05 PM UTC, original submission:  

splitting off bug #41205.

please keep requests for specific keybindings separate from the current generic feature; e.g., for Windows-specific, bug #40156.

Felipe G. Nievinski <fgnievinski>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #31758:  swap_scintilla_keys.patch added by pantxo (2KiB - text/x-diff)
file #31697:  shortcuts_mac.patch added by pantxo (13KiB - text/x-diff)

 

Carbon-Copy List
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by fgnievinski (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-12-21 ttl StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2014-07-23 ttl StatusIn Progress Ready For Test
    2014-07-23 pantxo Attached File- Added swap_scintilla_keys.patch, #31758
    2014-07-10 pantxo Attached File- Added shortcuts_mac.patch, #31697
    2014-04-01 ttl StatusNone In Progress
    2014-01-16 jordigh Dependencies- bugs #41180 is dependent
    2014-01-16 jordigh Dependencies- bugs #41109 is dependent

    Back to the top

    Powered by Savane 3.14-f13d.
    Corresponding source code