Store current NVDA's language in globalVars regardless if it has been provided from the command line or not. #13082
Conversation
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
…age rather than accessing `curLang` directly.
lukaszgo1
changed the title
seanbudd
approved these changes
Nov 23, 2021
5 tasks
seanbudd
pushed a commit
that referenced
this pull request
Nov 29, 2021
…en the currently used config via RPC (#13099) Fixes #13094 Summary of the issue: When opening user configuration directory from the start menu NVDA first tries to open path from `globalVars.appArgs.configPath` and if this fails falls back to the default path for the configuration. In PR #13082 a 'fake' implementation of `globalVars.appArgs` has been provided, and therefore `configPath` always exists, but for nvda_slave is set to `None`. This causes a wrong folder to be opened (`ShellExecute` opens a default document folder when given `NULL` as a file to open for whatever reason). Aside from this newly introduced issue if the `configPath` is overridden from the command line for the running instance of NVDA this was not reflected when opening user configuration directory from the start menu since slave has no access to `globalVars` of the running NVDA. Description of how this pull request fixes the issue: - If NVDA is running slave requests NVDA to open currently used configuration folder via RPC - When NVDA is not running, or the version in use has older RPC interface which does not support opening of the config directory the behavior has not changed and the default directory is opened. - Type hints were added to our wrapper around `ShellExecute` and it is no longer possible to pass `None` as a file to be opened. Testing strategy: With the launcher created from the code in this PR: - Ensured that gesture for opening current user config opens correct folder both when configuration has been overridden from the command line or not - With the build from this PR installed and NVDA running activated an option to open configuration directory from the start menu both with the default config and the different folder specified from the command line - made sure that correct folder was opened in both cases - With the build from this PR installed and without running NVDA activated a shortcut to open config folder from the start menu - made sure that the default configuration directory was opened - With the build from this PR installed and older version of NVDA running made sure that when 'explore user configuration directory' is invoked from the start menu default configuration folder is opened and there is no error. - With the build from this PR installed and older version of NVDA running made sure #11867 has not been reintroduced by opening an add-on file from the Windows Explorer.
5 tasks
seanbudd
pushed a commit
that referenced
this pull request
Feb 24, 2022
… globalVars.appArgs in a boolean context (#13386) Fix-up of PR #13082 Summary of the issue: PR #13082 added fake implementation of globalVars.appArgs which is used in cases where command line arguments are not parsed. It introduced two mistakes however: The fake class was named DefautAppArgs (note the missing "l") NVDA's logHandler uses appArgs in a boolean context to check if log viewer should be opened, determine if log fragment can be marked for copy etc. The new class was always True in a boolean context. Description of how this pull request fixes the issue: The class is renamed to DefaultAppArgs. logHandler no longer checks boolean value of appArgs. This is safe since: Both logHandler.Logger.markFragmentStart and logHandler.Logger.getFragment are used only in NVDA and they are invoked by the user action (even before #13082 appArgs was always truthy in that context). The check for appArgs being falthy in logHandler.Logger._log introduced in 2827a34 can also be safely removed since even for NVDA_slave activateLogViewer is set to False (only two cases where it is set to True occur when user invokes log viewer via keyboard command. this check has been added not for any additional security but just to avoid AttributeError being raised when trying to access globalVars.appArgs.secure when logging from slave.
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:
Follow up of #10089 and #12958
Summary of the issue:
PR #12958 introduced additional command line parameter which allows to set NVDA's language. The currently set language is stored in
globalVars.appArgs.languagehowever when it is not overridden from the CLI this variable is set toNone. TO improve consistency withglobalVars.appArgs.configPathit makes sense to store current NVDA's language in globalVars regardless of where it comes from.Description of how this pull request fixes the issue:
Current NVDA language is now stored in
globalVars.appArgs.language- it still can be accessed vialanguageHandler.getLanguagewhich is a recommended way to get its value for add-ons developers. To make this work for nvda_slave it has been necessary to provide a 'fake' implementation ofglobalVars.appArgswhich is used in cases where command line arguments are not available or not yet parsed. This also provides type hints for values ofappArgsso I'd say it is beneficial regardless.Testing strategy:
languageHandlerglobalVars.appArgs.languagecontains the language currently set in the preferences.Known issues with pull request:
None known
Change log entries:
For Developers
languageHandler.curLanghas been removed - to access current NVDA language uselanguageHandler.getLanguage()Code Review Checklist: