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

Streamline importing of addon directories for python packages #14350

Merged
merged 6 commits into from
Nov 10, 2022

Conversation

LeonarddeR
Copy link
Collaborator

@LeonarddeR LeonarddeR commented Nov 8, 2022

Link to issue number:

Closes #14340

Summary of the issue:

For addons, the several directories are added to the python package path using a function in the config module. This needs refactoring:

# FIXME: this should not be coupled to the config module....

Description of user facing changes

None

Description of development approach

This pr changes logic to add the several addon directories to the package path in the addonHandler.

Testing strategy:

Tested a source copy of NVDA. Ensured that at least appModules and globalPlugins are loaded correctly. synthDrivers, visionEnhancementProviders and brailleDisplayDrivers need to be tested.

Known issues with pull request:

None known

Change log entries:

deprecations:

- config.addConfigDirsToPythonPackagePath has been moved and renamed. Use addonHandler.packaging.addDirsToPythonPackagePath instead. (#14350)

Code Review Checklist:

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

@LeonarddeR LeonarddeR requested a review from a team as a code owner November 8, 2022 14:46
source/addonHandler/__init__.py Outdated Show resolved Hide resolved
source/addonHandler/__init__.py Outdated Show resolved Hide resolved
source/config/__init__.py Show resolved Hide resolved
source/addonHandler/__init__.py Outdated Show resolved Hide resolved
source/addonHandler/__init__.py Outdated Show resolved Hide resolved
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@LeonarddeR LeonarddeR marked this pull request as draft November 9, 2022 07:13
@AppVeyorBot
Copy link

See test results for failed build of commit 3ba0bb23c6

@LeonarddeR LeonarddeR marked this pull request as ready for review November 9, 2022 12:31
@AppVeyorBot
Copy link

See test results for failed build of commit dcbad591cf

@LeonarddeR
Copy link
Collaborator Author

I don't think these failing tests are related.

@seanbudd
Copy link
Member

I don't think these failing tests are related.

Yes, looks like this was caused by #14342

@seanbudd seanbudd merged commit 4ed2cd5 into nvaccess:master Nov 10, 2022
@nvaccessAuto nvaccessAuto added this to the 2023.1 milestone Nov 10, 2022
@LeonarddeR LeonarddeR mentioned this pull request Nov 28, 2022
6 tasks
seanbudd pushed a commit that referenced this pull request Nov 28, 2022
Fixes #14408

Summary of the issue:
PR #14350 streamlined import code for globalPlugins and appModules. However, the reloading of these modules was overlooked.

Description of user facing changes
Reloading of appModules/globalPlugins works again

Description of development approach
The reloading functions for both appModules and globalPlugins call addonhandler.packaging.addDirsToPythonPackagePath
seanbudd pushed a commit that referenced this pull request Apr 18, 2023
Fix-up of #14350.

Summary of the issue:
Running NVDA 2023.1 (and older), with add-ons and/or scratchpad enabled:

I get the following deprecation warnings:

When opening start menu:
WARNING - appModuleHandler._warnDeprecatedAliasAppModule (15:49:46.531) - MainThread (19688):
Importing appModules.searchapp is deprecated, instead import appModules.searchui.
When switching to Notepad++:
WARNING - appModuleHandler._warnDeprecatedAliasAppModule (15:50:26.621) - MainThread (19688):
Importing appModules.notepad++ is deprecated, instead import appModules.notepadPlusPlus.
And if I force errors on deprecated code by modifying NVDAState._allowDeprecatedAPI to return False, I get errors instead.

This is not expected since these warnings/errors come from NVDA's core code which should not (and actually does not) contain deprecated code.

Description of user facing changes
No more wrong deprecation warnings/errors due to NVDA's core appModules when opening start menu or switching to Notepad++

Description of development approach
appModules.__path__ contains the paths where NVDA has to search appModules. It contains first the folder in the scratchpad, then the folders in add-on containing appModules and finally the core appModule folder. Thus we need to look at the last element of appModules.__path__, not the first as it was done.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Streamline importing of addon directories for python packages
4 participants