Skip to content

Show proper display description when listing manually added braille display gestures in the input gestures dialog#8245

Merged
michaelDCurran merged 4 commits into
nvaccess:masterfrom
BabbageCom:mapGestureToDriver
Jul 31, 2018
Merged

Show proper display description when listing manually added braille display gestures in the input gestures dialog#8245
michaelDCurran merged 4 commits into
nvaccess:masterfrom
BabbageCom:mapGestureToDriver

Conversation

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Link to issue number:

Closes #8108

Summary of the issue:

When adding a braille display gesture to a script using the input gestures dialog, the added gesture shows up along with the description of the current display. However, when changing the current braille display, the gesture added earlier always shows up with the currently active display, not with the display driver description the gesture identifier belongs to.

Description of how this pull request fixes the issue:

  1. Updated braille._getDisplayDriver to accept an additional keyword argument, caseSensitive. If False, this function allows case insensitivity, in which case the source part of an identifier (e.g. "handytech" for the id "br(handytech):b1") can be used to find the handyTech BrailleDisplayDriver class.
  2. updated braille.BrailleDisplayGesture.getDisplayTextForIdentifier to find the description of the display the identifier belongs to. This is pretty easy now we can parse braille display gesture identifiers using the braille.BrailleDisplayGesture.ID_PARTS_REGEX.

Testing performed:

  • Added a gesture for the handy tech display. When selected no braille, the gesture still showed up as a gesture belonging to the handy tech driver.
  • In gestures.ini, changed handytech into handytechf. The gesture now showed up as referring to "Unknown braille display driver"

Known issues with pull request:

None

Change log entry:

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

@dkager: Would be good to know whether you have anything to say about this, since it was one of your annoyances until now ;)

@derekriemer

Copy link
Copy Markdown
Collaborator

does this need to incubate?

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

Hmm yes, I think it should.

LeonarddeR pushed a commit that referenced this pull request Jun 4, 2018
Merge branch 'mapGestureToDriver' into next
@josephsl

josephsl commented Jun 4, 2018

Copy link
Copy Markdown
Contributor

Hi,

Appveyor says the build process has failed, with the build log showing that one of the unit tests didn't pass.

Thanks.

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

Thanks for pointing out. I will undo the incubation and will request for another review after the unit test conflict has been resolved.

@LeonarddeR LeonarddeR removed their assignment Jun 4, 2018
LeonarddeR pushed a commit that referenced this pull request Jun 4, 2018
dkager
dkager previously approved these changes Jun 15, 2018
@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

@michaelDCurran: Could you do a review of the last changes before we incubate this again?

michaelDCurran
michaelDCurran previously approved these changes Jun 18, 2018
LeonarddeR pushed a commit that referenced this pull request Jun 18, 2018
…isplay gestures in the input gestures dialog. Incubates #8245
@michaelDCurran

Copy link
Copy Markdown
Member

This has been removed from next again. Unittests failed. If they are succeeding on this branch but not on next, it may be possible that new merges of this branch into next are breaking as there was a previous revert. Git does not cope well with this. A casing point as to why we are considering the new release process with PRs squashing merging straight to master.

@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

@michaelDCurran: What would be the best way to incubate this?

@michaelDCurran

Copy link
Copy Markdown
Member

You could rebase this branch on master and then we could incubate to next again. However, be aware that this will cause any review comments to become invalid.

@LeonarddeR LeonarddeR dismissed stale reviews from michaelDCurran and dkager via 0708e53 June 29, 2018 04:22
@LeonarddeR

Copy link
Copy Markdown
Collaborator Author

Thanks, I just rebased this unto master, thereby squashing everything into one commit.

michaelDCurran
michaelDCurran previously approved these changes Jul 31, 2018
@michaelDCurran michaelDCurran merged commit 870669e into nvaccess:master Jul 31, 2018
@nvaccessAuto nvaccessAuto added this to the 2018.3 milestone Jul 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Input Gestures window shows the name of the last braille display, not the originally defined one

6 participants