Add "report formatting for text under braille cell" command by dkager · Pull Request #7189 · nvaccess/nvda

dkager · 2017-05-19T10:00:24Z

Link to issue number:

Fixes #7106. Fixes #7869.

Summary of the issue:

This command is useful to obtain formatting information for text under a particular braille cell without first having to route the cursor there. It is an intuitive command to be tied to the Alva BC6 secondary routing keys. Displays without such keys can use a gesture that consists of the routing key with another braille display key.

Description of how this pull request fixes the issue:

Followed suggestions from @jcsteh in #7106 (comment)

Also incorporated PR #7990.

Testing performed:

Assigned the command to the Alva BC6 secondary routing keys. Then ran it on both Windows controls and text in various editors (Notepad, Word, Notepad++).

Known issues with pull request:

This command only makes sense when assigned to (secondary) cursor routing keys, because it reports formatting for text under a specific braille cell. However, this is not enforced in any way.

Change log entry:

In New Features:

Added a script to report formatting information for the text under a specific braille cell. (Add "report formatting under cursor routing key" command #7106)

Changes:

The secondary routing buttons on Alva BC6, EuroBraille and Papenmeier braille displays now report formatting information for the text under the braille cell of that button. (Add "report formatting under cursor routing key" command #7106)

Bug Fixes:

Pressing the report formatting script no longer moves the reviewPosition and therefore pressing it multiple times no longer gives different results. (The font color retreived in word is different if pressed once than twice #7869)

…ormatting command.

derekriemer · 2017-05-19T13:53:41Z

For other displays, what about tapping adjacent buttons?

…

On Fri, May 19, 2017 at 4:00 AM, Davy Kager ***@***.***> wrote: Link to issue number: Add "report formatting under cursor routing key" command #7106 <#7106> Summary of the issue: This command is useful to obtain formatting information for text under a particular braille cell without first having to route the cursor there. It is an intuitive command to be tied to the Alva BC6 secondary routing keys. Displays without such keys can use a gesture that consists of the routing key with another braille display key. Description of how this pull request fixes the issue: Followed suggestions from @jcsteh <https://github.com/jcsteh> in #7106 (comment) <#7106 (comment)> Testing performed: Tied the command to the Alva BC6 secondary keys. Then ran it on both Windows controls and text in various editors (Notepad, Word, Notepad++). Known issues with pull request: The command has only been added to the Alva BC6 keymap. @LeonarddeR <https://github.com/leonardder> has offered to add and test other displays in a separate PR. Change log entry: In New Features: - Added a command to report formatting information for the text under a specific braille cell. (#7106) In Changes: - The secondary routing buttons on Alva BC6 braille displays now report formatting information for the text under that button. (#7106) ------------------------------ You can view, comment on, or merge this pull request online at: #7189 Commit Summary - Add a "report formatting info for text under this braille cell" command. - Assign the Alva BC6 secondary cursor routing keys to the new report formatting command. File Changes - *M* source/braille.py <https://github.com/nvaccess/nvda/pull/7189/files#diff-0> (33) - *M* source/brailleDisplayDrivers/alvaBC6.py <https://github.com/nvaccess/nvda/pull/7189/files#diff-1> (1) - *M* source/globalCommands.py <https://github.com/nvaccess/nvda/pull/7189/files#diff-2> (26) Patch Links: - https://github.com/nvaccess/nvda/pull/7189.patch - https://github.com/nvaccess/nvda/pull/7189.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#7189>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFGivUaW-Sh6efKUWP0QVocrU-Wuiwwrks5r7Wg5gaJpZM4NgUrH> .

-- Derek Riemer: Improving the world one byte at a time! - University of Colorado Boulder Department of computer science, 4th year undergraduate student. - Accessibility enthusiast. - Proud user of the NVDA screen reader. - Open source enthusiast. - Skier. Personal website <http://derekriemer.com>

dkager · 2017-05-19T13:55:59Z

@LeonarddeR suggested space+routing.

LeonarddeR · 2017-05-19T15:04:49Z

@LeonarddeR suggested space+routing.

Yup, but this won't work with displays without a braille keyboard, Baum Vario 340 for example, or whatever their newest device is called. Also, the 80 cell Braillant displays do not have braille input from what I recall.

dkager · 2017-05-19T15:08:25Z

Also, the 80 cell Braillant displays do not have braille input from what I recall.

... And on the BI 40 I turn it off to avoid bogus input. But I think for anything else, like C1+routing on the Brailliant, users have to decide and define it themselves.

LeonarddeR · 2017-06-22T13:59:52Z

I just walked through this code and all looks good to me.

dkager · 2017-07-19T09:00:29Z

Just expertly dealt with a conflict situation, if I say so myself. :)

LeonarddeR

Ugh, merge conflicts again.

Also, I think it would be good if you would change the papenmeier drivers in this pr as well. See script_upperRouting in both of them.

derekriemer · 2017-09-09T02:24:13Z

still merge conflicts

… drivers. This removes their "route to and report formatting" behavior. Instead, it now only reports formatting without routing the cursor.

dkager · 2017-09-09T13:04:44Z

All fixed.

LeonarddeR · 2018-02-05T05:02:09Z

@michaelDCurran: Could it be that you forgot to incubate these?

Nevertheless, there are merge conflicts due to the new ALVA driver.

michaelDCurran · 2018-02-05T06:43:19Z

Looks like I just forgot. Weird. Assuming the merge conflicts are fixed, I'd still wait a bit for Next to settle down with the updater changes, then I will incubate this.

LeonarddeR · 2018-02-06T04:59:18Z

 	script_sayAll.category=SCRCAT_SYSTEMCARET

-	def script_reportFormatting(self,gesture):
+	def _reportFormatting(self, info, browseable=False):


May be call this _reportFormattingHelper or _formatReportingHelper?

LeonarddeR · 2018-02-06T05:03:28Z

 | Move braille display to next line | t4 |
 | Scroll braille display forward | t5 or etouch3 |
 | Route to braille cell | routing |
+| Reports formatting under braille cell | secondRouting |


Given the verbs above are toggle and route, I'd say this should be report instead of report.

LeonarddeR · 2018-02-06T05:05:20Z

+			# Translators: Reported when trying to obtain formatting information (such as font name, indentation and so on) but there is no formatting information for the text under cursor.
+			ui.message(_("No formatting information"))
+			return
+		self._reportFormatting(info, False)


I wonder whether reporting formatting info in a browsable message would make sense for routing with a braille display, e.g. after two or three presses?

dkager · 2018-02-06T19:53:09Z

@LeonarddeR One more round of review please.

…Braille displays.

LeonarddeR

Woops, my bad, I overlooked an important thing. When you report formatting under a braille cell, the formatting information is also shown in braille, for the first press, and than the second press applies to the formatting message and not to the text you're trying to review. I guess it's better to revert the double press functionality.

dkager · 2018-02-10T13:50:02Z

Interesting.
I also realized that it doesn't make sense to assign this command to a non-routing key, but that we can't enforce that right now.

…e cell command.

LeonarddeR · 2018-02-12T14:17:33Z

This is likely to conflict with #7990

michaelDCurran · 2018-02-12T22:47:22Z

Above the info.expand in _reportFormattingHelper, please at the following line:

info=info.copy()

This ensures that we don't accidentally mutate the passed in TextInfo, which may have been the review cursor or something.
This will then superseed PR #7990 and therefore fix #7869

dkager · 2018-02-13T06:21:58Z

@michaelDCurran I believe the last commit is what you meant, but please double-check.

dkager · 2018-02-13T18:02:10Z

Updated #7189 (comment) with info from the other PR and better changelog entries.

michaelDCurran · 2018-03-08T08:25:07Z

When this was merged to next, it failed a test.

Traceback (most recent call last):
  File "C:\Users\mick\programming\git\nvda\tests\unit\test_braille.py", line 32, in setUp
    braille.handler.handleGainFocus(self.obj)
  File "C:\Users\mick\programming\git\nvda\source\braille.py", line 1746, in handleGainFocus
    self._doNewObject(itertools.chain(getFocusContextRegions(obj, oldFocusRegions=self.mainBuffer.regions), getFocusRegi
ons(obj)))
  File "C:\Users\mick\programming\git\nvda\source\braille.py", line 1751, in _doNewObject
    for region in regions:
  File "C:\Users\mick\programming\git\nvda\source\braille.py", line 1449, in getFocusContextRegions
    region.update()
  File "C:\Users\mick\programming\git\nvda\source\braille.py", line 600, in update
    super(NVDAObjectRegion, self).update()
  File "C:\Users\mick\programming\git\nvda\source\braille.py", line 382, in update
    mode |= louis.pass1Only
AttributeError: 'module' object has no attribute 'pass1Only'

LeonarddeR · 2018-03-08T09:26:19Z

hmm, it looks like the merge went wrong in that case. Liblouis pass1only has been removed as part of #7839.

michaelDCurran · 2018-03-08T09:40:03Z

Right. But they are both now in Next. I'm assuming that #7189 is making reference to passOneOnly, but the Liblouis PR has removed it. The easiest thing to do is to probably change #7189 to not reference passOneOnly. It does mean though that we aare assuming that the libLouis pr will definitely make it to master before #7189, which I think is fine.

michaelDCurran · 2018-03-08T16:58:38Z

Sorry, when I tested locally I had not updated my liblouis submodule it seems. The error in the build was
simply a missing translator comment. I shall fix myself.

dkager · 2018-03-09T09:03:20Z

Just to confirm: this PR does not depend on any liblouis features.

michaelDCurran · 2018-03-09T09:58:20Z

Yep, my mistake. Sorry about that. However, there was a missing translation comment, but I added it.

dkager added 2 commits May 18, 2017 13:19

Add a "report formatting info for text under this braille cell" command.

f9a7571

Assign the Alva BC6 secondary cursor routing keys to the new report f…

ecebe94

…ormatting command.

dkager added 3 commits June 5, 2017 16:55

Update copyrights.

bc39da2

Merge branch 'master' into i7106

1910074

Update user guide for the new Alva BC6 key assignment.

75bf996

Merge branch 'master' into i7106

983d0e3

LeonarddeR suggested changes Sep 8, 2017

View reviewed changes

dkager added 2 commits September 9, 2017 14:53

Merge branch 'master' into i7106

d46e560

Also use the new report formatting braille command in both Papenmeier…

9e00c8b

… drivers. This removes their "route to and report formatting" behavior. Instead, it now only reports formatting without routing the cursor.

LeonarddeR approved these changes Sep 25, 2017

View reviewed changes

michaelDCurran added 2 commits December 12, 2017 15:11

Merge branch 'master' into i7106

11f17c8

Merge branch 'master' into i7106

31e41aa

michaelDCurran approved these changes Dec 12, 2017

View reviewed changes

dkager added 2 commits February 5, 2018 18:51

Merge branch 'master' into i7106

d909615

Merge branch 'i7106' of github.com:dkager/nvda into i7106

1be4824

dkager requested a review from LeonarddeR February 5, 2018 18:00

LeonarddeR reviewed Feb 6, 2018

View reviewed changes

Address review actions.

ae26aaf

dkager added 3 commits February 9, 2018 21:45

Merge branch 'master' into i7106

8096686

Update user docs.

09026c5

Add the report text formatting under braille cell command to the Euro…

10e1f75

…Braille displays.

LeonarddeR suggested changes Feb 10, 2018

View reviewed changes

dkager added 2 commits February 10, 2018 22:04

Revert browseable message for the report text formatting under braill…

310473f

…e cell command.

Merge branch 'master' into i7106

50644b7

LeonarddeR approved these changes Feb 12, 2018

View reviewed changes

LeonarddeR mentioned this pull request Feb 12, 2018

Report the same formatting when pressing the reportFormatting script multiple times #7990

Closed

Incorporate PR nvaccess#7990.

ca41d27

michaelDCurran approved these changes Mar 8, 2018

View reviewed changes

michaelDCurran added a commit that referenced this pull request Mar 8, 2018

Incubates #7189

cd1e710

nvaccessAuto added the incubating label Mar 8, 2018

nvaccessAuto assigned michaelDCurran Mar 8, 2018

Add translator comment

c948e73

michaelDCurran added a commit that referenced this pull request Mar 8, 2018

Incubates #7189

891a9c9

michaelDCurran merged commit 9dd4836 into nvaccess:master Mar 28, 2018

nvaccessAuto removed the incubating label Mar 28, 2018

nvaccessAuto added this to the 2018.2 milestone Mar 28, 2018

Uh oh!

Conversation

dkager commented May 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing performed:

Known issues with pull request:

Change log entry:

Uh oh!

derekriemer commented May 19, 2017 via email

Uh oh!

dkager commented May 19, 2017

Uh oh!

LeonarddeR commented May 19, 2017

Uh oh!

dkager commented May 19, 2017

Uh oh!

LeonarddeR commented Jun 22, 2017

Uh oh!

dkager commented Jul 19, 2017

Uh oh!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

derekriemer commented Sep 9, 2017

Uh oh!

dkager commented Sep 9, 2017

Uh oh!

LeonarddeR commented Feb 5, 2018

Uh oh!

michaelDCurran commented Feb 5, 2018 via email

Uh oh!

LeonarddeR Feb 6, 2018

Choose a reason for hiding this comment

Uh oh!

LeonarddeR Feb 6, 2018

Choose a reason for hiding this comment

Uh oh!

LeonarddeR Feb 6, 2018

Choose a reason for hiding this comment

Uh oh!

dkager commented Feb 6, 2018

Uh oh!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

dkager commented Feb 10, 2018

Uh oh!

LeonarddeR commented Feb 12, 2018

Uh oh!

michaelDCurran commented Feb 12, 2018

Uh oh!

dkager commented Feb 13, 2018

Uh oh!

dkager commented Feb 13, 2018

Uh oh!

michaelDCurran commented Mar 8, 2018

Uh oh!

LeonarddeR commented Mar 8, 2018

Uh oh!

michaelDCurran commented Mar 8, 2018 via email

Uh oh!

michaelDCurran commented Mar 8, 2018

Uh oh!

dkager commented Mar 9, 2018

Uh oh!

michaelDCurran commented Mar 9, 2018 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dkager commented May 19, 2017 •

edited

Loading