Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ability to translate the user guide on Crowdin #17050

Merged
merged 88 commits into from
Sep 2, 2024
Merged

add ability to translate the user guide on Crowdin #17050

merged 88 commits into from
Sep 2, 2024

Conversation

michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Aug 26, 2024

Link to issue number:

None.

Summary of the issue:

Currently user documentation such as the user guide are made translatable via a custom (and very old) translation system hosted by NV Access. For many reasons we need to move away from this old system to something more mainstream and maintainable. We have already successfully moved translation of NVDA interface messages to Crowdin, and we should do the same for the user guide and other documentation.

Description of user facing changes

None.

Description of development approach

  • Added markdownTranslate.py, which contains several commands for generating and updating xliff files from markdown files. These xliff files can then be uploaded to Crowdin for translation, and eventually downloaded again and converted back to markdown files. Commands include:
    • generateXliff: to generate an xliff file from a markdown file. Firstly a 'skeleton' of the markdown file is produced which is all the structure of a markdown file, but the translatable content on each line has been replaced by a special translation ID. Lines such as blank lines, hidden header rows, or table header separator lines are included in the skeleton in tact and are not available for translation. The xliff file is then produced, which contains one translatable string per translation unit, keyed by its respective translation ID. Each unit also contains translator notes to aide in translation, such as the line number, and any prefix or suffix markdown structure. E.g. a heading might have a prefix of ### and a suffix of {#Intro}. The skeleton is also embedded into the xliff file so that it is possible to update the xliff file keeping existing translation IDs, and or generate the existing markdown file from the xliff file.
    • generateMarkdown: Given an xliff file, a the original markdown file is reproduced from the embedded skeleton, and either the translated or source strings from the xliff file, depending on whether you want a translated or untranslated markdown file.
    • updateXliff: to update an existing xliff file with changes from a markdown file, ensuring that IDs of existing translatable strings are kept in tact. This command extracts the skeleton from the xliff file, makes a diff of the old and new markdown files, then applies this diff to the skeleton file I.e. removes skeleton lines that were removed from the markdown file, and adds skeleton lines (with new IDs) for lines that are newly added to the markdown file. All existing lines stay as is keeping their existing translation IDs. Finally a new xliff file is generated from the up to date markdown file and skeleton, resulting in an xliff file that contains all translatable strings from the new markdown file, but reusing translation IDs for existing strings.
    • translateXliff: given an xliff file, and a pretranslated markdown file that matches the skeleton, a new xliff file is produced containing translations for all strings.
    • pretranslateAllPossibleLangs: this walks the NVDA user_docs directory, and for each language, pretranslates the English xliff file using the existing pretranslated markdown file from the old translation system (if it matches the skeleton exactly) producing a translated xliff file that can be uploaded to Crowdin to bring an existing translation up to where it was in the old system.
    • runTests: runs some tests using 2024.2 and 2024.3 English and French user guides to ensure files are translated and generated as expected.
  • Added a generated xliff file for the current English user guide markdown file. Note that this has been uploaded to Crowdin for translation.
  • Added a GitHub action that runs on the beta branch if English userGuide.md changes. The action regenerates the original markdown file from the current English user guide xliff, then updates the xliff file based on the changes from the original markdown file to the current markdown file. This xliff file is then uploaded to Crowdin, and also committed and pushed to beta.

Testing strategy:

  • Uploaded the English user guide xliff to Crowdin.
  • pretranslated a French xliff file from the English user guide xliff and a pretranslated French markdown file.
  • Uploaded the pretranslated French xliff file to Crowdin for French.
  • Downloaded the latest French xliff file from Crwdin, ensuring all strings were translated.
  • Added / removed several lines from the English user guide markdown file and committed changes.
  • Ensured the GitHub action ran with no errors.
  • Downloded the French xliff from Crowdin and confirmed that the new added strings were available for translation, the removed strings were no longer available, and that all existing strings were still translated appropriately.

Known issues with pull request:

None known.

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.

Summary by CodeRabbit

  • New Features

    • Introduced an automated workflow for updating user documentation based on changes in markdown files, enhancing documentation consistency.
  • Bug Fixes

    • Improved formatting and readability of various functions and classes across the codebase without altering functionality.
  • Documentation

    • Enhanced clarity in comments and docstrings throughout the code, contributing to better maintainability.
  • Chores

    • General code style improvements, including consistent spacing and formatting adjustments to adhere to coding standards.

Copy link
Contributor

coderabbitai bot commented Aug 26, 2024

Walkthrough

The changes encompass the introduction of a new GitHub Actions workflow for automating user documentation updates, modifications to Python scripts for improved formatting and readability, and various minor adjustments across multiple files to enhance code consistency. The updates include changes to string formatting, function signatures, and the organization of code, while maintaining the existing functionality and logic throughout the project.

Changes

Files Change Summary
.github/workflows/regenerate_english_userDocs_translation_source.yml New workflow automates user documentation updates based on changes in specific markdown files, processing modified files and syncing updates to Crowdin.
appveyor/crowdinSync.py, appveyor/mozillaSyms.py, site_scons/... Formatting improvements across multiple Python files, including adjustments to function signatures, consolidation of multi-line statements, and consistent use of string delimiters.
extras/controllerClient/examples/example_python.py, source/... Minor formatting changes, including string delimiter adjustments and whitespace modifications, enhancing code consistency without altering functionality.
projectDocs/dev/developerGuide/conf.py, source/NVDAObjects/... Formatting and style consistency improvements, including whitespace adjustments and standardization of string delimiters, applied across various configuration and source files.
source/NVDAHelper.py, source/NVDAObjects/UIA/... Code formatting changes for improved readability, including consistent spacing around operators and method parameters, with no changes to the underlying logic or functionality.
source/NVDAObjects/IAccessible/..., source/NVDAObjects/UIA/... Adjustments to class and method definitions for better readability, including formatting changes in comments and method signatures, while preserving the original functionality.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant GitHub Actions
    participant Python Script
    participant Crowdin

    User->>GitHub Actions: Push changes to markdown files
    GitHub Actions->>Python Script: Trigger update documentation process
    Python Script->>Python Script: Process modified markdown files
    Python Script->>Crowdin: Upload updated .xliff files
    Crowdin-->>Python Script: Confirmation of upload
    Python Script->>GitHub Actions: Commit changes
    GitHub Actions-->>User: Documentation updated
Loading

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@michaelDCurran
Copy link
Member Author

@seanbudd It seems that precommit is reformatting all files in the repo which makes this pr impossible to review. I guess I'll need to wait for all the reformatting to reach beta and then remove the final precommit here before I can make it ready for review?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

Outside diff range, codebase verification and nitpick comments (16)
source/NVDAObjects/IAccessible/sysListView32.py (2)

Line range hint 421-466: LGTM! But address the static analysis hints.

The methods within the ListItem class are correctly defined. However, address the static analysis hints about undefined imports.

Apply this diff to explicitly import the required modules:

+from ctypes import byref, sizeof

class ListItem(RowWithFakeNavigation, RowWithoutCellObjects, ListItemWithoutColumnSupport):
	parent: List

	def _getColumnLocationRawInProc(self, index: int) -> ctypes.wintypes.RECT:
		"""Retrieves rectangle containing coordinates for a given column.
		Note that this method operates in process and cannot be used in situations where NVDA cannot inject
		i.e when running as a Windows Store application or when no focus event was received on startup.
		"""
		item = self.IAccessibleChildID - 1
		subItem = index
		rect = ctypes.wintypes.RECT()
		if (
			watchdog.cancellableExecute(
				NVDAHelper.localLib.nvdaInProcUtils_sysListView32_getColumnLocation,
				self.appModule.helperLocalBindingHandle,
				self.windowHandle,
				item,
				subItem,
				ctypes.byref(rect),
			)
			!= 0
		):
			return None
		return rect
Tools
Ruff

452-452: sizeof may be undefined, or defined from star imports

(F405)


456-456: byref may be undefined, or defined from star imports

(F405)


456-456: sizeof may be undefined, or defined from star imports

(F405)


Line range hint 507-560: LGTM! But address the static analysis hints.

The methods within the ListItem class are correctly defined. However, address the static analysis hints about undefined imports.

Apply this diff to explicitly import the required modules:

+from ctypes import byref, sizeof

class ListItem(RowWithFakeNavigation, RowWithoutCellObjects, ListItemWithoutColumnSupport):
	parent: List

	def _getColumnContentRawInProc(self, index: int) -> Optional[str]:
		"""Retrieves text for a given column.
		Note that this method operates in process and cannot be used in situations where NVDA cannot inject
		i.e when running as a Windows Store application or when no focus event was received on startup.
		"""
		item = self.IAccessibleChildID - 1
		subItem = index
		text = AutoFreeBSTR()
		if (
			watchdog.cancellableExecute(
				NVDAHelper.localLib.nvdaInProcUtils_sysListView32_getColumnContent,
				self.appModule.helperLocalBindingHandle,
				self.windowHandle,
				item,
				subItem,
				ctypes.byref(text),
			)
			!= 0
		):
			return None
		return text.value
Tools
Ruff

531-531: sizeof may be undefined, or defined from star imports

(F405)


546-546: byref may be undefined, or defined from star imports

(F405)


546-546: sizeof may be undefined, or defined from star imports

(F405)


553-553: byref may be undefined, or defined from star imports

(F405)


553-553: sizeof may be undefined, or defined from star imports

(F405)

source/NVDAHelper.py (14)

69-70: Remove unnecessary noqa comment.

The noqa comment is not necessary for this line and can be removed.

Apply this diff to remove the noqa comment:

-	cast(getattr(dll, name), POINTER(c_void_p)).contents.value = cast(cfunc, c_void_p).value  # noqa: F405
+	cast(getattr(dll, name), POINTER(c_void_p)).contents.value = cast(cfunc, c_void_p).value

74-81: Move import statement to the top of the file.

The import statement for speech should be at the top of the file to follow best practices.

Apply this diff to move the import statement:

74c74
< @WINFUNCTYPE(c_long, c_wchar_p)
---
> import speech
81c81
< 	import speech
---
> 

Line range hint 90-152: Move import statements to the top of the file and refactor the function.

The import statements for speech, SymbolLevel, SpeechPriority, and _getSpeakSsmlSpeech should be at the top of the file to follow best practices. Additionally, the function could be simplified by breaking it down into smaller helper functions.

Apply this diff to move the import statements:

90c90
< @WINFUNCTYPE(c_long, c_wchar_p, c_int, c_int, c_bool)
---
> import speech
> from characterProcessing import SymbolLevel
> from speech.priorities import SpeechPriority
> from speech.speech import _getSpeakSsmlSpeech
> 
> @WINFUNCTYPE(c_long, c_wchar_p, c_int, c_int, c_bool)

Consider refactoring the function into smaller helper functions to improve readability and maintainability.


174-179: Move import statement to the top of the file.

The import statement for speech should be at the top of the file to follow best practices.

Apply this diff to move the import statement:

174c174
< @WINFUNCTYPE(c_long)
---
> import speech
179c179
< 	import speech
---
> 

183-190: Move import statement to the top of the file.

The import statement for braille should be at the top of the file to follow best practices.

Apply this diff to move the import statement:

183c183
< @WINFUNCTYPE(c_long, c_wchar_p)
---
> import braille
190c190
< 	import braille
---
> 

225-241: Remove unnecessary noqa comments.

The noqa comments are not necessary for these lines and can be removed.

Apply this diff to remove the noqa comments:

225c225
< 	windll.rpcrt4.I_RpcBindingInqLocalClientPID(None, byref(pid))  # noqa: F405
---
> 	windll.rpcrt4.I_RpcBindingInqLocalClientPID(None, byref(pid))
241c241
< 		windll.rpcrt4.RpcBindingFree(byref(bindingHandle))  # noqa: F405
---
> 		windll.rpcrt4.RpcBindingFree(byref(bindingHandle))

Line range hint 252-285: Move import statements to the top of the file.

The import statements for speech, braille, AriaLivePoliteness, and Spri should be at the top of the file to follow best practices.

Apply this diff to move the import statements:

252c252
< @WINFUNCTYPE(c_long, c_wchar_p, c_wchar_p)
---
> import speech
> import braille
> from aria import AriaLivePoliteness
> from speech.priorities import Spri
> 
> @WINFUNCTYPE(c_long, c_wchar_p, c_wchar_p)

289-292: Move import statement to the top of the file.

The import statement for displayModel should be at the top of the file to follow best practices.

Apply this diff to move the import statement:

289c289
< @WINFUNCTYPE(c_long, c_long, c_long, c_long, c_long, c_long)
---
> import displayModel
292c292
< 	import displayModel
---
> 

297-304: Move import statements to the top of the file.

The import statements for eventHandler and Window should be at the top of the file to follow best practices.

Apply this diff to move the import statements:

297c297
< @WINFUNCTYPE(c_long, c_long, c_long, c_long, c_long, c_long)
---
> import eventHandler
> from NVDAObjects.window import Window
304c304
< 	import eventHandler
---
> 

309-318: Move import statement to the top of the file.

The import statement for getAppNameFromProcessID should be at the top of the file to follow best practices.

Apply this diff to move the import statement:

309c309
< @WINFUNCTYPE(c_long, c_long, c_long, c_wchar_p)
---
> from appModuleHandler import getAppNameFromProcessID
318c318
< 	from appModuleHandler import getAppNameFromProcessID
---
> 

321-354: Move import statements to the top of the file.

The import statements for speech, characterProcessing, InputComposition, and ModernCandidateUICandidateItem should be at the top of the file to follow best practices.

Apply this diff to move the import statements:

321c321
< def handleInputCompositionEnd(result):
---
> import speech
> import characterProcessing
> from NVDAObjects.inputComposition import InputComposition
> from NVDAObjects.IAccessible.mscandui import ModernCandidateUICandidateItem
> 
> def handleInputCompositionEnd(result):
327c327
< 	import speech
---
> 

363-389: Move import statements to the top of the file.

The import statements for speech, InputComposition, and CandidateItem should be at the top of the file to follow best practices.

Apply this diff to move the import statements:

363c363
< def handleInputCompositionStart(compositionString, selectionStart, selectionEnd, isReading):
---
> import speech
> from NVDAObjects.inputComposition import InputComposition
> from NVDAObjects.behaviors import CandidateItem
> 
> def handleInputCompositionStart(compositionString, selectionStart, selectionEnd, isReading):
367c367
< 	import speech
---
> 

395-414: Move import statements to the top of the file.

The import statements for InputComposition and ModernCandidateUICandidateItem should be at the top of the file to follow best practices.

Apply this diff to move the import statements:

395c395
< @WINFUNCTYPE(c_long, c_wchar_p, c_int, c_int, c_int)
---
> from NVDAObjects.inputComposition import InputComposition
> from NVDAObjects.IAccessible.mscandui import ModernCandidateUICandidateItem
> 
> @WINFUNCTYPE(c_long, c_wchar_p, c_int, c_int, c_int)
398c398
< 	from NVDAObjects.inputComposition import InputComposition
---
> 

418-459: Move import statements to the top of the file.

The import statements for speech and CandidateItem should be at the top of the file to follow best practices.

Apply this diff to move the import statements:

418c418
< def handleInputCandidateListUpdate(candidatesString, selectionIndex, inputMethod):
---
> import speech
> from NVDAObjects.inputComposition import CandidateItem
> 
> def handleInputCandidateListUpdate(candidatesString, selectionIndex, inputMethod):
422c422
< 	import speech
---
> 

source/IAccessibleHandler/internalWinEventHandler.py Outdated Show resolved Hide resolved
sconstruct Outdated Show resolved Hide resolved
source/NVDAObjects/IAccessible/sysListView32.py Outdated Show resolved Hide resolved
source/NVDAObjects/IAccessible/sysListView32.py Outdated Show resolved Hide resolved
source/NVDAObjects/IAccessible/sysListView32.py Outdated Show resolved Hide resolved
source/NVDAHelper.py Outdated Show resolved Hide resolved
source/NVDAObjects/window/edit.py Outdated Show resolved Hide resolved
source/NVDAObjects/window/_msOfficeChart.py Outdated Show resolved Hide resolved
@AppVeyorBot
Copy link

  • FAIL: Translation comments check. Translation comments missing or unexpectedly included. See build log for more information.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/i1hc4h6dbx1cpvah/artifacts/output/nvda_snapshot_pr17050-33517,1eeabac9.exe
  • CI timing (mins):
    INIT 0.0,
    INSTALL_START 1.3,
    INSTALL_END 1.0,
    BUILD_START 0.0,
    BUILD_END 12.9,
    TESTSETUP_START 0.0,
    TESTSETUP_END 0.4,
    TEST_START 0.0,
    TEST_END 18.4,
    FINISH_END 0.1

See test results for failed build of commit 1eeabac94e

@CyrilleB79
Copy link
Collaborator

Before merging this PR, could you send a message to the translators mailing list and give a deadline for them to provide up-to-date translations?
You should explain that it's the last chance for them to reach the identical structure state, allowing to benefit from automatic translation in the new file.
Thanks.

@dpy013
Copy link
Contributor

dpy013 commented Aug 26, 2024

Before merging this PR, could you send a message to the translators mailing list and give a deadline for them to provide up-to-date translations? You should explain that it's the last chance for them to reach the identical structure state, allowing to benefit from automatic translation in the new file. Thanks.
Suggest updating as well:
#15014
Update on the plan

@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Aug 27, 2024
@seanbudd seanbudd added this to the 2024.4 milestone Aug 27, 2024
…g post table header line and hidden header row.
…skel files if English user documentation markdown files change.
…file so that it is theoretically possible to regenerate the markdown withonly the xliff. generateMarkdown and translateXliff commands now read the skeleton straight from the xliff file, rather than receiving it as an argument.
…nt, as it already reads the skeleton content from the xliff.
@AppVeyorBot
Copy link

See test results for failed build of commit 71f69ee1fd

@michaelDCurran michaelDCurran marked this pull request as ready for review August 29, 2024 23:29
@AppVeyorBot
Copy link

See test results for failed build of commit 52b91b3423

tests/unit/test_markdownTranslate.py Outdated Show resolved Hide resolved
user_docs/markdownTranslate.py Show resolved Hide resolved
user_docs/markdownTranslate.py Outdated Show resolved Hide resolved
user_docs/markdownTranslate.py Outdated Show resolved Hide resolved
user_docs/markdownTranslate.py Outdated Show resolved Hide resolved
@AppVeyorBot
Copy link

See test results for failed build of commit 7bd7ab11ff

@AppVeyorBot
Copy link

See test results for failed build of commit 32ae2bcc24

@michaelDCurran
Copy link
Member Author

@coderabbitai resolve

@michaelDCurran michaelDCurran merged commit 6fbbc10 into beta Sep 2, 2024
2 checks passed
@michaelDCurran michaelDCurran deleted the xliff branch September 2, 2024 03:32
@dpy013 dpy013 mentioned this pull request Sep 2, 2024
5 tasks
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.

6 participants