Add toggleReportCellBorders script to input gestures. by jakubl7545 · Pull Request #10408 · nvaccess/nvda

jakubl7545 · 2019-10-21T06:04:00Z

Link to issue number:

Summary of the issue:

Currently script for reporting cell borders is not present in input gestures dialog so it is not possible to add a gesture for that.

Description of how this pull request fixes the issue:

It adds toggleReportCellBorders script to globalCommands.

Testing performed:

I opened document formatting group in input gestures and found added script there. Then assigned gesture and checked that it works.

Known issues with pull request:

None.

Change log entry:

It is now possible to assign gesture to script reporting cell borders.
Section: Bug fixes

jakubl7545 · 2019-10-21T06:13:35Z

I closed previous PR and open a new one but it fails with the same message:
Error sending GitHubPullRequest notification: Error commenting on GitHub pull request: {"message":"Validation Failed","errors":[{"resource":"IssueComment","code":"unprocessable","field":"data","message":"Body cannot be blank"}],"documentation_url":"https://developer.github.com/v3/issues/comments/#create-a-comment"}
Any help with that will be appreciated.

lukaszgo1 · 2019-10-21T11:05:52Z

+	def script_toggleReportCellBorders(self, gesture):
+		if not config.conf["documentFormatting"]["reportBorderStyle"] and not config.conf["documentFormatting"]["reportBorderColor"]:
+			# Translators: A message reported when cycling through cell borders settings.
+			config.conf["documentFormatting"]["reportBorderStyle"] = True


At the moment check for translatable commends fails because the commend is not placed directly above the message.

AppVeyorBot · 2019-10-21T13:30:45Z

PR introduces Flake8 errors 😲

See test results for Failed build of commit c517cd8955

LeonarddeR

Thanks for taking this

LeonarddeR · 2019-10-21T13:39:52Z

@@ -1,9 +1,8 @@
 # -*- coding: UTF-8 -*-


Feel free to remove this line

It must be present because there is a letter Ł in one of contributors name.

Now the encoding declaration can be removed, since the encoding is specified when invoking Gettext.

LeonarddeR · 2019-10-21T13:40:16Z

@@ -1,9 +1,8 @@
 # -*- coding: UTF-8 -*-
-#globalCommands.py
 #A part of NonVisual Desktop Access (NVDA)


Could you please add a space after every hash sign while at it?

Suggested change

#A part of NonVisual Desktop Access (NVDA)

# A part of NonVisual Desktop Access (NVDA)

LeonarddeR · 2019-10-21T13:41:07Z

 		ui.message(state)
 	# Translators: Input help mode message for toggle report font name command.
-	script_toggleReportFontName.__doc__=_("Toggles on and off the reporting of font changes")
+	script_toggleReportFontName.__doc__=_("Toggles on and off the reporting of font name changes")


As you are touching code for this script, could you please also add the script decorator to this one?

LeonarddeR · 2019-10-21T13:44:00Z


+	@script(
+		# Translators: Input help mode message for toggle report cell borders command.
+		description=_("Cycles through cell borders settings"),


Suggested change

description=_("Cycles through cell borders settings"),

description=_("Cycles through the cell border reporting settings"),

AppVeyorBot · 2019-10-21T13:49:29Z

PR introduces Flake8 errors 😲

See test results for Failed build of commit fa5bebbff0

jakubl7545 · 2019-10-21T17:44:58Z

@LeonarddeR All comments are addressed. And thanks a lot @lukaszgo1 for help.

lukaszgo1 · 2021-01-14T13:33:22Z

@jakubl7545 Do you intent to fix the merge conflicts here?

AppVeyorBot · 2021-01-14T17:11:09Z

Build (for testing PR)

See test results for failed build of commit ea28de4f3e

lukaszgo1 · 2021-07-06T10:52:12Z

Is there anything blocking this PR from being looked at? cc @seanbudd

seanbudd · 2021-07-07T01:10:55Z


+	@script(
+		# Translators: Input help mode message for toggle report cell borders command.
+		description=_("Cycles through  the cell border reporting settings"),


Suggested change

description=_("Cycles through the cell border reporting settings"),

description=_("Cycles through the cell border reporting settings"),

seanbudd

Thanks @jakubl7545, looks good to me otherwise

seanbudd · 2021-07-07T05:10:09Z

+			not config.conf["documentFormatting"]["reportBorderStyle"] and not
+			config.conf["documentFormatting"]["reportBorderColor"]


Just to improve readability

Suggested change

not config.conf["documentFormatting"]["reportBorderStyle"] and not

config.conf["documentFormatting"]["reportBorderColor"]

not config.conf["documentFormatting"]["reportBorderStyle"] and

not config.conf["documentFormatting"]["reportBorderColor"]

seanbudd · 2021-07-07T05:10:34Z

+			config.conf["documentFormatting"]["reportBorderStyle"] and not
+			config.conf["documentFormatting"]["reportBorderColor"]


Suggested change

config.conf["documentFormatting"]["reportBorderStyle"] and not

config.conf["documentFormatting"]["reportBorderColor"]

config.conf["documentFormatting"]["reportBorderStyle"] and

not config.conf["documentFormatting"]["reportBorderColor"]

AppVeyorBot · 2021-07-21T15:23:32Z

PASS: Translation comments check.
PASS: Unit tests.
FAIL: Lint check. See test results for more information.
PASS: System tests.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/25wcqgy5igmclau2/artifacts/output/nvda_snapshot_pr10408-23395,a30119b1.exe

See test results for failed build of commit a30119b14b

jakubl7545 · 2021-07-21T17:35:53Z

@seanbudd Changes applied.

jakubl7545 · 2021-07-21T20:56:46Z

That was discussed long time ago. Don't remember the details but I was asked to do that by the reviewer.

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

jakubl7545 added 5 commits October 20, 2019 20:25

Add reportCellBorders script to input gestures

feb3108

Use script decorator.

471b1c8

Modify copyright header.

b3cc7fe

Add missing comma.

9ec7f98

Add another missing comma.

55ce4ad

lukaszgo1 reviewed Oct 21, 2019

View reviewed changes

Place message directly below translatable comment.

77e55c3

Address linting errors

29e1f9f

LeonarddeR suggested changes Oct 21, 2019

View reviewed changes

jakubl7545 added 2 commits October 21, 2019 16:22

Address review and linting comments.

fa09ac5

Bring back encoding declaration.

4fe2600

Merge branch 'master' into i9507

3458bcd

feerrenrut added feature app/microsoft-office labels Apr 8, 2020

Merge branch 'master' into i9507

865ceda

CyrilleB79 mentioned this pull request Apr 11, 2020

Add a script to toggle the report of superscripts and subscripts #10985

Merged

Merge master and fix conflicts

525abc5

Linting fix

fea7808

seanbudd self-requested a review July 7, 2021 01:12

seanbudd reviewed Jul 7, 2021

View reviewed changes

seanbudd self-requested a review July 7, 2021 01:13

seanbudd requested changes Jul 7, 2021

View reviewed changes

Merge branch 'master' into i9507

6b85f10

Apply review comments

5580747

jakubl7545 requested a review from a team as a code owner July 21, 2021 15:06

Linting

bda42fe

CyrilleB79 reviewed Jul 21, 2021

View reviewed changes

Comment thread source/globalCommands.py Outdated

seanbudd and others added 4 commits July 22, 2021 13:33

remove change of command description

ddeab15

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

update changes

d09222e

Merge branch 'master' into i9507

7c3fb58

fix changes

d80f795

seanbudd approved these changes Jul 22, 2021

View reviewed changes

seanbudd merged commit f9fc324 into nvaccess:master Jul 22, 2021

nvaccessAuto added this to the 2021.3 milestone Jul 22, 2021

jakubl7545 deleted the i9507 branch July 22, 2021 09:34

	#A part of NonVisual Desktop Access (NVDA)
	# A part of NonVisual Desktop Access (NVDA)

	description=_("Cycles through cell borders settings"),
	description=_("Cycles through the cell border reporting settings"),

		not config.conf["documentFormatting"]["reportBorderStyle"] and not
		config.conf["documentFormatting"]["reportBorderColor"]

Uh oh!

Conversation

jakubl7545 commented Oct 21, 2019 • edited by seanbudd 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!

jakubl7545 commented Oct 21, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AppVeyorBot commented Oct 21, 2019

Uh oh!

LeonarddeR left a comment

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!

AppVeyorBot commented Oct 21, 2019

Uh oh!

jakubl7545 commented Oct 21, 2019

Uh oh!

lukaszgo1 commented Jan 14, 2021

Uh oh!

AppVeyorBot commented Jan 14, 2021

Uh oh!

lukaszgo1 commented Jul 6, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanbudd left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

AppVeyorBot commented Jul 21, 2021

Uh oh!

jakubl7545 commented Jul 21, 2021

Uh oh!

Uh oh!

jakubl7545 commented Jul 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jakubl7545 commented Oct 21, 2019 •

edited by seanbudd

Loading

seanbudd left a comment •

edited

Loading