Am Freitag, 8. September 2006 15:18 schrieb Lars Gullik Bjønnes:
> Helge Hafting <[EMAIL PROTECTED]> writes:
> 
> | if os.system(r'python -tt '/usr/local/share/lyx/scripts/ly2png.py' ' +
> | '"' + infile + '"' + ' ' + '"' + outfile + '"' + '') != 0:
> |   unlinkNoThrow(outfile)
> |   sys.exit(1)
> 
> The quoting looks wrong. I guess it would work if the quotes around
> ly2png.py was removed or changed to "".

Right. I overlooked the case where a converter in $$s/scripts is used when 
I fixed bug 2637.

The attached pair of patches for 1.4 and 1.5 fixes this problem by 
introducing proper python quoting in quoteName. Up to now we used the 
shell qouting also for python. Helge, please confirm whether it works for 
you.


Georg
Index: src/graphics/GraphicsConverter.C
===================================================================
--- src/graphics/GraphicsConverter.C	(Revision 14948)
+++ src/graphics/GraphicsConverter.C	(Arbeitskopie)
@@ -38,6 +38,7 @@ using support::LibScriptSearch;
 using support::OnlyPath;
 using support::OnlyFilename;
 using support::QuoteName;
+using support::quote_python;
 using support::subst;
 using support::tempName;
 using support::unlink;
@@ -325,9 +326,8 @@ void build_script(string const & from_fi
 	// in python, but the converters might be shell scripts and have more
 	// troubles with it.
 	string outfile = ChangeExtension(to_base, GetExtension(from_file));
-	script << "infile = '"
-	       << subst(subst(from_file, "\\", "\\\\"), "'", "\\'") << "'\n"
-	          "outfile = " << QuoteName(outfile) << "\n"
+	script << "infile = " << QuoteName(from_file, quote_python) << "\n"
+	          "outfile = " << QuoteName(outfile, quote_python) << "\n"
 	          "shutil.copy(infile, outfile)\n";
 
 	if (edgepath.empty()) {
@@ -335,13 +335,18 @@ void build_script(string const & from_fi
 		// converter path from from_format to to_format, so we use
 		// the default converter.
 		script << "infile = outfile\n"
-		       << "outfile = " << QuoteName(to_file) << '\n';
+		       << "outfile = " << QuoteName(to_file, quote_python)
+		       << '\n';
 
 		ostringstream os;
-		os << "python \""
-		   << LibFileSearch("scripts", "convertDefault.py") << "\" ";
+		os << "python "
+		   << LibScriptSearch("$$s/scripts/convertDefault.py",
+		                      quote_python) << ' ';
 		if (!from_format.empty())
 			os << from_format << ':';
+		// The extra " quotes around infile and outfile are needed
+		// because the filename may contain spaces and it is used
+		// as argument of os.system().
 		os << "' + '\"' + infile + '\"' + ' "
 		   << to_format << ":' + '\"' + outfile + '\"' + '";
 		string const command = os.str();
@@ -371,21 +376,23 @@ void build_script(string const & from_fi
 		outfile = ChangeExtension(to_base, conv.To->extension());
 
 		// Store these names in the python script
-		script << "infile = "      << QuoteName(infile) << '\n'
-		       << "infile_base = " << QuoteName(infile_base) << '\n'
-		       << "outfile = "     << QuoteName(outfile) << '\n';
+		script << "infile = "      << QuoteName(infile, quote_python) << "\n"
+		          "infile_base = " << QuoteName(infile_base, quote_python) << "\n"
+		          "outfile = "     << QuoteName(outfile, quote_python) << '\n';
 
+		// See comment about extra " quotes above (although that
+		// applies only for the first loop run here).
 		string command = conv.command;
 		command = subst(command, token_from, "' + '\"' + infile + '\"' + '");
 		command = subst(command, token_base, "' + '\"' + infile_base + '\"' + '");
 		command = subst(command, token_to,   "' + '\"' + outfile + '\"' + '");
-		command = LibScriptSearch(command);
+		command = LibScriptSearch(command, quote_python);
 
 		build_conversion_command(command, script);
 	}
 
 	// Move the final outfile to to_file
-	script << move_file("outfile", QuoteName(to_file));
+	script << move_file("outfile", QuoteName(to_file, quote_python));
 	lyxerr[Debug::GRAPHICS] << "ready!" << endl;
 }
 
Index: src/support/filetools.C
===================================================================
--- src/support/filetools.C	(Revision 14948)
+++ src/support/filetools.C	(Arbeitskopie)
@@ -133,11 +133,26 @@ string const MakeLatexName(string const 
 }
 
 
-string const QuoteName(string const & name)
+string const QuoteName(string const & name, quote_style style)
 {
-	return (os::shell() == os::UNIX) ?
-		'\'' + name + '\'':
-		'"' + name + '"';
+	switch(style) {
+	case quote_shell:
+		// This does not work for filenames containing " (windows)
+		// or ' (all other OSes). This can't be changed easily, since
+		// we would need to adapt the command line parser in
+		// Forkedcall::generateChild. Therefore we don't pass user
+		// filenames to child processes if possible. We store them in
+		// a python script instead, where we don't have these
+		// limitations.
+		return (os::shell() == os::UNIX) ?
+			'\'' + name + '\'':
+			'"' + name + '"';
+	case quote_python:
+		return "\"" + subst(subst(name, "\\", "\\\\"), "\"", "\\\"")
+		     + "\"";
+	}
+	// shut up stupid compiler
+	return string();
 }
 
 
@@ -319,7 +334,7 @@ string const i18nLibFileSearch(string co
 }
 
 
-string const LibScriptSearch(string const & command_in)
+string const LibScriptSearch(string const & command_in, quote_style style)
 {
 	static string const token_scriptpath = "$$s/";
 
@@ -345,7 +360,7 @@ string const LibScriptSearch(string cons
 	} else {
 		// Replace "$$s/foo/some_script" with "<path to>/some_script".
 		string::size_type const size_replace = size_script + 4;
-		command.replace(pos1, size_replace, QuoteName(script));
+		command.replace(pos1, size_replace, QuoteName(script, style));
 	}
 
 	return command;
Index: src/support/filetools.h
===================================================================
--- src/support/filetools.h	(Revision 14948)
+++ src/support/filetools.h	(Arbeitskopie)
@@ -94,13 +94,28 @@ std::string const
 i18nLibFileSearch(std::string const & dir, std::string const & name,
 		  std::string const & ext = std::string());
 
+/// How to quote a filename
+enum quote_style {
+	/** Quote for the (OS dependant) shell. This is needed for command
+	    line arguments of subprocesses. */
+	quote_shell,
+	/** Quote for python. Use this if you want to store a filename in a
+	    python script. Example: \code
+	    os << "infile = " << QuoteName(filename) << '\\n';
+	    \endcode This uses double quotes, so that you can also use this
+	    to quote filenames as part of a string if the string is quoted
+	    with single quotes. */
+	quote_python
+};
+
 /** Takes a command such as "python $$s/scripts/convertDefault.py file.in file.out"
  *  and replaces "$$s/" with the path to the LyX support directory containing
  *  this script. If the script is not found, "$$s/" is removed. Executing the
  *  command will still fail, but the error message will make some sort of
  *  sense ;-)
  */
-std::string const LibScriptSearch(std::string const & command);
+std::string const LibScriptSearch(std::string const & command,
+		quote_style style = quote_shell);
 
 enum latex_path_extension {
 	PROTECT_EXTENSION,
@@ -138,8 +153,9 @@ std::string const latex_path(std::string
 /// Substitutes active latex characters with underscores in filename
 std::string const MakeLatexName(std::string const & file);
 
-/// Put the name in quotes suitable for the current shell
-std::string const QuoteName(std::string const & file);
+/** Put the name in quotes suitable for the current shell or python,
+    depending on \p style. */
+std::string const QuoteName(std::string const & file, quote_style style = quote_shell);
 
 /// Add a filename to a path. Any path from filename is stripped first.
 std::string const AddName(std::string const & path, std::string const & fname);
Index: src/graphics/GraphicsConverter.C
===================================================================
--- src/graphics/GraphicsConverter.C	(Revision 14948)
+++ src/graphics/GraphicsConverter.C	(Arbeitskopie)
@@ -39,6 +39,7 @@ using support::libScriptSearch;
 using support::onlyPath;
 using support::onlyFilename;
 using support::quoteName;
+using support::quote_python;
 using support::subst;
 using support::tempName;
 using support::unlink;
@@ -327,9 +328,8 @@ void build_script(string const & from_fi
 	// in python, but the converters might be shell scripts and have more
 	// troubles with it.
 	string outfile = changeExtension(to_base, getExtension(from_file));
-	script << "infile = '"
-	       << subst(subst(from_file, "\\", "\\\\"), "'", "\\'") << "'\n"
-	          "outfile = " << quoteName(outfile) << "\n"
+	script << "infile = " << quoteName(from_file, quote_python) << "\n"
+	          "outfile = " << quoteName(outfile, quote_python) << "\n"
 	          "shutil.copy(infile, outfile)\n";
 
 	if (edgepath.empty()) {
@@ -337,13 +337,18 @@ void build_script(string const & from_fi
 		// converter path from from_format to to_format, so we use
 		// the default converter.
 		script << "infile = outfile\n"
-		       << "outfile = " << quoteName(to_file) << '\n';
+		       << "outfile = " << quoteName(to_file, quote_python)
+		       << '\n';
 
 		ostringstream os;
-		os << support::os::python() << " \""
-		   << libFileSearch("scripts", "convertDefault.py") << "\" ";
+		os << support::os::python() << ' '
+		   << libScriptSearch("$$s/scripts/convertDefault.py",
+		                      quote_python) << ' ';
 		if (!from_format.empty())
 			os << from_format << ':';
+		// The extra " quotes around infile and outfile are needed
+		// because the filename may contain spaces and it is used
+		// as argument of os.system().
 		os << "' + '\"' + infile + '\"' + ' "
 		   << to_format << ":' + '\"' + outfile + '\"' + '";
 		string const command = os.str();
@@ -373,21 +378,23 @@ void build_script(string const & from_fi
 		outfile = changeExtension(to_base, conv.To->extension());
 
 		// Store these names in the python script
-		script << "infile = "      << quoteName(infile) << '\n'
-		       << "infile_base = " << quoteName(infile_base) << '\n'
-		       << "outfile = "     << quoteName(outfile) << '\n';
+		script << "infile = "      << quoteName(infile, quote_python) << "\n"
+		          "infile_base = " << quoteName(infile_base, quote_python) << "\n"
+		          "outfile = "     << quoteName(outfile, quote_python) << '\n';
 
+		// See comment about extra " quotes above (although that
+		// applies only for the first loop run here).
 		string command = conv.command;
 		command = subst(command, token_from, "' + '\"' + infile + '\"' + '");
 		command = subst(command, token_base, "' + '\"' + infile_base + '\"' + '");
 		command = subst(command, token_to,   "' + '\"' + outfile + '\"' + '");
-		command = libScriptSearch(command);
+		command = libScriptSearch(command, quote_python);
 
 		build_conversion_command(command, script);
 	}
 
 	// Move the final outfile to to_file
-	script << move_file("outfile", quoteName(to_file));
+	script << move_file("outfile", quoteName(to_file, quote_python));
 	lyxerr[Debug::GRAPHICS] << "ready!" << endl;
 }
 
Index: src/support/filetools.C
===================================================================
--- src/support/filetools.C	(Revision 14948)
+++ src/support/filetools.C	(Arbeitskopie)
@@ -133,11 +133,26 @@ string const makeLatexName(string const 
 }
 
 
-string const quoteName(string const & name)
+string const quoteName(string const & name, quote_style style)
 {
-	return (os::shell() == os::UNIX) ?
-		'\'' + name + '\'':
-		'"' + name + '"';
+	switch(style) {
+	case quote_shell:
+		// This does not work for filenames containing " (windows)
+		// or ' (all other OSes). This can't be changed easily, since
+		// we would need to adapt the command line parser in
+		// Forkedcall::generateChild. Therefore we don't pass user
+		// filenames to child processes if possible. We store them in
+		// a python script instead, where we don't have these
+		// limitations.
+		return (os::shell() == os::UNIX) ?
+			'\'' + name + '\'':
+			'"' + name + '"';
+	case quote_python:
+		return "\"" + subst(subst(name, "\\", "\\\\"), "\"", "\\\"")
+		     + "\"";
+	}
+	// shut up stupid compiler
+	return string();
 }
 
 
@@ -313,7 +328,7 @@ string const i18nLibFileSearch(string co
 }
 
 
-string const libScriptSearch(string const & command_in)
+string const libScriptSearch(string const & command_in, quote_style style)
 {
 	static string const token_scriptpath = "$$s/";
 
@@ -339,7 +354,7 @@ string const libScriptSearch(string cons
 	} else {
 		// Replace "$$s/foo/some_script" with "<path to>/some_script".
 		string::size_type const size_replace = size_script + 4;
-		command.replace(pos1, size_replace, quoteName(script));
+		command.replace(pos1, size_replace, quoteName(script, style));
 	}
 
 	return command;
Index: src/support/filetools.h
===================================================================
--- src/support/filetools.h	(Revision 14948)
+++ src/support/filetools.h	(Arbeitskopie)
@@ -98,13 +98,28 @@ i18nLibFileSearch(std::string const & di
 		  std::string const & name,
 		  std::string const & ext = std::string());
 
+/// How to quote a filename
+enum quote_style {
+	/** Quote for the (OS dependant) shell. This is needed for command
+	    line arguments of subprocesses. */
+	quote_shell,
+	/** Quote for python. Use this if you want to store a filename in a
+	    python script. Example: \code
+	    os << "infile = " << quoteName(filename) << '\\n';
+	    \endcode This uses double quotes, so that you can also use this
+	    to quote filenames as part of a string if the string is quoted
+	    with single quotes. */
+	quote_python
+};
+
 /** Takes a command such as "python $$s/scripts/convertDefault.py file.in file.out"
  *  and replaces "$$s/" with the path to the LyX support directory containing
  *  this script. If the script is not found, "$$s/" is removed. Executing the
  *  command will still fail, but the error message will make some sort of
  *  sense ;-)
  */
-std::string const libScriptSearch(std::string const & command);
+std::string const libScriptSearch(std::string const & command,
+		quote_style style = quote_shell);
 
 enum latex_path_extension {
 	PROTECT_EXTENSION,
@@ -142,8 +157,9 @@ std::string const latex_path(std::string
 /// Substitutes active latex characters with underscores in filename
 std::string const makeLatexName(std::string const & file);
 
-/// Put the name in quotes suitable for the current shell
-std::string const quoteName(std::string const & file);
+/** Put the name in quotes suitable for the current shell or python,
+    depending on \p style. */
+std::string const quoteName(std::string const & file, quote_style style = quote_shell);
 
 /// Add a filename to a path. Any path from filename is stripped first.
 std::string const addName(std::string const & path, std::string const & fname);

Reply via email to