address review comments

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.

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.

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,

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:

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}