From 403917ded1048de72764c91ae9a762260d061f66 Mon Sep 17 00:00:00 2001 From: Sean Budd Date: Fri, 14 Jun 2024 15:37:03 +1000 Subject: [PATCH] address review comments --- source/core.py | 9 ++- source/gui/installerGui.py | 113 +++++++++++++++++++++++++------------ source/nvda.pyw | 19 ++++++- user_docs/en/changes.md | 2 +- user_docs/en/userGuide.md | 2 +- 5 files changed, 102 insertions(+), 43 deletions(-) diff --git a/source/core.py b/source/core.py index 0ababdd32ca..193c28edfbd 100644 --- a/source/core.py +++ b/source/core.py @@ -806,8 +806,13 @@ def main(): ) elif globalVars.appArgs.portablePath and (globalVars.appArgs.createPortable or globalVars.appArgs.createPortableSilent): import gui.installerGui - wx.CallAfter(gui.installerGui.doCreatePortable,portableDirectory=globalVars.appArgs.portablePath, - silent=globalVars.appArgs.createPortableSilent,startAfterCreate=not globalVars.appArgs.createPortableSilent) + wx.CallAfter( + gui.installerGui.doCreatePortable, + portableDirectory=globalVars.appArgs.portablePath, + silent=globalVars.appArgs.createPortableSilent, + startAfterCreate=not globalVars.appArgs.createPortableSilent, + warnForNonEmptyDirectory=not globalVars.appArgs.createPortableSilent, + ) elif not globalVars.appArgs.minimal: try: # Translators: This is shown on a braille display (if one is connected) when NVDA starts. diff --git a/source/gui/installerGui.py b/source/gui/installerGui.py index 84745d59640..a5cc0c2596a 100644 --- a/source/gui/installerGui.py +++ b/source/gui/installerGui.py @@ -18,6 +18,7 @@ import gui.contextHelp from gui.dpiScalingHelper import DpiScalingHelperMixinWithoutInit import systemUtils +import ui from NVDAState import WritePaths from .message import displayDialogAsModal @@ -340,6 +341,59 @@ def showInstallGui(): gui.mainFrame.postPopup() +def _warnForNonEmptyDirectory(portableDirectory: str) -> bool: + """ + Display a warning message if the specified directory is not empty. + :param portableDirectory: The directory to check. + :return: True if the user wants to continue, False if the user wants to cancel. + """ + if os.path.exists(portableDirectory): + dirContents = os.listdir(portableDirectory) + else: + dirContents = [] + if len(dirContents) > 0: + if "nvda.exe" in dirContents: + if wx.NO == gui.messageBox( + # Translators: The message displayed when the user has specified a destination directory + # that already has a portable copy in the Create Portable NVDA dialog. + _("A portable copy already exists in this directory. Do you want to update it?"), + # Translators: The title of a dialog presented when the user has specified a destination directory + # that already has a portable copy in the Create Portable NVDA dialog. + _("Portable Copy Exists"), + wx.YES_NO | wx.ICON_QUESTION + ): + return False + elif wx.NO == gui.messageBox( + _( + # Translators: The message displayed when the user has specified a destination directory + # that already exists in the Create Portable NVDA dialog. + "The specified directory is not empty. " + "Proceeding will delete and replace existing files in the directory. " + "Do you want to overwrite the contents of this folder? " + ), + # Translators: The title of a dialog presented when the user has specified a destination directory + # that already exists in the Create Portable NVDA dialog. + _("Directory Exists"), + wx.YES_NO | wx.ICON_QUESTION + ): + return False + return True + + +def _getUniqueNewPortableDirectory(basePath: str) -> str: + """ + Generate a new directory name for a portable copy of NVDA. + :param basePath: The base path to use. + :return: A new directory name. + """ + newPortableDirectory = os.path.join(basePath, "NVDA") + i = 1 + while os.path.exists(newPortableDirectory): + newPortableDirectory = os.path.join(basePath, f"NVDA_{i}") + i += 1 + return newPortableDirectory + + class PortableCreaterDialog( gui.contextHelp.ContextHelpMixin, wx.Dialog, # wxPython does not seem to call base class initializer, put last in MRO @@ -436,46 +490,18 @@ def onCreatePortable(self, evt): # needed. The OS's idea of the current drive is used, as in os.getcwd(). (#14681) expandedPortableDirectory = os.path.abspath(expandedPortableDirectory) if self.newFolderCheckBox.Value: - newPortableDirectory = os.path.join(expandedPortableDirectory, "NVDA") - i = 1 - while os.path.exists(newPortableDirectory): - newPortableDirectory = os.path.join(expandedPortableDirectory, f"NVDA_{i}") - i += 1 - expandedPortableDirectory = newPortableDirectory - - if os.path.exists(expandedPortableDirectory): - dirContents = os.listdir(expandedPortableDirectory) - else: - dirContents = [] - if len(dirContents) > 0: - if "nvda.exe" in dirContents: - if wx.NO == gui.messageBox( - # Translators: The message displayed when the user has specified a destination directory - # that already has a portable copy in the Create Portable NVDA dialog. - _("A portable copy already exists in this directory. Do you want to update it?"), - # Translators: The title of a dialog presented when the user has specified a destination directory - # that already has a portable copy in the Create Portable NVDA dialog. - _("Portable Copy Exists"), - wx.YES_NO | wx.ICON_QUESTION - ): - return - elif wx.NO == gui.messageBox( - # Translators: The message displayed when the user has specified a destination directory - # that already exists in the Create Portable NVDA dialog. - _("The specified directory is not empty. Do you want to overwrite the contents of this folder?"), - # Translators: The title of a dialog presented when the user has specified a destination directory - # that already exists in the Create Portable NVDA dialog. - _("Directory Exists"), - wx.YES_NO | wx.ICON_QUESTION - ): - return + expandedPortableDirectory = _getUniqueNewPortableDirectory(expandedPortableDirectory) + + if not _warnForNonEmptyDirectory(expandedPortableDirectory): + return self.Hide() doCreatePortable( expandedPortableDirectory, - self.copyUserConfigCheckbox.Value, - False, - self.startAfterCreateCheckbox.Value + copyUserConfig=self.copyUserConfigCheckbox.Value, + silent=False, + startAfterCreate=self.startAfterCreateCheckbox.Value, + warnForNonEmptyDirectory=False, ) self.Destroy() @@ -487,8 +513,21 @@ def doCreatePortable( portableDirectory: str, copyUserConfig: bool = False, silent: bool = False, - startAfterCreate: bool = False + startAfterCreate: bool = False, + warnForNonEmptyDirectory: bool = True, ) -> None: + """ + Create a portable copy of NVDA. + :param portableDirectory: The directory in which to create the portable copy. + :param copyUserConfig: Whether to copy the current user configuration. + :param silent: Whether to suppress messages. + :param startAfterCreate: Whether to start the new portable copy after creation. + :param warnForNonEmptyDirectory: Whether to warn if the destination directory is not empty. + """ + if warnForNonEmptyDirectory and not _warnForNonEmptyDirectory(portableDirectory): + # Translators: The message displayed when the user cancels the creation of a portable copy of NVDA. + ui.message(_("Portable copy creation cancelled.")) + return d = gui.IndeterminateProgressDialog( gui.mainFrame, # Translators: The title of the dialog presented while a portable copy of NVDA is being created. diff --git a/source/nvda.pyw b/source/nvda.pyw index 0f7b57d22ae..d9e8c1c633a 100755 --- a/source/nvda.pyw +++ b/source/nvda.pyw @@ -191,8 +191,23 @@ parser.add_argument('--no-sr-flag',action="store_false",dest='changeScreenReader installGroup = parser.add_mutually_exclusive_group() installGroup.add_argument('--install',action="store_true",dest='install',default=False,help="Installs NVDA (starting the new copy after installation)") installGroup.add_argument('--install-silent',action="store_true",dest='installSilent',default=False,help="Installs NVDA silently (does not start the new copy after installation).") -installGroup.add_argument('--create-portable',action="store_true",dest='createPortable',default=False,help="Creates a portable copy of NVDA (starting the new copy after installation)") -installGroup.add_argument('--create-portable-silent',action="store_true",dest='createPortableSilent',default=False,help="Creates a portable copy of NVDA silently (does not start the new copy after installation).") +installGroup.add_argument( + "--create-portable", + action="store_true", + dest="createPortable", + default=False, + help="Creates a portable copy of NVDA (starting the new copy after installation). " + "Requires `--portable-path` to be specified. " +) +installGroup.add_argument( + "--create-portable-silent", + action="store_true", + dest="createPortableSilent", + default=False, + help="Creates a portable copy of NVDA silently (does not start the new copy after installation). " + "Note this will suppresses warnings when overwriting non-empty directories. " + "Requires --portable-path to be specified. " +) parser.add_argument('--portable-path',dest='portablePath',default=None,type=str,help="The path where a portable copy will be created") parser.add_argument('--launcher',action="store_true",dest='launcher',default=False,help="Started from the launcher") parser.add_argument('--enable-start-on-logon',metavar="True|False",type=stringToBool,dest='enableStartOnLogon',default=None, diff --git a/user_docs/en/changes.md b/user_docs/en/changes.md index 45882db80d6..de94db5eb31 100644 --- a/user_docs/en/changes.md +++ b/user_docs/en/changes.md @@ -36,7 +36,7 @@ Unicode CLDR has been updated. * NVDA will now report figures with no accessible children, but with a label or description. (#14514) * When reading by line in browse mode, "caption" is no longer reported on each line of a long figure or table caption. (#14874) * In the Python console, the last unexecuted command will no longer be lost when moving in the input history. (#16653, @CyrilleB79) -* By default, a new folder will be created when making a portable copy. (#16684) +* By default, a new folder will be created when making a portable copy. Warnings have been added when writing to a non-empty directory. (#16684) ### Bug Fixes * Windows 11 fixes: diff --git a/user_docs/en/userGuide.md b/user_docs/en/userGuide.md index 3f299bed4fb..c24119bb26b 100644 --- a/user_docs/en/userGuide.md +++ b/user_docs/en/userGuide.md @@ -5083,7 +5083,7 @@ Following are the command line options for NVDA: |None |`--enable-start-on-logon=True|False` |When installing, enable NVDA's [Use NVDA during Windows sign-in](#StartAtWindowsLogon)| |None |`--copy-portable-config` |When installing, copy the portable configuration from the provided path (`--config-path`, `-c`) to the current user account| |None |`--create-portable` |Creates a portable copy of NVDA (starting the newly created copy). Requires `--portable-path` to be specified| -|None |`--create-portable-silent` |Creates a portable copy of NVDA (does not start the newly installed copy). Requires `--portable-path` to be specified| +|None |`--create-portable-silent` |Creates a portable copy of NVDA (does not start the newly installed copy). Note this will suppress warnings when writing to non-empty directories. Requires `--portable-path` to be specified| |None |`--portable-path=PORTABLEPATH` |The path where a portable copy will be created| ### System Wide Parameters {#SystemWideParameters}