Support for uncontracted and contracted braille input. by jcsteh · Pull Request #6449 · nvaccess/nvda

jcsteh · 2016-10-11T06:34:31Z

Beyond the main code in brailleInput to support uncontracted/contracted input, this includes the following changes:

The list of braille tables has been moved out of the braille module into a separate brailleTables module. Braille tables are now added with a function rather than directly adding them to the data structure. Aside from being necessary in order to specify and check whether a table is contracted, this also makes the data about tables more extensible in future.
As the data structure for braille tables has now changed and is no longer ordered, this was a good opportunity to sort the list of tables alphabetically when displaying them to the user.
brailleInput is now notified when reverting config or changing config profiles. This is necessary because brailleInput now maintains some state when the input table is changed.
brailleInput is now initialised before braille at startup. This is because braille depends on brailleInput to get the currently untranslated input.
Dot7 and dot8 are now universally bound to braille input specific scripts for erase and enter. Any braille display drivers that had bindings for backspace/enter for braille input have been adjusted accordingly.
In the User Guide, a "Braille" section has been added above "Application Specific Features". The "Braille Control Types and States" section has been moved to a sub-section of this, and a sub-section on Braille Input has been added.

Fixes #2439. Fixes #6054. Fixes #6113. Fixes #6935.

What's New entries:

In New Features:

- You can now type in both contracted and uncontracted braille on a braille display with a braille keyboard. See the Braille Input section of the User Guide for details. (#2439, #6054)

In Changes:

- The output and input table lists in the Braille Settings dialog are now sorted alphabetically. (#6113)
- Updated liblouis braille translator to 3.2.0. (#6935)

Beyond the main code in brailleInput to support uncontracted/contracted input, this includes the following changes: * The list of braille tables has been moved out of the braille module into a separate brailleTables module. Braille tables are now added with a function rather than directly adding them to the data structure. Aside from being necessary in order to specify and check whether a table is contracted, this also makes the data about tables more extensible in future. * As the data structure for braille tables has now changed and is no longer ordered, this was a good opportunity to sort the list of tables alphabetically when displaying them to the user. * brailleInput is now notified when reverting config or changing config profiles. This is necessary because brailleInput now maintains some state when the input table is changed. * brailleInput is now initialised before braille at startup. This is because braille depends on brailleInput to get the currently untranslated input. * Dot7 and dot8 are now universally bound to braille input specific scripts for erase and enter. Any braille display drivers that had bindings for backspace/enter for braille input have been adjusted accordingly.

jcsteh · 2016-10-11T06:37:10Z

@feerrenrut, would you mind reviewing this please? Note that I've tried to explain some of the more obscure changes in the description of this pull request.

feerrenrut · 2016-10-11T09:47:53Z

 		chunk.setEndPoint(sel, "endToStart")
 		self._addTextWithFields(chunk, formatConfig)
+		# If the user is entering braille, place any untranslated braille before the selection.
+		import brailleInput


Why is this import local and not at the top of the file? Perhaps add a comment

feerrenrut · 2016-10-11T09:48:24Z

 	def routeTo(self, braillePos):
+		if self._brailleInputStart is not None and self._brailleInputStart <= braillePos <= self._brailleInputEnd:
+			# The user is moving within untranslated braille input.
+			import brailleInput


Same question here, not at the top of the file?

feerrenrut · 2016-10-11T09:51:34Z

+		if text:
+			rawInputStart = len(self.rawText)
+			self._addFieldText(text, None, separate=False)
+			rawInputEnd = len(self.rawText)


Is this correct? rawImputStart and rawInputEnd are both being initialised to the same thing:len(self.rawText). If so I think the intent is clearer if it is written rawInputEnd=rawInputStart

feerrenrut · 2016-10-11T09:54:41Z

 		self.focusToHardLeft = self._isMultiline()
 		super(TextInfoRegion, self).update()

+		if rawInputStart is not None:


It might be a good idea to also check that rawInputEnd is not None

feerrenrut · 2016-10-11T09:56:59Z

 			"kb:control+home": ("br(braillenote):space+d1+d2+d3",),
 			"kb:control+end": ("br(braillenote):space+d4+d5+d6",),
-			"kb:enter": ("br(braillenote):space+d8",),
+			"braille_enter": ("br(braillenote):space+d8",),


I'm assuming this change is a new line conversion?

Weird. This doesn't even show up in diffs from command line git. I reckon this is a GitHub bogus thingy. :(

feerrenrut · 2016-10-11T11:28:30Z

+
+	def script_braille_enter(self, gesture):
+		brailleInput.handler.enter()
+	script_braille_enter.__doc__= _("Translates any braille input and presses the enter key")


missing translators comment

feerrenrut · 2016-10-11T11:32:29Z

 		label = wx.StaticText(self, wx.ID_ANY, label=_("&Input table:"))
-		self.inputTableNames = [table[0] for table in braille.INPUT_TABLES]
-		self.inputTableList = wx.Choice(self, wx.ID_ANY, choices=[table[1] for table in braille.INPUT_TABLES])
+		self.inputTableList = wx.Choice(self, wx.ID_ANY, choices=tableChoices)


wx.ID_ANY is unnecessary, I think this might conflict with next

feerrenrut · 2016-10-11T11:38:32Z

+	@type number: int
+	"""
+	global _suppressSpeakTypedCharactersData
+	_suppressSpeakTypedCharactersData = (number, time.time())


Can this be called twice before number has had a chance to decrement to 0?

feerrenrut · 2016-10-11T11:50:12Z

+		timeOk = time.time() - supTime <= 0.1
+		suppress = number > 0 and timeOk
+		number -= 1
+		if number > 0 and timeOk:


I think this logic could be cleaned up so the condition does not need to be repeated. It would have the implication that _suppressSpeakTypedCharactersData exists for longer, and number is allowed to reach zero.

eg:

if _suppressSpeakTypedCharactersData: ... timeOk = time.time() - supTime <= 0.1 suppress = number > 0 and timeOk if suppress: _suppressSpeakTypedCharactersData = (--number, supTime) else: _suppressSpeakTypedCharactersData = None else: suppress = False if not suppress and config.conf["keyboard"]["speakTypedCharacters"] and ord(ch)>=32: speakSpelling(realChar)

feerrenrut · 2016-10-11T11:51:59Z

+			_suppressSpeakTypedCharactersData = None
+	else:
+		suppress = False
+	if not suppress and config.conf["keyboard"]["speakTypedCharacters"] and ord(ch)>=32:


I'm guessing 32 is a limit for non-printable characters? A named constant would make this more readable.

Quite true. However, as that's very much unrelated code that existed previously (I just added an extra condition), I'd prefer to address this somewhere else.

…e, typing two cells then pressing backspace. This occurred because cellsWithText was being updated even for the space at the end of a word. At this point, we're starting a new word, so cellsWithText needs to be empty.

…rd you are typing.

…or pressing enter.

jcsteh · 2016-10-12T06:46:11Z

@feerrenrut, I'm done with this batch of changes. I addressed your code review comments plus some other stuff noted below. Would you mind taking a look again?

@derekriemer reported a bug to me via IRC. STR:

Enable input with UEB grade 2.
Type "b" and press space.
Press dot 6 twice.
Press backspace.

This causes an exception. I've fixed this in 0b643d6.

I've also addressed a couple of other issues @derekriemer reported in recent comments on #2439.

jcsteh · 2016-10-12T23:58:56Z

@feerrenrut, I'm done with this batch of changes.

Actually, I just pushed one more commit to mask characters in protected fields (e.g. password fields). Thanks @derekriemer for catching!

… several pending upstream changes related to braille input.

This necessitated the ability to have tables specific to either output or input, rather than assuming all tables can handle both.

* Always set noUndefinedDots so undefined dots (such as indicators that mean nothing by themselves) don't get translated to, for example, "\456/" for dots 4 5 6. * Set partialTrans when reporting contracted cells, thus avoiding the "zz" hack which was breaking French where z is actually a contraction.

jcsteh · 2016-11-03T06:46:52Z

Holding this back from the 2016.4 release, as there are some major outstanding issues:

Investigate/fix effect of undefined dots on subsequent translation. For example, with UEB g2, ⠰⠁⠃ gets translated to "about" instead of "ab" with noUndefinedDots. Without that mode, you get "\56/ab".
When there is untranslated input, routing braille outside of the untranslated input should clear the untranslated input.
Further UEB back-translation fixes from APH.

…ess branch for now due to some changes that aren't yet in 3.1.0.

…so, remove "grade 1" from the description, since there's no other grade.

jcsteh · 2017-06-19T05:10:31Z

@feerrenrut, this is finally ready for another round of review. There have been a lot of changes and quite a long time since you last reviewed it, so you may want to just review from scratch. In case it's helpful, though, the changes since your last review are 5ff07e5b..ff6f627a.

feerrenrut · 2017-06-19T07:35:45Z

+#: * fileName: The file name of the table.
+#: * displayname: The name of the table as displayed to the user. This should be translatable.
+#: * contracted: C{True} if the table is contracted, C{False} if uncontracted.
+BrailleTable = collections.namedtuple("BrailleTable", ("fileName", "displayName", "contracted", "output", "input"))


This docstring should also have output and input

feerrenrut · 2017-06-19T07:47:21Z

+			_suppressSpeakTypedCharactersTime = None
+	else:
+		suppress = False
+	if not suppress and config.conf["keyboard"]["speakTypedCharacters"] and ord(ch)>=32:


Could you replace the value 32 with a name please? I'm not really sure what this is testing for

derekriemer · 2017-06-19T18:40:39Z

+#A part of NonVisual Desktop Access (NVDA)
+#This file is covered by the GNU General Public License.
+#See the file COPYING for more details.
+#Copyright (C) 2008-2016 NV Access Limited, Joseph Lee


update copyright year

…have been merged.

…2439). Fixes #6956.

…rs; we got rid of that in #6962. Make it possible to get the display text from a braille keyboard gesture identifier (as used in the Input Gestures dialog).

jcsteh · 2017-06-21T05:12:58Z

@feerrenrut, can you please review the latest commit? Sorry. :(

feerrenrut · 2017-06-21T10:18:15Z

+		return self._makeDisplayText(self.dots, self.space)
+
+	@classmethod
+	def getDisplayTextForIdentifier(cls, identifier):


Where is this called?

Perhaps a docstring on this to explain what its for? Also, it would be nice to see a couple of examples of what identifier might be.

…res dialog. Incubates #6449 (issue #2439).

derekriemer · 2017-06-28T03:25:22Z

fixes #6956

derekriemer · 2017-07-08T10:13:59Z

I'm amazed at how many tickets this autoclosed. What a huge chunk of work.

jcsteh assigned feerrenrut Oct 11, 2016

feerrenrut suggested changes Oct 11, 2016

View reviewed changes

jcsteh added 4 commits October 12, 2016 09:56

Address code review comments.

eb7babf

User Guide: Add note about translation only reflecting the braille wo…

11912ca

…rd you are typing.

Pressing dot7+dot8 translates immediately but without adding a space …

f4859a5

…or pressing enter.

Mask braille input in protected fields.

5ff07e5

feerrenrut approved these changes Oct 13, 2016

View reviewed changes

Merge branch 'master' into i2439ContractedBrailleInput

2f3f4fc

jcsteh added a commit that referenced this pull request Oct 13, 2016

Incubates #6449.

b38d31e

nvaccessAuto added the incubating label Oct 13, 2016

nvaccessAuto assigned jcsteh and unassigned feerrenrut Oct 13, 2016

jcsteh added 3 commits October 17, 2016 16:16

Update to a temporary NV Access branch of liblouis which incorporates…

e78f955

… several pending upstream changes related to braille input.

Add the Unicode braille input table.

414504d

This necessitated the ability to have tables specific to either output or input, rather than assuming all tables can handle both.

jcsteh mentioned this pull request Nov 3, 2016

Support for entry of contracted braille via a braille keyboard #2439

Closed

jcsteh mentioned this pull request Mar 13, 2017

Contracted braille input: do something to alert the user they are editing a word, and mus use contractions properly #6956

Closed

jcsteh added z app/edge (archived) use app/edge/anaheim and removed z app/edge (archived) use app/edge/anaheim labels Mar 19, 2017

liblouis sconscript: Add additional source file added in liblouis 3.1.0.

a224cab

jcsteh mentioned this pull request Mar 20, 2017

Upgrade to Liblouis 3.2.0 #6935

Closed

jcsteh added 3 commits March 20, 2017 15:40

Merge branch 'master' into i2439ContractedBrailleInput

84bbd53

Merge branch 'i6935Liblouis3.1' into i2439ContractedBrailleInput

f240afc

Update to liblouis 3.1.0, though we're still using a temporary NV Acc…

61d7c16

…ess branch for now due to some changes that aren't yet in 3.1.0.

jcsteh requested a review from feerrenrut June 19, 2017 04:45

brailleTables: Update for rename of Greek table in liblouis 3.2.0. Al…

ff6f627

…so, remove "grade 1" from the description, since there's no other grade.

feerrenrut suggested changes Jun 19, 2017

View reviewed changes

derekriemer reviewed Jun 19, 2017

View reviewed changes

jcsteh added 2 commits June 20, 2017 13:59

Switch liblouis to official liblouis master, since all fixes from us …

e2f8609

…have been merged.

Review actions.

c0004f0

jcsteh requested a review from feerrenrut June 20, 2017 04:21

feerrenrut approved these changes Jun 20, 2017

View reviewed changes

jcsteh added a commit that referenced this pull request Jun 20, 2017

Significant fixes to contracted braille input. Incubates #6449 (issue #…

9b0534f

…2439). Fixes #6956.

BrailleInputGesture: Don't use Python set order for gesture identifie…

84e6565

…rs; we got rid of that in #6962. Make it possible to get the display text from a braille keyboard gesture identifier (as used in the Input Gestures dialog).

jcsteh requested a review from feerrenrut June 21, 2017 05:12

feerrenrut suggested changes Jun 21, 2017

View reviewed changes

inputCore: Update code doc.

c2c7162

feerrenrut approved these changes Jun 22, 2017

View reviewed changes

jcsteh added a commit that referenced this pull request Jun 22, 2017

Show user friendly names for braille keyboard keys in the Input Gestu…

e456f16

…res dialog. Incubates #6449 (issue #2439).

jcsteh merged commit 1c0d490 into master Jul 7, 2017

nvaccessAuto removed the incubating label Jul 7, 2017

nvaccessAuto added this to the 2017.3 milestone Jul 7, 2017

jcsteh deleted the i2439ContractedBrailleInput branch July 7, 2017 05:05

This was referenced Jul 7, 2017

Make it easier for add-ons to supply custom braille tables #5489

Closed

Farsi is not in the input table list in braille settings? #6231

Closed

Danish Braille table for entering of Braille is not in NVDA #6773

Closed

AAClause mentioned this pull request Jul 9, 2017

Braille: giving choice concerning Dot7 and Dot8 #7369

Closed

This was referenced Jul 14, 2017

nvda input tables list doesn't contain an arabic tables #3549

Closed

Automatic braille table suggestion based on the user's language #290

Closed

Adriani90 mentioned this pull request Dec 6, 2018

Custom braille tables #3304

Closed

Uh oh!

Conversation

jcsteh commented Oct 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcsteh commented Oct 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcsteh commented Oct 12, 2016

Uh oh!

jcsteh commented Oct 12, 2016

Uh oh!

jcsteh commented Nov 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcsteh commented Jun 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcsteh commented Jun 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

derekriemer commented Jun 28, 2017

Uh oh!

derekriemer commented Jul 8, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jcsteh commented Oct 11, 2016 •

edited

Loading

jcsteh commented Nov 3, 2016 •

edited

Loading