Modernize installer and reg key usage#18851
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the installer and registry key usage by refactoring the installer code to centralize registry keys in config.registry and move relevant paths to WritePaths. It adds type hints throughout the installer module and updates the registry key deletion mechanism to use RegDeleteTreeW instead of a custom implementation.
Key changes:
- Centralized registry keys and paths for better organization
- Added comprehensive type hints to improve code maintainability
- Modernized registry key deletion using Windows API
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| source/winBindings/advapi32.py | Added RegDeleteTreeW Windows API binding for registry operations |
| source/installer.py | Major refactoring with type hints, moved paths to WritePaths, and registry keys to config.registry |
| source/gui/installerGui.py | Updated to use new WritePaths for default install directory |
| source/gui/addonStoreGui/controls/storeDialog.py | Updated import to use new registry constant location |
| source/config/registry.py | Added new registry keys and constants for addon handling |
| source/addonHandler/init.py | Added deprecation handling for moved constants |
| source/NVDAState.py | Added new properties for install directories and start menu folder handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
LeonarddeR
reviewed
Sep 2, 2025
seanbudd
commented
Sep 2, 2025
SaschaCowley
requested changes
Sep 10, 2025
SaschaCowley
left a comment
Member
There was a problem hiding this comment.
Looks pretty good, just a couple of minor things
Co-authored-by: Sascha Cowley <16543535+SaschaCowley@users.noreply.github.com>
de93a9e to
cfc713d
Compare
SaschaCowley
approved these changes
Sep 19, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Part of #16304
Summary of the issue:
We need to refactor large parts of the installer code as part of the 64bit migration.
This is to ensure the old copy of NVDA is cleaned up correctly.
Description of user facing changes:
None
Description of developer facing changes:
Description of development approach:
RegDeleteTreeWto update_deleteKeyAndSubkeys. The current design of_deleteKeyAndSubkeyswas to support Windows XP and olderTesting strategy:
None (suggestions welcome)
Known issues with pull request:
Code Review Checklist: