[Scummvm-git-logs] scummvm master -> c1b4dfad77113012dfea4e9688613c75012e3312

sev- noreply at scummvm.org
Sat Jun 15 00:07:42 UTC 2024


This automated email contains information about 3 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
b930232e6d COMMON: Punyencode path if it's already encoded.
f675e643b8 COMMON: Add helper function to parse command-line provided paths
c1b4dfad77 BASE: Rework path parsing on command line


Commit: b930232e6dfa93c1fc0bdc282b2154078918c311
    https://github.com/scummvm/scummvm/commit/b930232e6dfa93c1fc0bdc282b2154078918c311
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-06-15T02:07:38+02:00

Commit Message:
COMMON: Punyencode path if it's already encoded.

This allows to have puny-encoded folders on disk and use them without
seeing them decoded by ScummVM like if they were encoded by us.

Changed paths:
    common/punycode.cpp
    test/common/path.h


diff --git a/common/punycode.cpp b/common/punycode.cpp
index 8ff95eb36eb..9791858e802 100644
--- a/common/punycode.cpp
+++ b/common/punycode.cpp
@@ -151,6 +151,10 @@ String punycode_encode(const U32String &src) {
 		// endings
 		if (src[h - 1] == ' ' || src[h - 1] == '.')
 			return dst + '-';
+		if (srclen > 4 && src[0] == 'x' && src[1] == 'n' &&
+			src[2] == '-' && src[3] == '-')
+			return dst + '-';
+
 
 		return src;
 	}
diff --git a/test/common/path.h b/test/common/path.h
index e2c44421602..c14cfcd4f2e 100644
--- a/test/common/path.h
+++ b/test/common/path.h
@@ -405,6 +405,12 @@ class PathTestSuite : public CxxTest::TestSuite
 		Common::Path p5 = p4.punycodeEncode();
 		TS_ASSERT_EQUALS(p5.toString('/'), "parent/dir/xn--Sound Manager 3.1  SoundLib-lba84k/Sound");
 
+		Common::Path p6 = p3.punycodeEncode();
+		TS_ASSERT_EQUALS(p6.toString('/'), "parent/dir/xn--xn--Sound Manager 3.1  SoundLib-lba84k-/Sound");
+
+		Common::Path p7 = p6.punycodeDecode();
+		TS_ASSERT_EQUALS(p7.toString('/'), "parent/dir/xn--Sound Manager 3.1  SoundLib-lba84k/Sound");
+
 		typedef Common::HashMap<Common::Path, bool,
 				Common::Path::IgnoreCaseAndMac_Hash, Common::Path::IgnoreCaseAndMac_EqualTo> TestPathMap;
 		TestPathMap map;


Commit: f675e643b89b066acc94565e188ba4c68bbb2512
    https://github.com/scummvm/scummvm/commit/f675e643b89b066acc94565e188ba4c68bbb2512
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-06-15T02:07:38+02:00

Commit Message:
COMMON: Add helper function to parse command-line provided paths

Changed paths:
    common/path.cpp
    common/path.h


diff --git a/common/path.cpp b/common/path.cpp
index 8638f0c8772..16561795c09 100644
--- a/common/path.cpp
+++ b/common/path.cpp
@@ -1119,4 +1119,23 @@ Path Path::fromConfig(const String &value) {
 	return Path(value, '/').punycodeDecode();
 }
 
+Path Path::fromCommandLine(const String &value) {
+	if (value.empty()) {
+		return Path();
+	}
+
+	// WIN32 accepts / and \ as separators
+#if defined(WIN32)
+	if (strchr(value.c_str(), Path::kNativeSeparator)) {
+		String value_ = value;
+		// User may have mixed \ and /
+		// As / is forbidden in paths under WIN32, this will never be a collision
+		value_.replace(Path::kNativeSeparator, '/');
+		return Path(value_, '/');
+	}
+#endif
+	// Unlike for options the paths provided by the user are not punyencoded
+	return Path(value, '/');
+}
+
 } // End of namespace Common
diff --git a/common/path.h b/common/path.h
index 4ab1231f6d2..1b77d76c162 100644
--- a/common/path.h
+++ b/common/path.h
@@ -557,9 +557,14 @@ public:
 	}
 
 	/**
-	 * Used by ConfigManager to parse a configuration value in a bacwards compatible way
+	 * Used by ConfigManager to parse a configuration value in a backwards compatible way
 	 */
 	static Path fromConfig(const String &value);
+
+	/**
+	 * Creates a path from a string given by the user
+	 */
+	static Path fromCommandLine(const String &value);
 };
 
 /** @} */


Commit: c1b4dfad77113012dfea4e9688613c75012e3312
    https://github.com/scummvm/scummvm/commit/c1b4dfad77113012dfea4e9688613c75012e3312
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-06-15T02:07:38+02:00

Commit Message:
BASE: Rework path parsing on command line

Make a macro to unify processing.
Use the new Path helper to handle WIN32 and save to settings using the
toConfig helper.
This makes sure the path is properly encoded and will not be improperly
decoded later.

Changed paths:
    base/commandLine.cpp


diff --git a/base/commandLine.cpp b/base/commandLine.cpp
index a546b59cad3..96a745f2c07 100644
--- a/base/commandLine.cpp
+++ b/base/commandLine.cpp
@@ -486,9 +486,7 @@ static Common::String createTemporaryTarget(const Common::String &engineId, cons
  * Note 2: The method will work with paths that are symbolic links to folders (isDirectory() returns true),
  * but for symbolic links to files it will not deduce a valid folder path and will just return false.
  *
- * @param settings A reference to the settings map used by parseCommandLine()
- * @param optionKeyStr The key string for updating the value for this path option on the settings map, if needed
- * @param path The path node that was created from the command line value for this path option
+ * @param node The filesystem node created from the command line value and modified if needed to get a folder from a file
  * @param ensureWriteable A boolean flag that is set true if the path should be writeable, false otherwise
  * @param ensureReadable A boolean flag that is set true if the path should be readable, false otherwise
  * @param acceptFile true if the command line option allows (tolerates) a file path to deduce the folder path from
@@ -496,22 +494,19 @@ static Common::String createTemporaryTarget(const Common::String &engineId, cons
  * was deduced from it, and the path (original or deduced respectively) meets the specified
  * readability / writeability requirements.
  */
-bool ensureAccessibleDirectoryForPathOption(Common::StringMap &settings,
-                                            const Common::String optionKeyStr,
-                                            const Common::FSNode &path,
+bool ensureAccessibleDirectoryForPathOption(Common::FSNode &node,
                                             bool ensureWriteable,
                                             bool ensureReadable,
                                             bool acceptFile) {
-	if (path.isDirectory()) {
-		if ((!ensureWriteable || path.isWritable())
-		    && (!ensureReadable || path.isReadable())
+	if (node.isDirectory()) {
+		if ((!ensureWriteable || node.isWritable())
+		    && (!ensureReadable || node.isReadable())
 		    && (ensureWriteable || ensureReadable)) {
 			return true;
 		}
-	} else if (acceptFile
-		    && ensureAccessibleDirectoryForPathOption(settings, optionKeyStr, path.getParent(), ensureWriteable, ensureReadable, false)) {
-			settings[optionKeyStr] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
-			return true;
+	} else if (acceptFile) {
+		node = node.getParent();
+		return ensureAccessibleDirectoryForPathOption(node, ensureWriteable, ensureReadable, false);
 	}
 	return false;
 }
@@ -564,6 +559,22 @@ bool ensureAccessibleDirectoryForPathOption(Common::StringMap &settings,
 		const char *option = boolValue ? "true" : "false"; \
 		settings[longCmd] = option;
 
+#define DO_OPTION_PATH(shortCmd, longCmd) \
+	DO_OPTION(shortCmd, longCmd) \
+	Common::FSNode node(Common::Path::fromCommandLine(option)); \
+	if (!node.exists()) { \
+		usage("Non-existent path '%s' for option %s%c%s", option, \
+				isLongCmd ? "--" : "-", \
+				isLongCmd ? longCmd[0] : shortCmd, \
+				isLongCmd ? longCmd + 1 : ""); \
+	} else if (!ensureAccessibleDirectoryForPathOption(node, false, true, true)) { \
+		usage("Non-readable path '%s' for option %s%c%s", option, \
+				isLongCmd ? "--" : "-", \
+				isLongCmd ? longCmd[0] : shortCmd, \
+				isLongCmd ? longCmd + 1 : ""); \
+	} \
+	settings[longCmd] = node.getPath().toConfig();
+
 // Use this for options which never have a value, i.e. for 'commands', like "--help".
 #define DO_COMMAND(shortCmd, longCmd) \
 	if (isLongCmd ? (!strcmp(s + 2, longCmd)) : (tolower(s[1]) == shortCmd)) { \
@@ -579,6 +590,7 @@ bool ensureAccessibleDirectoryForPathOption(Common::StringMap &settings,
 #define DO_LONG_OPTION(longCmd)         DO_OPTION(0, longCmd)
 #define DO_LONG_OPTION_INT(longCmd)     DO_OPTION_INT(0, longCmd)
 #define DO_LONG_OPTION_BOOL(longCmd)    DO_OPTION_BOOL(0, longCmd)
+#define DO_LONG_OPTION_PATH(longCmd)    DO_OPTION_PATH(0, longCmd)
 #define DO_LONG_COMMAND(longCmd)        DO_COMMAND(0, longCmd)
 
 // End an option handler
@@ -712,13 +724,7 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
 			DO_OPTION('l', "logfile")
 			END_OPTION
 
-			DO_LONG_OPTION("screenshotpath")
-				Common::FSNode path(option);
-				if (!path.exists()) {
-					usage("Non-existent screenshot path '%s'", option);
-				} else if (!ensureAccessibleDirectoryForPathOption(settings, "screenshotpath", path, true, false, true)) {
-					usage("Non-writable screenshot path '%s'", option);
-				}
+			DO_LONG_OPTION_PATH("screenshotpath")
 			END_OPTION
 #endif
 
@@ -812,13 +818,7 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
 			DO_OPTION_BOOL('n', "subtitles")
 			END_OPTION
 
-			DO_OPTION('p', "path")
-				Common::FSNode path(option);
-				if (!path.exists()) {
-					usage("Non-existent game path '%s'", option);
-				} else if (!ensureAccessibleDirectoryForPathOption(settings, "path", path, false, true, true)) {
-					usage("Non-readable game path '%s'", option);
-				}
+			DO_OPTION_PATH('p', "path")
 			END_OPTION
 
 			DO_OPTION('q', "language")
@@ -856,7 +856,7 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
 			END_OPTION
 
 			DO_LONG_OPTION("soundfont")
-				Common::FSNode path(option);
+				Common::FSNode path(Common::Path::fromConfig(option));
 				if (!path.exists()) {
 					usage("Non-existent soundfont path '%s'", option);
 				} else if (!path.isReadable()) {
@@ -908,31 +908,13 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
 			DO_LONG_OPTION_BOOL("show-fps")
 			END_OPTION
 
-			DO_LONG_OPTION("savepath")
-				Common::FSNode path(option);
-				if (!path.exists()) {
-					usage("Non-existent saved games path '%s'", option);
-				} else if (!ensureAccessibleDirectoryForPathOption(settings, "savepath", path, true, true, true)) {
-					usage("Non-writable saved games path '%s'", option);
-				}
+			DO_LONG_OPTION_PATH("savepath")
 			END_OPTION
 
-			DO_LONG_OPTION("extrapath")
-				Common::FSNode path(option);
-				if (!path.exists()) {
-					usage("Non-existent extra path '%s'", option);
-				} else if (!ensureAccessibleDirectoryForPathOption(settings, "extrapath", path, false, true, true)) {
-					usage("Non-readable extra path '%s'", option);
-				}
+			DO_LONG_OPTION_PATH("extrapath")
 			END_OPTION
 
-			DO_LONG_OPTION("iconspath")
-				Common::FSNode path(option);
-				if (!path.exists()) {
-					usage("Non-existent icons path '%s'", option);
-				} else if (!ensureAccessibleDirectoryForPathOption(settings, "iconspath", path, true, true, true)) {
-					usage("Non-readable icons path '%s'", option);
-				}
+			DO_LONG_OPTION_PATH("iconspath")
 			END_OPTION
 
 			DO_LONG_OPTION("md5-path")
@@ -967,13 +949,7 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
 			DO_LONG_OPTION_BOOL("exit")
 			END_OPTION
 
-			DO_LONG_OPTION("themepath")
-				Common::FSNode path(option);
-				if (!path.exists()) {
-					usage("Non-existent theme path '%s'", option);
-				} else if (!ensureAccessibleDirectoryForPathOption(settings, "themepath", path, false, true, true)) {
-					usage("Non-readable theme path '%s'", option);
-				}
+			DO_LONG_OPTION_PATH("themepath")
 			END_OPTION
 
 			DO_LONG_COMMAND("list-themes")
@@ -2011,7 +1987,7 @@ bool processSettings(Common::String &command, Common::StringMap &settings, Commo
 			// From an UX point of view, however, it might get confusing.
 			// Consider removing this if consensus says otherwise.
 		} else {
-			Common::Path path(settings["path"], Common::Path::kNativeSeparator);
+			Common::Path path(Common::Path::fromConfig(settings["path"]));
 			command = detectGames(path, gameOption.engineId, gameOption.gameId, resursive);
 			if (command.empty()) {
 				err = Common::kNoGameDataFoundError;
@@ -2019,11 +1995,11 @@ bool processSettings(Common::String &command, Common::StringMap &settings, Commo
 			}
 		}
 	} else if (command == "detect") {
-		Common::Path path(settings["path"], Common::Path::kNativeSeparator);
+		Common::Path path(Common::Path::fromConfig(settings["path"]));
 		detectGames(path, gameOption.engineId, gameOption.gameId, settings["recursive"] == "true");
 		return cmdDoExit;
 	} else if (command == "add") {
-		Common::Path path(settings["path"], Common::Path::kNativeSeparator);
+		Common::Path path(Common::Path::fromConfig(settings["path"]));
 		addGames(path, gameOption.engineId, gameOption.gameId, settings["recursive"] == "true");
 		return cmdDoExit;
 	} else if (command == "md5" || command == "md5mac") {




More information about the Scummvm-git-logs mailing list