customizing multiple keys shortcut doesn't work

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Simon.pasquet
Posts: 4
Joined: Thu Aug 14, 2014 6:59 pm

customizing multiple keys shortcut doesn't work

Postby Simon.pasquet » Sat Aug 16, 2014 10:39 pm

Hi!
My name Simon, I'm new here. I'm an architecture student interrested in coding and I would like to help.
English is not my mother thong, feel free to correct myself on gramar and spelling :D I'm here to learn.

I would like to report a bug and may be work on it.
When you try to customize shortcuts with several keys in it, the second key erase the first.
It might be a bug because, wall in Arch's workbench has the "wa" shortcut.

I have looked in the code, and the function responsible of this is:
AccelLineEdit::keyPressEvent() in ./src/Gui/Widgets.cpp

Presently, I have fixed some aspect of the bug but it's not perfect.
I can customize the shortcut I want, adding "," before the key, but there isn't any validation on the string passed. For exemple the number of keys that can be used. (4 for Qtshortcut)
I have never used Qt and my nivel in c++ is quit low, I use to code in python.
For exemple, line 390-391, I dont understand why there are parenthesis around QString, nor what is ks(). is ks() a function?:

Code: Select all

QKeySequence ks(Qt::SHIFT+key);
txt += (QString)(ks)


I began to work on this bug to learn. So I report it if someone with better ability want to fix it, otherwise I can sumit what I've done when finished but it might be dirty.

the bug on Mantis
http://www.freecadweb.org/tracker/view.php?id=1694

OS: Ubuntu 14.04.1 LTS
Word size: 64-bit
Version: 0.15.3881 (Git)
Branch: master
Hash: 7665e34a3a844dc0e21aa86375ff9de1e632ec36
Python version: 2.7.6
Qt version: 4.8.6
Coin version: 4.0.0a
SoQt version: 1.6.0a
OCC version: 6.7.0
wmayer
Site Admin
Posts: 15487
Joined: Thu Feb 19, 2009 10:32 am

Re: customizing multiple keys shortcut doesn't work

Postby wmayer » Sun Aug 17, 2014 8:11 am

Hi Simon,

thanks for looking into this issue!
For exemple, line 390-391, I dont understand why there are parenthesis around QString, nor what is ks(). is ks() a function?:
C++ has many stuff other languages do not have and thus it's sometimes hard to understand the code for non-C++ coders.

Code: Select all

QKeySequence ks(Qt::SHIFT+key);
So, in this case "ks" is an instance of "QKeySequence".

Code: Select all

txt += (QString)(ks)
In C++ you can implement special operator functions in a class interface which are used to do an automatic conversion. So, the relevant part of the class QKeySequence looks like this:

Code: Select all

class Q_GUI_EXPORT QKeySequence
{
public:
...
    operator QString() const;  // <<=== this is the above function
    operator QVariant() const;
    operator int() const;
...
};
So, this means you can create a QString representation of the QKeySequence. Actually it suffices to write:

Code: Select all

txt += (QString)ks; 
i.e. the parenthesis around "ks" are not needed. As you can see from the class interface you can also convert "ks" into an "int" or "QVariant" representation.
User avatar
yorik
Site Admin
Posts: 11700
Joined: Tue Feb 17, 2009 9:16 pm
Location: São Paulo, Brazil
Contact:

Re: customizing multiple keys shortcut doesn't work

Postby yorik » Sun Aug 17, 2014 3:54 pm

I think one good way to solve this would be:
- you press one key: it works like now
- you press a second key: the shortcut becomes a two-keys shortcut (add a comma)
- you press a third key: it switch back to one-key shortcut.

I don't think it's possible to have two-key shortcuts with modifier (Ctrl, shift), so one needs to decide how to treat that (only apply the modifier if we have only one key?)
Simon.pasquet
Posts: 4
Joined: Thu Aug 14, 2014 6:59 pm

Re: customizing multiple keys shortcut doesn't work

Postby Simon.pasquet » Mon Aug 18, 2014 12:15 pm

Thank you for your responses.

wmayer:
I understand the operator stuff. It's seem like an operator overloading but instead of overloading you create a new one. Although I don't understand the advantage over

Code: Select all

txt += ks.toQString()
. May be it's just syntactic sugar.

yorik:
Qt accept two keys (exactly four) with modifier. for example "Ctrl+D, Ctrl+G" work for me ("Ctrl+Q, Ctrl+Alt+A, Ctrl+Shift+S, Z" as well).
- you press one key: it works like now
- you press a second key: the shortcut becomes a two-keys shortcut (add a comma)
It's already done.
- you press a third key: it switch back to one-key shortcut.
Now, to clear the shortcut, I press Backspace.
What I can do is : set the max length to two-keys and when you reach the max length, the shortcut switch back to one-key.
Like this, if one day we want three or four-keys shortcut, we will need only to change a constant.
I see three way to accomplish that :
- analyzing the text, counting the number of "," ;
- set a Class Variable that actually count the number of key that have been Pressed ;
- find if Qt offer a standard way to do that.

I find the same function (AccelLineEdit::AccelLineEdit) in: ./src/Tools/plugins/widget/customwidgets.cpp. what should i do with it? It doesn't seem to change anything when I modify it.
wmayer
Site Admin
Posts: 15487
Joined: Thu Feb 19, 2009 10:32 am

Re: customizing multiple keys shortcut doesn't work

Postby wmayer » Mon Aug 18, 2014 1:08 pm

I understand the operator stuff. It's seem like an operator overloading but instead of overloading you create a new one. Although I don't understand the advantage over
This is very old code, I think roughly 10 years now. I assume at this time the only way was to use the QString operator to get the short cut as string. Now when checking the docs it says that the QString operator is marked as obsolete and in new code it should be "toString(QKeySequence::NativeText)".
I find the same function (AccelLineEdit::AccelLineEdit) in: ./src/Tools/plugins/widget/customwidgets.cpp. what should i do with it? It doesn't seem to change anything when I modify it.
All widgets there are only dummy widgets. Their only purpose is to provide a graphical representation in order to be used inside QtDesigner.
User avatar
DevJohan
Posts: 41
Joined: Sun Jul 13, 2014 2:36 pm
Location: Stockholm, Sweden

Re: customizing multiple keys shortcut doesn't work

Postby DevJohan » Mon Aug 18, 2014 1:25 pm

Code: Select all

txt += (QString)ks;
This is really a C-style cast and IMO (not just my opinion) should very rarely if ever be used in C++code. In this case if there are ambiguities I would use

Code: Select all

txt += static_cast<QString>(ks);
or if this is dependent on a deprecated operator

Code: Select all

txt += ks.toQString();
. The reason C-style casts should be avoided is they cast just about anything without requiring checks if the cast is sensible. If the cast is sensible or well defined C-style casts do what you expect, if not you're down to the bare metal.
Last edited by DevJohan on Mon Aug 18, 2014 2:37 pm, edited 1 time in total.
User avatar
yorik
Site Admin
Posts: 11700
Joined: Tue Feb 17, 2009 9:16 pm
Location: São Paulo, Brazil
Contact:

Re: customizing multiple keys shortcut doesn't work

Postby yorik » Mon Aug 18, 2014 2:26 pm

Simon.pasquet wrote:What I can do is : set the max length to two-keys and when you reach the max length, the shortcut switch back to one-key.
Like this, if one day we want three or four-keys shortcut, we will need only to change a constant.
Perfect! Or let the possibility to set 4 letters already, why not after all! (Not sure a 4-letter shortcut can still be called a shortcut :) )
Simon.pasquet
Posts: 4
Joined: Thu Aug 14, 2014 6:59 pm

Re: customizing multiple keys shortcut doesn't work

Postby Simon.pasquet » Thu Aug 21, 2014 10:14 pm

Hi!

The first bug is fixed but on the other side other bugs appears:
1) I have added the meta(windows/super) modifier. Maybe i should not have because it collapse with Gnome. For example I can't enter the shortcut "Meta+s" because it is intercepted first by Gnome;

2) when you press shift+alt in this order (or ctrl+shift+alt, shift+ctrl+alt still in this order) the "ៀ" string is printed.
but when I added the Meta modifier, the bug disappeared :shock: ( and it reappear if I disable the Meta modifier. :lol: );

3) when you press Altgr, "ៀ" is also printed. I think that's because this key is not a Qt::KeyboardModifier but a Qt::Key.
However this key doesn't seem to have a string representation like the other Qt::Key:
In this post (in french):
http://www.developpez.net/forums/d11619 ... trl-shift/
They propose to look in the definition of keyname[] in qkeysequence.cpp. Indeed AltGr is not defined there. They also suggest to intercept manualy the key. I don't feel able to do that :( .

To finish with AltGr, in the documentation of Qt::nameSpace we can read.
On Windows, when the KeyDown event for this key is sent, the Ctrl+Alt modifiers are also set.
unfortunately, the best way to fix that might be to forbid the use of this very special key.

2) et 3) are present in the official development build too. Should I create a new bug on Mantis?
and 3) might be a bug of Qt.



To conclude, perhaps it is time to share my work, because i don't know how many time it would take to fix the bug 2) and 3).

Nota: I don't have change yet the "(QString)ks;" to "Something.toString(QKeySequence::NativeText)"
Simon.pasquet
Posts: 4
Joined: Thu Aug 14, 2014 6:59 pm

Re: customizing multiple keys shortcut doesn't work

Postby Simon.pasquet » Sun Aug 24, 2014 10:41 pm

I have post a patch on mantis.
I used:
$git format-patch master --stdout > multipleKeysShortcuts.patch
I don't know if it's the right way to do it, nor the right place to post it.

the 2) 3) bug of my previous post are fixed and ks.toString() is used instead of (QString)(ks)

http://www.freecadweb.org/tracker/view.php?id=1694
User avatar
saso
Posts: 1418
Joined: Fri May 16, 2014 1:14 pm
Contact:

Re: customizing multiple keys shortcut doesn't work

Postby saso » Sun Aug 24, 2014 10:44 pm