Support adding gestures to enable configuration profiles#8844
Conversation
|
Apologies if this is not relevant, but what stops one from hijacking a
gesture which is used elsewhere for an important nvda feature?
|
Could you give a real world use case scenario why someone should "hijack" such a gesture? |
I expect the concern is around accidental name matching with existing gestures, thereby unintentionally "hijacking" the gesture. |
|
I think there is no such risk, as this would only occur when the creation of a new profile causes the script name of the newly created profile to be equal to another script. since profile scripts have the "script_profile_" prefix, this is pretty unlikely. |
| if not c.isalnum() and c != "_": | ||
| invalidChars.add(c) | ||
| for c in invalidChars: | ||
| name=name.replace(c,'_') |
There was a problem hiding this comment.
This will increase the chances of a name collision. Consider two profile names chrome^ and chrome*. How are name collisions detected and handled?
I suggest looking to see if the name is already used and append an int ID.
There was a problem hiding this comment.
Given you need the name created to reproducible every time so you can find it again in the removeScriptForProfile function, you could just append a base16 encoding of the string (from the value prior to substitutions). If you want to reduce the length of this string, increase the size of the base encoding (up to the number of valid characters).
|
Would it be ok for you to just replace every invalid character with its baase16 counterpart? This replaces a space with 20 in script names. However, note that these script names are usually not exposed to the public, only saved in the gestures file, which is never advised to be touched by end users. |
This is fine, as long as there is no need to be able to go in the opposite direction. Eg from script name back to the original. I don't see why there would be such a need. |
|
The original profile name is always saved on the script, so that should be fine, indeed. |
feerrenrut
left a comment
There was a problem hiding this comment.
Can you let me know if you are happy with this (it's all tested)? Then I will merge it.
|
I've had another look at the code and did another round of testing, and all seems to work well, so I'm happy with merging this. In my last commit, I only touched doc strings. |
| if not c.isalnum() and c != "_": | ||
| invalidChars.add(c) | ||
| for c in invalidChars: | ||
| name=name.replace(c, b16encode(c)) |
There was a problem hiding this comment.
I don't think that b16encode works for non-ascii values (I.e. any other unicode value).
Someone sent NV Access an email showing the following traceback from a very recent alpha snapshot:
ERROR - keyboardHandler.internal_keyDownEvent (14:59:07.003):
internal_keyDownEvent
Traceback (most recent call last):
File "keyboardHandler.pyc", line 178, in internal_keyDownEvent
File "inputCore.pyc", line 421, in executeGesture
File "baseObject.pyc", line 47, in __get__
File "baseObject.pyc", line 147, in _getPropertyViaCache
File "inputCore.pyc", line 145, in _get_script
File "scriptHandler.pyc", line 68, in findScript
File "globalCommands.pyc", line 2525, in <module>
File "globalCommands.pyc", line 2427, in __new__
File "globalCommands.pyc", line 2466, in addScriptForProfile
File "globalCommands.pyc", line 2438, in _getScriptNameForProfile
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf1' in position 12: ordinal not in range(128)
Link to issue number:
Closes #4209
Summary of the issue:
Configuration profile can either be activated using triggers or by manual activation in the configuration profiles dialogs. Activation with gestures is not yet supported.
Description of how this pull request fixes the issue:
This pr adds a new ConfigProfileActivationCommands class to GlobalCommands, which has a single instance, globalCommands.configProfileActivationCommands. This class is used as follows.
Testing performed:
Known issues with pull request:
Config profiles are bound to file naming restrictions, whereas script names are bound to python variable naming rules. The latter rules are stricter. This is now taken care of by changing any thing that is not either an alphanumeric or an underscore to its base16 encoding. Doing so meets the naming requirements and avoids a name collision when having a "Test profile" and "Test_profile" profile.
Change log entry:
Section: New features