Currently we have several UNICODE FIXMEs in the code where we pass an utf8
encoded filename to an fstream and hope that it will work. This is wrong,
because the encoding of filenames depends on the locale on linux. I don't
know what it is on windows, I never use non-ascii filenames.

My plan to solve this problem is this:

- Split the existing FileName class into a general part that can be used for
any filename, and a part that is specialized for files that appear in
documents. This is done by the attached patch.
- Convert both file name classes to docstring
- Store all filenames not as string or docstring, but in the FileName class
and convert all functions that take a filename argument to the FileName
class. That will eliminate a lot of assertions on absolute paths.
- Add a method to the FileName class that returns the name as a std::string
in the right encoding for the file system.

This will generate rather large, but mechanical patches, but it will
basically not change the code, and it will guarantee that no file name is
stored in the wrong encoding.
Later we could for example change the internal storage of the FileName class
to use boost::filesystem if that has benefits.

If this is OK I will do this step by step during the next days.


Georg
Index: src/insets/insetbibtex.C
===================================================================
--- src/insets/insetbibtex.C	(Revision 15949)
+++ src/insets/insetbibtex.C	(Arbeitskopie)
@@ -44,7 +44,7 @@ using support::ascii_lowercase;
 using support::changeExtension;
 using support::contains;
 using support::copy;
-using support::FileName;
+using support::DocFileName;
 using support::findtexfile;
 using support::isFileReadable;
 using support::latex_path;
@@ -175,7 +175,7 @@ int InsetBibtex::latex(Buffer const & bu
 		    isFileReadable(in_file)) {
 
 			// mangledFilename() needs the extension
-			database = removeExtension(FileName(in_file).mangledFilename());
+			database = removeExtension(DocFileName(in_file).mangledFilename());
 			string const out_file = makeAbsPath(database + ".bib",
 					buffer.getMasterBuffer()->temppath());
 
@@ -231,7 +231,7 @@ int InsetBibtex::latex(Buffer const & bu
 		    isFileReadable(in_file)) {
 			// use new style name
 			base = removeExtension(
-					FileName(in_file).mangledFilename());
+					DocFileName(in_file).mangledFilename());
 			string const out_file = makeAbsPath(base + ".bst",
 					buffer.getMasterBuffer()->temppath());
 			bool const success = copy(in_file, out_file);
Index: src/insets/insetgraphics.C
===================================================================
--- src/insets/insetgraphics.C	(Revision 15949)
+++ src/insets/insetgraphics.C	(Arbeitskopie)
@@ -95,7 +95,7 @@ using support::bformat;
 using support::changeExtension;
 using support::compare_timestamps;
 using support::contains;
-using support::FileName;
+using support::DocFileName;
 using support::float_equal;
 using support::getExtension;
 using support::isFileReadable;
@@ -483,7 +483,7 @@ copyFileIfNeeded(string const & file_in,
 
 
 std::pair<CopyStatus, string> const
-copyToDirIfNeeded(FileName const & file, string const & dir)
+copyToDirIfNeeded(DocFileName const & file, string const & dir)
 {
 	using support::rtrim;
 
Index: src/insets/insetexternal.h
===================================================================
--- src/insets/insetexternal.h	(Revision 15949)
+++ src/insets/insetexternal.h	(Arbeitskopie)
@@ -79,7 +79,7 @@ public:
 	std::string const & templatename() const { return templatename_; }
 
 	/// The external file.
-	support::FileName filename;
+	support::DocFileName filename;
 	/// How the inset is to be displayed by LyX.
 	external::DisplayType display;
 	/// The scale of the displayed graphic (if shown).
Index: src/insets/insetgraphicsParams.h
===================================================================
--- src/insets/insetgraphicsParams.h	(Revision 15949)
+++ src/insets/insetgraphicsParams.h	(Arbeitskopie)
@@ -30,7 +30,7 @@ class InsetGraphicsParams
 {
 public:
 	/// Image filename.
-	support::FileName filename;
+	support::DocFileName filename;
 	/// Scaling the Screen inside Lyx
 	unsigned int lyxscale;
 	/// How to display the image inside LyX
Index: src/insets/insetinclude.C
===================================================================
--- src/insets/insetinclude.C	(Revision 15949)
+++ src/insets/insetinclude.C	(Arbeitskopie)
@@ -59,7 +59,7 @@ using support::bformat;
 using support::changeExtension;
 using support::contains;
 using support::copy;
-using support::FileName;
+using support::DocFileName;
 using support::getFileContents;
 using support::isFileReadable;
 using support::isLyXFilename;
@@ -390,7 +390,7 @@ int InsetInclude::latex(Buffer const & b
 
 	// write it to a file (so far the complete file)
 	string const exportfile = changeExtension(incfile, ".tex");
-	string const mangled = FileName(changeExtension(included_file,
+	string const mangled = DocFileName(changeExtension(included_file,
 							".tex")).mangledFilename();
 	string const writefile = makeAbsPath(mangled, m_buffer->temppath());
 
@@ -524,7 +524,7 @@ int InsetInclude::docbook(Buffer const &
 	if (loadIfNeeded(buffer, params_)) {
 		Buffer * tmp = theBufferList().getBuffer(included_file);
 
-		string const mangled = FileName(writefile).mangledFilename();
+		string const mangled = DocFileName(writefile).mangledFilename();
 		writefile = makeAbsPath(mangled,
 					buffer.getMasterBuffer()->temppath());
 		if (!runparams.nice)
@@ -568,7 +568,7 @@ void InsetInclude::validate(LaTeXFeature
 		writefile = included_file;
 
 	if (!features.runparams().nice && !isVerbatim(params_)) {
-		incfile = FileName(writefile).mangledFilename();
+		incfile = DocFileName(writefile).mangledFilename();
 		writefile = makeAbsPath(incfile,
 					buffer.getMasterBuffer()->temppath());
 	}
Index: src/support/filename.C
===================================================================
--- src/support/filename.C	(Revision 15949)
+++ src/support/filename.C	(Arbeitskopie)
@@ -31,18 +31,46 @@ namespace support {
 
 
 FileName::FileName()
-	: save_abs_path_(true)
 {}
 
 
-FileName::FileName(string const & abs_filename, bool save_abs)
-	: name_(abs_filename), save_abs_path_(save_abs), zipped_valid_(false)
+FileName::FileName(string const & abs_filename)
+	: name_(abs_filename)
 {
 	BOOST_ASSERT(absolutePath(name_));
 }
 
 
-void FileName::set(string const & name, string const & buffer_path)
+void FileName::set(string const & abs_filename)
+{
+	name_ = abs_filename;
+	BOOST_ASSERT(absolutePath(name_));
+}
+
+
+bool operator==(FileName const & lhs, FileName const & rhs)
+{
+	return lhs.absFilename() == rhs.absFilename();
+}
+
+
+bool operator!=(FileName const & lhs, FileName const & rhs)
+{
+	return lhs.absFilename() != rhs.absFilename();
+}
+
+
+DocFileName::DocFileName()
+	: save_abs_path_(true)
+{}
+
+
+DocFileName::DocFileName(string const & abs_filename, bool save_abs)
+	: FileName(abs_filename), save_abs_path_(save_abs), zipped_valid_(false)
+{}
+
+
+void DocFileName::set(string const & name, string const & buffer_path)
 {
 	save_abs_path_ = absolutePath(name);
 	name_ = save_abs_path_ ? name : makeAbsPath(name, buffer_path);
@@ -50,28 +78,28 @@ void FileName::set(string const & name, 
 }
 
 
-void FileName::erase()
+void DocFileName::erase()
 {
 	name_.erase();
 	zipped_valid_ = false;
 }
 
 
-string const FileName::relFilename(string const & path) const
+string const DocFileName::relFilename(string const & path) const
 {
 	return makeRelPath(name_, path);
 }
 
 
-string const FileName::outputFilename(string const & path) const
+string const DocFileName::outputFilename(string const & path) const
 {
 	return save_abs_path_ ? name_ : makeRelPath(name_, path);
 }
 
 
-string const FileName::mangledFilename(std::string const & dir) const
+string const DocFileName::mangledFilename(std::string const & dir) const
 {
-	// We need to make sure that every FileName instance for a given
+	// We need to make sure that every DocFileName instance for a given
 	// filename returns the same mangled name.
 	typedef map<string, string> MangledMap;
 	static MangledMap mangledNames;
@@ -125,7 +153,7 @@ string const FileName::mangledFilename(s
 }
 
 
-bool FileName::isZipped() const
+bool DocFileName::isZipped() const
 {
 	if (!zipped_valid_) {
 		zipped_ = zippedFile(name_);
@@ -135,20 +163,20 @@ bool FileName::isZipped() const
 }
 
 
-string const FileName::unzippedFilename() const
+string const DocFileName::unzippedFilename() const
 {
 	return unzippedFileName(name_);
 }
 
 
-bool operator==(FileName const & lhs, FileName const & rhs)
+bool operator==(DocFileName const & lhs, DocFileName const & rhs)
 {
 	return lhs.absFilename() == rhs.absFilename() &&
 		lhs.saveAbsPath() == rhs.saveAbsPath();
 }
 
 
-bool operator!=(FileName const & lhs, FileName const & rhs)
+bool operator!=(DocFileName const & lhs, DocFileName const & rhs)
 {
 	return !(lhs == rhs);
 }
Index: src/support/filename.h
===================================================================
--- src/support/filename.h	(Revision 15949)
+++ src/support/filename.h	(Arbeitskopie)
@@ -19,13 +19,38 @@ namespace lyx {
 namespace support {
 
 
+/**
+ * Class for storing file names
+ */
 class FileName {
 public:
 	FileName();
 	/** \param abs_filename the file in question. Must have an absolute path.
+	 */
+	FileName(std::string const & abs_filename);
+	/** \param filename the file in question. Must be absolute.
+	 */
+	virtual ~FileName() {}
+	virtual void set(std::string const & filename);
+	virtual void erase() { name_.erase(); }
+	bool empty() const { return name_.empty(); }
+	std::string const absFilename() const { return name_; }
+protected:
+	std::string name_;
+};
+
+
+bool operator==(FileName const &, FileName const &);
+bool operator!=(FileName const &, FileName const &);
+
+
+class DocFileName : public FileName {
+public:
+	DocFileName();
+	/** \param abs_filename the file in question. Must have an absolute path.
 	 *  \param save_abs_path how is the file to be output to file?
 	 */
-	FileName(std::string const & abs_filename, bool save_abs_path = true);
+	DocFileName(std::string const & abs_filename, bool save_abs_path = true);
 
 	/** \param filename the file in question. May have either a relative
 	 *  or an absolute path.
@@ -35,10 +60,8 @@ public:
 	void set(std::string const & filename, std::string const & buffer_path);
 
 	void erase();
-	bool empty() const { return name_.empty(); }
 
 	bool saveAbsPath() const { return save_abs_path_; }
-	std::string const absFilename() const { return name_; }
 	/// \param buffer_path if empty, uses `pwd`
 	std::string const relFilename(std::string const & buffer_path = std::string()) const;
 	/// \param buf_path if empty, uses `pwd`
@@ -73,7 +96,6 @@ public:
 	std::string const unzippedFilename() const;
 
 private:
-	std::string name_;
 	bool save_abs_path_;
 	/// Cache for isZipped() because zippedFile() is expensive
 	mutable bool zipped_;
@@ -82,8 +104,8 @@ private:
 };
 
 
-bool operator==(FileName const &, FileName const &);
-bool operator!=(FileName const &, FileName const &);
+bool operator==(DocFileName const &, DocFileName const &);
+bool operator!=(DocFileName const &, DocFileName const &);
 
 
 } // namespace support

Reply via email to