refactor(braille): split braille.py into a package by LeonarddeR · Pull Request #20252 · nvaccess/nvda

LeonarddeR · 2026-05-30T20:37:26Z

Link to issue number:

Closes #12772

Summary of the issue:

source/braille.py had grown to ~4256 lines, making it difficult to navigate, review, and maintain.

Description of user facing changes:

None. This is a pure refactor.

Description of developer facing changes:

The braille module is now a package (source/braille/), split into focused submodules:

braille.constants — value constants: cursor/selection shapes, untranslated-input indicators, port constants, the text separator, and focus-context presentation constants (leaf module, no intra-package imports)
braille.labels — the role / state / landmark label dictionaries
braille.formatting — the font-formatting markers (FormatTagDelimiter, FormattingMarker, fontAttributeFormattingMarkers), getParagraphStartMarker, and the private tag-rendering helpers (_getFormattingTags, _appendFormattingMarker) (leaf module)
braille.regions — a sub-package (source/braille/regions/) containing the full Region hierarchy and focus-region generators, split into focused modules:
- regions.base — the base Region class
- regions.NVDAObject — the NVDAObjectRegion
- regions.properties — braille field/property helpers
- regions.textInfo — TextInfoRegion
- regions.focus — getFocusContextRegions, getFocusRegions, invalidateCachedFocusAncestors, and the _cachedFocusAncestorsEnd cache
- regions._routing — internal routing helpers
- Internal consumers (brailleHandler, buffers) import directly from the relevant submodule rather than through the regions/__init__.py facade.
braille.buffers — BrailleBuffer (and _WindowRowPositions)
braille.display — a sub-package (source/braille/display/) split into:
- display.driver — BrailleDisplayDriver, DisplayDimensions, and driver discovery
- display.gesture — BrailleDisplayGesture
braille.brailleHandler — BrailleHandler, formatCellsForLog, and FALLBACK_TABLE
braille.extensions — all extension points (pre_writeCells, filter_displaySize, filter_displayDimensions, displaySizeChanged, displayChanged, decide_enabled, and the private Remote Access points)

braille/__init__.py defines only the handler global plus initialize / pumpAll / terminate; everything else is re-exported from the submodules. Because every symbol now lives in a real submodule (no submodule imports names back from the package facade), all of __init__.py's imports sit at the top of the file with no # noqa: E402.

The public API is unchanged: every symbol previously accessed as braille.X remains available through the braille/__init__.py facade. The __all__ list is now explicit. Private symbols stay private to their submodules: external callers import them from where they live (braille.display._getDisplayDriver, braille.extensions._pre_showBrailleMessage / _post_dismissBrailleMessage / _decide_disabledIncludesMessages, braille.buffers._WindowRowPositions) rather than through the package facade.

All copyright headers in the package have been updated to the current 4-line GPL format.

Description of development approach:

Extracted submodules one at a time, each guarded by the existing tests/unit/test_braille/ suite.
Added a public-surface regression test (test_publicSurface.py) as an anchor before any extraction.
Symbols were re-homed by responsibility: labels holds only label dictionaries; pure value constants live in a new leaf constants module; the font-formatting markers and helpers live in a new leaf formatting module; DisplayDimensions lives with the display code in display; FALLBACK_TABLE lives with its only consumer in brailleHandler; the focus-region helpers live with the Region classes they build in regions.
__init__.py is a pure facade: re-exports, __all__, the handler global, and initialize/pumpAll/terminate.
braille.regions and braille.display were further split into sub-packages. regions is divided by responsibility (base region, NVDAObject region, property helpers, TextInfo region, focus-region generators, routing helpers); display is divided into driver-side (driver) and gesture-side (gesture) concerns. Internal consumers import directly from the relevant submodule rather than through each sub-package's __init__.py.
Submodules that need the handler global reference it as braille.handler at runtime (never from braille import handler, which would capture a stale None).
Extension points extracted into braille.extensions so add-on authors have a clear, named home separate from implementation.
brailleHandler.py is named with the braille prefix to avoid shadowing the braille.handler module-level global.

Testing strategy:

test_publicSurface.py — characterizes the full braille.X public surface and __all__; guards against silent symbol loss across all tasks.
Existing tests/unit/test_braille/ suite (routing, buffers, regions, display drivers, extension points, language indexes).
No logic was changed — only import wiring and handler → braille.handler qualification where submodules reference the module-level global. Verified via diff.

Known issues with pull request:

Submodules that access the handler global (regions/focus.py, display/driver.py, brailleHandler.py) use import braille + braille.handler at call time to avoid capturing the None value at import time. This circular reference is intentional and safe, but could be eliminated in a follow-up by e.g. passing the handler explicitly or using a module-level accessor.

Code Review Checklist:

Documentation:
- Change log entry
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
Testing:
- Unit tests
- System (end to end) tests
- Manual testing
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
API is compatible with existing add-ons.
Security precautions taken.

Adds the brainstorming design for issue nvaccess#12772: split source/braille.py into a braille/ package following the controlTypes precedent, preserving the public braille.X API. Module relocations and shims are deferred to a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Task-by-task TDD plan for issue nvaccess#12772: anchor with a public-surface regression test, convert the module to a package, then extract labels/regions/buffer/display/_handler one submodule at a time. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…__all__ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…gions Move the Region class hierarchy, braille field/property helpers, and speak-on-navigation helpers from braille.__init__ into the new braille.regions submodule. Re-export public symbols from __init__ for backwards compatibility. Update tests to patch the canonical location. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…lle.buffer

…access#12772)

…opyright headers - Rename braille/_handler.py → braille/brailleHandler.py (avoids shadowing braille.handler global) - Rename braille/buffer.py → braille/buffers.py - Extract 9 extension points into new braille/extensions.py - Update all braille submodule copyright headers to new 4-line GPL format - Update test_handlerExtensionPoints.py header and year to 2022-2026 - Fix patch() targets in tests to match new submodule names Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

CyrilleB79 · 2026-05-31T14:40:18Z

@LeonarddeR you write:

Submodules that need the handler global reference it as braille.handler at runtime (never from braille import handler, which would capture a stale None).

Wouldn't getter/setter functions be more suitable here, with deprecation of braille.handler?
Or maybe in a subsequent PR though to keep this PR simple clear.

LeonarddeR · 2026-06-01T05:33:17Z

@CyrilleB79 wrote:

Wouldn't getter/setter functions be more suitable here, with deprecation of braille.handler? Or maybe in a subsequent PR though to keep this PR simple clear.

I thought about this yes and I think the current approach is a method to keep the pr as clear as possible.
Deprecation path would be:

Renaming handler to _handler
Create a getter function getHandler, no setter. The handler should be internal set only.
Add a deprecation warning for handler > getHandler
Keep in mind though that braille.handler is used many times throughout the code base and this is only a problem to the package internals, not for external code. So I'm still not sure.

LeonarddeR · 2026-06-01T16:38:08Z

I reconsidered hiding braille.handler behind a getter but braille.handler usage throughout the code base is really heavy. We can still consider a private getter to avoid the import braille, but I doubt whether it is worth it.

…age facade The package split left several symbols in submodules that did not match their responsibility. Re-home them: - New braille/constants.py (leaf, no package imports) holds the value constants previously dumped in labels.py: cursor/selection shapes, input indicators, port constants, the text separator and focus-context presentation constants. - labels.py now contains only the role/state/landmark label dictionaries. - The formatting-marker types (FormatTagDelimiter, FormattingMarker, fontAttributeFormattingMarkers), DisplayDimensions, the focus-region helpers (getFocusContextRegions/getFocusRegions plus the shared _cachedFocusAncestorsEnd cache and invalidateCachedFocusAncestors) and FALLBACK_TABLE move into the package __init__, in original braille.py order. - formatCellsForLog moves to brailleHandler, its only consumer. Public surface (braille.* and __all__) is unchanged; test_publicSurface passes without edits. The interleaved submodule imports in __init__ carry noqa: E402 as they must follow the definitions they depend on. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR converts the braille module into a package while preserving the existing braille.X public API surface, updating internal imports/patch targets, and adding regression tests to prevent accidental API drift.

Changes:

Split braille.py into a source/braille/ package with submodules (handler, buffers, regions, display, etc.).
Updated unit tests to patch the new module paths and added a regression test to guard the public symbol surface.
Documented the packaging change in user-facing changelog notes.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
user_docs/en/changes.md	Notes that `braille` is now a package and claims API compatibility.
source/braille/init.py	Re-exports the public API and defines `__all__` to preserve `braille.X` access.
source/braille/regions.py	Houses region classes and braille text generation previously in `braille.py`.
source/braille/buffers.py	Contains `BrailleBuffer` and window/word-wrapping logic moved out of the monolith.
source/braille/brailleHandler.py	Core handler moved into the package; wires buffers/display/extensions together.
source/braille/display.py	Display drivers and gesture logic moved into the package.
source/braille/extensions.py	Defines extension points for braille output/display changes.
source/braille/constants.py	Centralizes constants previously in `braille.py`.
source/braille/labels.py	Centralizes label dictionaries previously in `braille.py`.
tests/unit/test_braille/test_publicSurface.py	New regression test ensuring `braille.X` and `braille.__all__` stay stable.
tests/unit/test_braille/test_regionLanguageIndexes.py	Updates `patch(...)` targets to new submodule paths.
tests/unit/test_braille/test_calculateWindowRowBufferOffsets.py	Updates `patch(...)` targets to new submodule paths.

LeonarddeR · 2026-06-01T20:37:53Z

+	errorMessage = None
+	if errorMessage and State.INVALID_ENTRY in states:
+		errorMessage = field.get("errorMessage", None)


@SaschaCowley You introduced this in #16411.
I think it must be something like:

Suggested change

errorMessage = None

if errorMessage and State.INVALID_ENTRY in states:

errorMessage = field.get("errorMessage", None)

errorMessage = field.get("errorMessage", None) if State.INVALID_ENTRY in states else None

LeonarddeR · 2026-06-01T20:41:05Z

+			region, regionStart, regionEnd = list(self.regionsWithPositions)[-1]
+			# Show paragraph start indicator if it is now at the left of the current braille window
+			if startPos <= len(paragraphStartMarker) + 1:
+				startPos = self.regionPosToBufferPos(region, regionStart)


Suggested change

region, regionStart, regionEnd = list(self.regionsWithPositions)[-1]

# Show paragraph start indicator if it is now at the left of the current braille window

if startPos <= len(paragraphStartMarker) + 1:

startPos = self.regionPosToBufferPos(region, regionStart)

region, regionStart, regionEnd = list(self.regionsWithPositions)[-1]

# Show paragraph start indicator if it is now at the left of the current braille window

if startPos <= len(paragraphStartMarker) + 1:

startPos = regionStart

+		if self.handler.displaySize == 0:
+			return 0
+		windowPos = max(min(windowPos, self.handler.displaySize), 0)
+		row, col = divmod(windowPos, self.handler.displayDimensions.numCols)
+		if row < len(self._windowRowBufferOffsets):
+			rowPositions = self._windowRowBufferOffsets[row]
+			return max(min(rowPositions.start + col, rowPositions.end - 1), 0)
+		raise ValueError("Position outside window")


+				msg = f"Error in moveToCodepointOffset in iteration {i + 1} (position {curPos}"
+				if i + 1 >= maxIterations or (exceeded := time.time() - startTime > 0.5):
+					logFunc = log.exception
+					curPos = pos
+					if exceeded:
+						msg += ", exceeded time limit of 0.5 seconds"
+				else:
+					logFunc = log.debug
+				logFunc(msg)


+def _getFormattingTags(
+	field: dict[str, str],
+	fieldCache: dict[str, str],
+) -> str | None:
+	"""Get the formatting tags for the given field and cache.
+
+	Formatting tags are calculated according to the preferences passed in formatConfig.
+
+	:param field: The format current field.
+	:param fieldCache: The previous format field.
+	:param formatConfig: The user's formatting preferences.
+	:return: The braille formatting tag as a string, or None if no pertinant formatting is applied.
+	"""


+@param currentCellCount: The current number of cells
+@type currentCellCount: bool


+Filter that allows components or add-ons to change the number of rows and columns used for braille output.
+For example, when a system has a display with 10 rows and 20 columns, but is being controlled by a remote system with a display of 5 rows and 40 coluns, the display number of rows should be lowered to 5.


LeonarddeR · 2026-06-01T20:43:55Z

Not sure what to do with the copilot comments here. Some of them look serious enough to fix, but I don't think this pr is the right place to do so.

seanbudd · 2026-06-03T06:09:40Z

@@ -0,0 +1,1485 @@
+# A part of NonVisual Desktop Access (NVDA)


can this file be split up further? e.g. turn regions into it's own submodule?

Both display and regions are now split even more.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… directly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

NVDAObject.py and textInfo.py now match the casing of the NVDAObjects and textInfos packages respectively. Update patch paths in tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

LeonarddeR and others added 13 commits May 30, 2026 09:05

test(braille): characterize public surface before package split

7920c06

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor(braille): convert braille module to a package with explicit …

1f24085

…__all__ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor(braille): extract labels and constants into braille.labels

daa6ec8

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor(braille): extract BrailleBuffer and focus handling into brai…

8200827

…lle.buffer

refactor(braille): extract display driver classes into braille.display

8869103

refactor(braille): extract BrailleHandler into braille._handler

3f6826c

refactor(braille): finalize package facade and document the split (nv…

f85c0e3

…access#12772)

Remove now redundant docs

efc20a7

Pre-commit auto-fix

160b350

LeonarddeR force-pushed the splitBraillePackage branch from 1500c61 to ef799a8 Compare June 1, 2026 17:47

Fix pointer to braille.handler in BrailleHandler itself

fd836bf

LeonarddeR marked this pull request as ready for review June 1, 2026 19:46

LeonarddeR requested a review from a team as a code owner June 1, 2026 19:46

LeonarddeR requested review from Copilot and seanbudd and removed request for Copilot June 1, 2026 19:46

Update changes

bedb918

Copilot AI review requested due to automatic review settings June 1, 2026 19:49

Copilot AI reviewed Jun 1, 2026

View reviewed changes

seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jun 1, 2026

seanbudd reviewed Jun 2, 2026

View reviewed changes

Comment thread tests/unit/test_braille/test_publicSurface.py Outdated

seanbudd reviewed Jun 2, 2026

View reviewed changes

Comment thread source/braille/__init__.py

LeonarddeR added 4 commits June 2, 2026 21:49

Merge remote-tracking branch 'origin/master' into splitBraillePackage

d488696

Restructure

8972f5a

Sort improts

eec62df

No longer export internals in braille/__init__

ccb079e

seanbudd reviewed Jun 3, 2026

View reviewed changes

Comment thread source/braille/brailleHandler.py

LeonarddeR and others added 4 commits June 6, 2026 09:31

Merge remote-tracking branch 'origin/master' into splitBraillePackage

4048889

refactor(braille): split display.py into display/ package

347bcd3

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor(braille): split regions.py into regions/ package

85231d1

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor(braille): update consumers to import from regions submodules…

df5fa44

… directly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

LeonarddeR requested a review from seanbudd June 6, 2026 16:24

refactor(braille): rename region submodules to match package conventions

96fac63

NVDAObject.py and textInfo.py now match the casing of the NVDAObjects and textInfos packages respectively. Update patch paths in tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(braille): split braille.py into a package#20252

refactor(braille): split braille.py into a package#20252
LeonarddeR wants to merge 25 commits into
nvaccess:masterfrom
LeonarddeR:splitBraillePackage

LeonarddeR commented May 30, 2026 •

edited

Loading

Uh oh!

CyrilleB79 commented May 31, 2026

Uh oh!

LeonarddeR commented Jun 1, 2026

Uh oh!

LeonarddeR commented Jun 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

LeonarddeR Jun 1, 2026

Uh oh!

LeonarddeR Jun 1, 2026

Uh oh!

LeonarddeR commented Jun 1, 2026

Uh oh!

Uh oh!

Uh oh!

seanbudd Jun 3, 2026

Uh oh!

LeonarddeR Jun 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@param currentCellCount: The current number of cells
		@type currentCellCount: bool

		Filter that allows components or add-ons to change the number of rows and columns used for braille output.
		For example, when a system has a display with 10 rows and 20 columns, but is being controlled by a remote system with a display of 5 rows and 40 coluns, the display number of rows should be lowered to 5.

		@@ -0,0 +1,1485 @@
		# A part of NonVisual Desktop Access (NVDA)

Uh oh!

Conversation

LeonarddeR commented May 30, 2026 • 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 user facing changes:

Description of developer facing changes:

Description of development approach:

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Uh oh!

CyrilleB79 commented May 31, 2026

Uh oh!

LeonarddeR commented Jun 1, 2026

Uh oh!

LeonarddeR commented Jun 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

LeonarddeR Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

LeonarddeR Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

LeonarddeR commented Jun 1, 2026

Uh oh!

Uh oh!

Uh oh!

seanbudd Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

LeonarddeR Jun 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

LeonarddeR commented May 30, 2026 •

edited

Loading