languageHandler.getAvailableLanguages: use globalVars.appDir rather than depending on the current directory - #11778
Merged
Merged
Conversation
…han relying on the current working directory.
michaelDCurran
changed the title
feerrenrut
previously approved these changes
Oct 21, 2020
feerrenrut
left a comment
feerrenrut
left a comment
Contributor
There was a problem hiding this comment.
Couple of minor things, but doesn't require a second a review.
| locales = [ | ||
| x for x in os.listdir(localesDir) if ( | ||
| not x.startswith('.') | ||
| and os.path.isfile(os.path.join(localesDir, x, 'lc_messages', 'nvda.mo')) |
Contributor
There was a problem hiding this comment.
Note, that "lc_messages" was previously capitalized. Not that this really matters on Windows, but it doesn't hurt to be consistent with what is actually there.
| localesDir = os.path.join(globalVars.appDir, 'locale') | ||
| locales = [ | ||
| x for x in os.listdir(localesDir) if ( | ||
| not x.startswith('.') |
Contributor
There was a problem hiding this comment.
Any idea why this is here? So languages can be disabled? Or because there may have been "dot files" in this directory?
If you know a comment would be good.
Member
Author
|
I believe ignoring directories prefixed with '.' would have been for the
'.svn' directories that would have been present in all directories when
we used subversion.
I think it is definitely safe enough to remove this filter now.
Also thanks for catching the case for LC_MESSAGES, I'll correct that.
|
…cales as we don't use subversion anymore.
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
3 participants
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:
Following on from pr #11650
Summary of the issue:
Pr #11650 aimed to remove all dependence on the current directory being set to NVDA's application directory.
However, another place where we depend on the current directory has been identified:
languageHandler.getAvailableLanguages assumes the 'locale' directory is in the current directory. This should be changed to use globalVars.appDir.
If the current directory is changed to something else, such as when running SAPI5 voices from harposoftware, it is impossible to open the NVDA settings dialog set to the General settings panel as it cannot populate the languages dropdown list. And once this fails, it is then impossible to open Settings ever again until restarting NVDA, as it thinks that the settings dialog is still open.
Description of how this pull request fixes the issue:
Changed languageHandler.getAvailableLanguages to use globalVars.appDir rather than the current directory when listing locales.
Testing performed:
Ran NvDA with the Ivona voice from Harposoftware, and opened the NVDA settings dialog set to the General settings panel by pressing NvDA+control+g. The panel correctly opens and the language list is populated.
Known issues with pull request:
None.
Change log entry:
None needed.