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.

Sign up for GitHub

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

Refactor wxWidgets app creation from core.main #16081

Merged
merged 1 commit into from
Jan 29, 2024
Merged

Refactor wxWidgets app creation from core.main #16081

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 66 additions & 45 deletions source/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from dataclasses import dataclass
from typing import (
TYPE_CHECKING,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Love this. TIL about TYPE_CHECKING variable.

Any,
List,
Optional,
Expand All @@ -31,6 +32,9 @@
import NVDAState
from NVDAState import WritePaths

if TYPE_CHECKING:
import wx


def __getattr__(attrName: str) -> Any:
"""Module level `__getattr__` used to preserve backward compatibility.
Expand Down Expand Up @@ -527,6 +531,66 @@ def _doLoseFocus():
log.exception("Lose focus error")


def _setUpWxApp() -> "wx.App":
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really like the refactor of this out of main(). Much better.

import six
import wx

import config
import nvwave
import speech

log.info(f"Using wx version {wx.version()} with six version {six.__version__}")

# Disables wx logging in secure mode due to a security issue: GHSA-h7pp-6jqw-g3pj
# This is due to the wx.LogSysError dialog allowing a file explorer dialog to be opened.
wx.Log.EnableLogging(not globalVars.appArgs.secure)

class App(wx.App):
def OnAssert(self, file: str, line: str, cond: str, msg: str):
message = f"{file}, line {line}:\nassert {cond}: {msg}"
log.debugWarning(message, codepath="wxWidgets", stack_info=True)

def InitLocale(self):
"""Custom implementation of `InitLocale` which ensures that wxPython does not change the locale.
The current wx implementation (as of wxPython 4.1.1) sets Python locale to an invalid one
which triggers Python issue 36792 (#12160).
The new implementation (wxPython 4.1.2) sets locale to "C" (basic Unicode locale).
While this is not wrong as such NVDA manages locale themselves using `languageHandler`
and it is better to remove wx from the equation so this method is a No-op.
This code may need to be revisited when we update Python / wxPython.
"""
pass

app = App(redirect=False)

# We support queryEndSession events, but in general don't do anything for them.
# However, when running as a Windows Store application, we do want to request to be restarted for updates
def onQueryEndSession(evt):
if config.isAppX:
# Automatically restart NVDA on Windows Store update
ctypes.windll.kernel32.RegisterApplicationRestart(None, 0)

app.Bind(wx.EVT_QUERY_END_SESSION, onQueryEndSession)

def onEndSession(evt):
# NVDA will be terminated as soon as this function returns, so save configuration if appropriate.
config.saveOnExit()
speech.cancelSpeech()
if not globalVars.appArgs.minimal and config.conf["general"]["playStartAndExitSounds"]:
try:
nvwave.playWaveFile(
os.path.join(globalVars.appDir, "waves", "exit.wav"),
asynchronous=False
)
except Exception:
log.exception("Error playing exit sound")
log.info("Windows session ending")

app.Bind(wx.EVT_END_SESSION, onEndSession)

return app


def main():
"""NVDA's core main loop.
This initializes all modules such as audio, IAccessible, keyboard, mouse, and GUI.
Expand Down Expand Up @@ -611,53 +675,10 @@ def main():
log.debugWarning("Slow starting core (%.2f sec)" % timeSinceStart)
# Translators: This is spoken when NVDA is starting.
speech.speakMessage(_("Loading NVDA. Please wait..."))
import wx
import six

# Disables wx logging in secure mode due to a security issue: GHSA-h7pp-6jqw-g3pj
# This is due to the wx.LogSysError dialog allowing a file explorer dialog to be opened.
wx.Log.EnableLogging(not globalVars.appArgs.secure)

log.info("Using wx version %s with six version %s"%(wx.version(), six.__version__))
class App(wx.App):
def OnAssert(self,file,line,cond,msg):
message="{file}, line {line}:\nassert {cond}: {msg}".format(file=file,line=line,cond=cond,msg=msg)
log.debugWarning(message,codepath="WX Widgets",stack_info=True)

def InitLocale(self):
"""Custom implementation of `InitLocale` which ensures that wxPython does not change the locale.
The current wx implementation (as of wxPython 4.1.1) sets Python locale to an invalid one
which triggers Python issue 36792 (#12160).
The new implementation (wxPython 4.1.2) sets locale to "C" (basic Unicode locale).
While this is not wrong as such NVDA manages locale themselves using `languageHandler`
and it is better to remove wx from the equation so this method is a No-op.
This code may need to be revisited when we update Python / wxPython.
"""
pass

import wx
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given that you're importing wx in 3 different places, including in the main execution flow, can we just put the single import up at the top of the core.py file? If there's no way to avoid loading it (which it looks like from def main()), we can simplify the imports and remove the TYPE_CHECKING logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe we are intentionally avoiding the wx import until further down. Dependency importing is incredibly order specific in nvda.pyw and core.py

app = _setUpWxApp()

app = App(redirect=False)
# We support queryEndSession events, but in general don't do anything for them.
# However, when running as a Windows Store application, we do want to request to be restarted for updates
def onQueryEndSession(evt):
if config.isAppX:
# Automatically restart NVDA on Windows Store update
ctypes.windll.kernel32.RegisterApplicationRestart(None,0)
app.Bind(wx.EVT_QUERY_END_SESSION, onQueryEndSession)
def onEndSession(evt):
# NVDA will be terminated as soon as this function returns, so save configuration if appropriate.
config.saveOnExit()
speech.cancelSpeech()
if not globalVars.appArgs.minimal and config.conf["general"]["playStartAndExitSounds"]:
try:
nvwave.playWaveFile(
os.path.join(globalVars.appDir, "waves", "exit.wav"),
asynchronous=False
)
except:
pass
log.info("Windows session ending")
app.Bind(wx.EVT_END_SESSION, onEndSession)
log.debug("Initializing braille input")
import brailleInput
brailleInput.initialize()
Expand Down