Skip to content

Split Turkish braille tables#18726

Merged
SaschaCowley merged 1 commit into
nvaccess:betafrom
OzancanKaratas:TurkishComputerBraille
Aug 19, 2025
Merged

Split Turkish braille tables#18726
SaschaCowley merged 1 commit into
nvaccess:betafrom
OzancanKaratas:TurkishComputerBraille

Conversation

@OzancanKaratas

Copy link
Copy Markdown
Collaborator

Link to issue number:

None

Summary of the issue:

The Turkish braille tables is incorrect in NVDA.

Description of user facing changes:

The user will see the Turkish 8 dot computer braille, Turkish grade 1 and Turkish grade 2.

Description of developer facing changes:

None

Description of development approach:

Add original Turkish grade 1, and rename tr.ctb as Turkish 8 dot computer braille.

Testing strategy:

Manual test using a braille display.

Known issues with pull request:

None

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.

@coderabbitai summary

@josephsl

Copy link
Copy Markdown
Contributor

Hi,

I advise rethinking the PR strategy as braille table splits/merges are first done on Liblouis before brought into NVDA. Further, you need someone who reads and writes Turkish (and Turkish braille code) to test changes.

Thanks.

@OzancanKaratas OzancanKaratas force-pushed the TurkishComputerBraille branch from 50fc739 to 45f76d4 Compare August 17, 2025 19:31
@OzancanKaratas

Copy link
Copy Markdown
Collaborator Author

These changes was already made in liblouis. The tables were mapped incorrectly in NVDA. I fixed this.

@josephsl

josephsl commented Aug 17, 2025 via email

Copy link
Copy Markdown
Contributor

@OzancanKaratas

Copy link
Copy Markdown
Collaborator Author

The changes here are part of liblouis 3.34. But I don't know how to rebase this pull request to the beta branch.

@OzancanKaratas OzancanKaratas marked this pull request as ready for review August 17, 2025 21:25
@OzancanKaratas OzancanKaratas requested a review from a team as a code owner August 17, 2025 21:25
@OzancanKaratas OzancanKaratas force-pushed the TurkishComputerBraille branch from 45f76d4 to a91fea0 Compare August 17, 2025 21:55
@OzancanKaratas OzancanKaratas force-pushed the TurkishComputerBraille branch from 92f4c17 to 45f76d4 Compare August 17, 2025 21:59
@seanbudd seanbudd changed the base branch from master to beta August 18, 2025 02:42
@seanbudd seanbudd requested a review from a team as a code owner August 18, 2025 02:42
@seanbudd seanbudd requested a review from Qchristensen August 18, 2025 02:42
@seanbudd

This comment was marked as outdated.

@seanbudd

Copy link
Copy Markdown
Member

Hi - please ignore the previous comment, here's some simpler suggestions

You'll need to use git rebase.

Make sure to back up your branch to another branch or tag with the same commit before performing the rebase.

Assuming your git remote for nvaccess/nvda is nvaccess:
git fetch nvaccess
git rebase --onto nvaccess/beta master

This may open up an interactive rebase editor if there are merge conflicts.

Alternatively, you can perform a cherry-pick.
Copy all the commits over from your old branch, to a new branch based off of beta.

git checkout beta
git checkout -b newBranchFromBeta
git cherry-pick <commit1FromOldBranch>

Repeat cherry-pick for all the commits you wish to copy over.
Make sure to copy them in the same order.
This effectively is the same action as a git rebase.

@seanbudd seanbudd added this to the 2025.3 milestone Aug 18, 2025
@OzancanKaratas OzancanKaratas force-pushed the TurkishComputerBraille branch 3 times, most recently from 1e661e7 to 8e4d086 Compare August 18, 2025 19:17
(cherry picked from commit 45f76d4)
@OzancanKaratas OzancanKaratas force-pushed the TurkishComputerBraille branch from 8e4d086 to 60488ef Compare August 18, 2025 19:20

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can only mark one table as inputForLangs and outputForLangs per language.

@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Aug 18, 2025
@SaschaCowley SaschaCowley merged commit 6ef33bd into nvaccess:beta Aug 19, 2025
53 of 57 checks passed
SaschaCowley added a commit that referenced this pull request Aug 19, 2025
@LeonarddeR

LeonarddeR commented Aug 19, 2025

Copy link
Copy Markdown
Collaborator

@SaschaCowley Why was this merged while my request for changes was still open? Did I miss something?
This pull request now introduces several warnings as caused by this code block:

	if inputForLangs is not None:
		for lang in inputForLangs:
			if lang in _inputTableForLangs:
				log.warning(
					f"input table lang {lang} already set to {_inputTableForLangs[lang]} overwriting to {table.fileName}",
				)
			_inputTableForLangs[lang] = table.fileName
	if outputForLangs is not None:
		for lang in outputForLangs:
			if lang in _outputTableForLangs:
				log.warning(
					f"output table lang {lang} already set to {_outputTableForLangs[lang]} overwriting to {table.fileName}",
				)

I think these should become errors or even exceptions to ensure this is not missed in the future.

@SaschaCowley

Copy link
Copy Markdown
Member

@LeonarddeR that was my mistake. Note the PR has now been reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants