Embedding: adjust how InsetGraphics interacts with EmbeddedFiles, the ?th overhaul of the EmbeddedFile class :-(

git-svn-id: svn://svn.lyx.org/lyx/lyx-devel/trunk@22377 a592a061-630c-0410-9148-cb99ea01b6c8

src/EmbeddedFiles.cpp

@@ -43,28 +43,78 @@ using namespace lyx::support;
 
 namespace lyx {
 
+/** Dir name used for ".." in the bundled file.
+
+Under the lyx temp directory, content.lyx and its embedded files are usually
+saved as
+
+$temp/file.lyx
+$temp/figure1.png     for ./figure1.png)
+$temp/sub/figure2.png for ./sub/figure2.png)
+
+This works fine for embedded files that are in the current or deeper directory
+of the document directory, but not for files such as ../figures/figure.png.
+A unique name $upDirName is chosen to represent .. in such filenames so that
+'up' directories can be stored 'down' the directory tree:
+
+$temp/$upDirName/figures/figure.png     for ../figures/figure.png
+$temp/$upDirName/$upDirName/figure.png  for ../../figure.png
+
+This name has to be fixed because it is used in lyx bundled .zip file.
+
+Note that absolute files are not embeddable because there is no easy
+way to put them under $temp.
+*/
+const std::string upDirName = "LyX.Embed.Dir.Up";
+
 namespace Alert = frontend::Alert;
 
-
+EmbeddedFile::EmbeddedFile(string const & file, std::string const & buffer_path)
-EmbeddedFile::EmbeddedFile(string const & file, string const & inzip_name,
+	: DocFileName("", false), inzip_name_(""), embedded_(false), inset_list_()
-	bool embed, Inset const * inset)
-	: DocFileName(file, true), inzip_name_(inzip_name), embedded_(embed),
-		inset_list_()
 {
-	if (inset != NULL)
+	set(file, buffer_path);
-		inset_list_.push_back(inset);
+}
+
+
+void EmbeddedFile::set(std::string const & filename, std::string const & buffer_path)
+{
+	DocFileName::set(filename, buffer_path);
+	if (filename.empty())
+		return;
+
+	inzip_name_ = to_utf8(makeRelPath(from_utf8(absFilename()),
+			from_utf8(buffer_path)));
+	// if inzip_name_ is an absolute path, this file is not embeddable
+	if (FileName(inzip_name_).isAbsolute())
+		inzip_name_ = "";
+	// replace .. by upDirName
+	if (prefixIs(inzip_name_, "."))
+		inzip_name_ = subst(inzip_name_, "..", upDirName);
+	LYXERR(Debug::FILES, "Create embedded file " << filename <<
+		" with in zip name " << inzip_name_ << endl);
 }
 
 
 string EmbeddedFile::embeddedFile(Buffer const * buf) const
 {
-	return addName(buf->temppath(), inzip_name_);
+	BOOST_ASSERT(embeddable());
+	string temp = buf->temppath();
+	if (!suffixIs(temp, '/'))
+		temp += '/';
+	return temp + inzip_name_;
+}
+
+
+string EmbeddedFile::availableFile(Buffer const * buf) const
+{
+	return embedded() ? embeddedFile(buf) : absFilename();
 }
 
 
 void EmbeddedFile::addInset(Inset const * inset)
 {
-	inset_list_.push_back(inset);
+	if (inset != NULL)
+		inset_list_.push_back(inset);
 }
 
 
@@ -76,36 +126,22 @@ Inset const * EmbeddedFile::inset(int idx) const
 }
 
 
-void EmbeddedFile::saveBookmark(Buffer const * buf, int idx) const
+void EmbeddedFile::setEmbed(bool embed)
 {
-	Inset const * ptr = inset(idx);
+	if (!embeddable() && embed) {
-	// This might not be the most efficient method ... 
+		Alert::error(_("Embedding failed."), bformat(
-	for (InsetIterator it = inset_iterator_begin(buf->inset()); it; ++it)
+			_("Cannot embed file %1$s because its path is not relative to document path."),
-		if (&(*it) == ptr) {
+			from_utf8(absFilename())));
-			// this is basically BufferView::saveBookmark(0)
+		return;
-			LyX::ref().session().bookmarks().save(
+	}
-				FileName(buf->absFileName()),
+	embedded_ = embed;
-				it.bottom().pit(),
-				it.bottom().pos(),
-				it.paragraph().id(),
-				it.pos(),
-				0
-			);
-		}
-	// this inset can not be located. There is something wrong that needs
-	// to be fixed.
-	BOOST_ASSERT(true);
-}
-
-
-string EmbeddedFile::availableFile(Buffer const * buf) const
-{
-	return embedded() ? embeddedFile(buf) : absFilename();
 }
 
 
 bool EmbeddedFile::extract(Buffer const * buf) const
 {
+	BOOST_ASSERT(embeddable());
+
 	string ext_file = absFilename();
 	string emb_file = embeddedFile(buf);
 
@@ -113,7 +149,10 @@ bool EmbeddedFile::extract(Buffer const * buf) const
 	FileName ext(ext_file);
 
 	if (!emb.exists())
-		return false;
+		throw ExceptionMessage(ErrorException, _("Failed to extract file"),
+			bformat(_("Cannot extract file '%1$s'.\n"
+			"Source file %2$s does not exist"),
+			from_utf8(outputFilename()), from_utf8(emb_file)));
 
 	// if external file already exists ...
 	if (ext.exists()) {
@@ -142,8 +181,10 @@ bool EmbeddedFile::extract(Buffer const * buf) const
 		return false;
 	}
 
-	if (emb.copyTo(ext))
+	if (emb.copyTo(ext)) {
+		LYXERR(Debug::FILES, "Extract file " << emb_file << " to " << ext_file << endl);
 		return true;
+	}
 
 	throw ExceptionMessage(ErrorException, _("Copy file failure"),
 		 bformat(_("Cannot copy file %1$s to %2$s.\n"
@@ -155,14 +196,25 @@ bool EmbeddedFile::extract(Buffer const * buf) const
 
 bool EmbeddedFile::updateFromExternalFile(Buffer const * buf) const
 {
+	BOOST_ASSERT(embeddable());
+
 	string ext_file = absFilename();
 	string emb_file = embeddedFile(buf);
 
 	FileName emb(emb_file);
 	FileName ext(ext_file);
 
-	if (!ext.exists())
+	if (!ext.exists()) {
-		return false;
+		// no need to update
+		if (emb.exists())
+			return true;
+		// no external and internal file
+		throw ExceptionMessage(ErrorException,
+			_("Failed to embed file"),
+			bformat(_("Failed to embed file %1$s.\n"
+			   "Please check whether this file exists and is readable."),
+				from_utf8(ext_file)));
+	}
 
 	// if embedded file already exists ...
 	if (emb.exists()) {
@@ -205,6 +257,20 @@ void EmbeddedFile::updateInsets(Buffer const * buf) const
 }
 
 
+bool operator==(EmbeddedFile const & lhs, EmbeddedFile const & rhs)
+{
+	return lhs.absFilename() == rhs.absFilename()
+		&& lhs.saveAbsPath() == rhs.saveAbsPath()
+		&& lhs.embedded() == rhs.embedded();
+}
+
+
+bool operator!=(EmbeddedFile const & lhs, EmbeddedFile const & rhs)
+{
+	return !(lhs == rhs);
+}
+
+
 bool EmbeddedFiles::enabled() const
 {
 	return buffer_->params().embedded;
@@ -232,25 +298,26 @@ void EmbeddedFiles::enable(bool flag)
 }
 
 
-EmbeddedFile & EmbeddedFiles::registerFile(string const & filename,
+EmbeddedFile & EmbeddedFiles::registerFile(EmbeddedFile const & file, Inset const * inset)
-	bool embed, Inset const * inset, string const & inzipName)
 {
-	// filename can be relative or absolute, translate to absolute filename
-	string abs_filename = makeAbsPath(filename, buffer_->filePath()).absFilename();
 	// try to find this file from the list
 	EmbeddedFileList::iterator it = file_list_.begin();
 	EmbeddedFileList::iterator it_end = file_list_.end();
 	for (; it != it_end; ++it)
-		if (it->absFilename() == abs_filename || it->embeddedFile(buffer_) == abs_filename)
+		if (it->absFilename() == file.absFilename()) {
-			break;
+			if (it->embedded() != file.embedded()) {
-	// find this filename, keep the original embedding status
+				Alert::error(_("Wrong embedding status."),
-	if (it != file_list_.end()) {
+					bformat(_("File %1$s is included in more than one insets, "
-		it->addInset(inset);
+						"but with different embedding status. Assuming embedding status."),
-		return *it;
+						from_utf8(it->outputFilename())));
-	}
+				it->setEmbed(true);
+			}
+			it->addInset(inset);
+			return *it;
+		}
 	//
-	file_list_.push_back(EmbeddedFile(abs_filename, 
+	file_list_.push_back(file);
-		getInzipName(abs_filename, inzipName), embed, inset));
+	file_list_.back().addInset(inset);
 	return file_list_.back();
 }
 
@@ -278,11 +345,14 @@ bool EmbeddedFiles::writeFile(DocFileName const & filename)
 	EmbeddedFileList::iterator it_end = file_list_.end();
 	for (; it != it_end; ++it) {
 		if (it->embedded()) {
-			string file = it->availableFile(buffer_);
+			string file = it->embeddedFile(buffer_);
-			if (file.empty())
+			if (!FileName(file).exists())
-				lyxerr << "File " << it->absFilename() << " does not exist. Skip embedding it. " << endl;
+				throw ExceptionMessage(ErrorException, _("Failed to write file"),
-			else
+					bformat(_("Embedded file %1$s does not exist. Did you tamper lyx temporary directory?"),
-				filenames.push_back(make_pair(file, it->inzipName()));
+						it->displayName()));
+			filenames.push_back(make_pair(file, it->inzipName()));
+			LYXERR(Debug::FILES, "Writing file " << it->outputFilename()
+				<< " as " << it->inzipName() << endl);
 		}
 	}
 	// write a zip file with all these files. Write to a temp file first, to
@@ -332,7 +402,7 @@ bool EmbeddedFiles::extractAll() const
 				count_extracted += 1;
 		} else
 			count_external += 1;
-	docstring const msg = bformat(_("%1$d external files are ignored.\n"
+	docstring const msg = bformat(_("%1$d external or non-embeddable files are ignored.\n"
 		"%2$d embedded files are extracted.\n"), count_external, count_extracted);
 	Alert::information(_("Unpacking all files"), msg);
 	return true;
@@ -353,51 +423,16 @@ bool EmbeddedFiles::updateFromExternalFile() const
 					bformat(_("Error: can not embed file %1$s.\n"), it->displayName()));
 				return false;
 			} else
-				count_external += 1;
+				count_embedded += 1;
 		} else
 			count_external += 1;
-	docstring const msg = bformat(_("%1$d external files are ignored.\n"
+	docstring const msg = bformat(_("%1$d external or non-embeddable files are ignored.\n"
 		"%2$d embeddable files are embedded.\n"), count_external, count_embedded);
 	Alert::information(_("Packing all files"), msg);
 	return true;
 }
 
 
-string const EmbeddedFiles::getInzipName(string const & abs_filename, string const & name)
-{
-	// register a new one, using relative file path as inzip_name
-	string inzip_name = name;
-	if (name.empty())
-		inzip_name = to_utf8(makeRelPath(from_utf8(abs_filename),
-			from_utf8(buffer_->filePath())));
-	// if inzip_name is an absolute path, use filename only to avoid
-	// leaking of filesystem information in inzip_name
-	// The second case covers cases '../path/file' and '.'
-	if (FileName(inzip_name).isAbsolute() || prefixIs(inzip_name, "."))
-		inzip_name = onlyFilename(abs_filename);
-	// if this name has been used...
-	// use _1_name, _2_name etc
-	string tmp = inzip_name;
-	EmbeddedFileList::iterator it;
-	EmbeddedFileList::iterator it_end = file_list_.end();
-	bool unique_name = false;
-	size_t i = 0;
-	while (!unique_name) {
-		unique_name = true;
-		if (i > 0)
-			inzip_name = convert<string>(i) + "_" + tmp;
-		it = file_list_.begin();
-		for (; it != it_end; ++it)
-			if (it->inzipName() == inzip_name) {
-				unique_name = false;
-				++i;
-				break;
-			}
-	}
-	return inzip_name;
-}
-
-
 void EmbeddedFiles::updateInsets() const
 {
 	EmbeddedFiles::EmbeddedFileList::const_iterator it = begin();
@@ -407,5 +442,4 @@ void EmbeddedFiles::updateInsets() const
 			it->updateInsets(buffer_);
 }
 
-
 }

src/EmbeddedFiles.h

@@ -56,7 +56,10 @@ be embedded. These embedded files can be viewed or edited through
 the embedding dialog. This file can be shared with others more
 easily. 
 
-Format a anb b can be converted easily, by packing/unpacking a .lyx file.
+Format a and b can be converted easily, by packing/unpacking a .lyx file.
+
+NOTE: With current implementation, files with absolute filenames (not in
+or deeper under the current document directory) can not be embedded.
 
 Implementation:
 ======================
@@ -95,14 +98,15 @@ class ErrorList;
 class EmbeddedFile : public support::DocFileName
 {
 public:
-	EmbeddedFile() {};
+	EmbeddedFile(std::string const & file = std::string(),
+		std::string const & buffer_path = std::string());
 	
-	EmbeddedFile(std::string const & file, std::string const & inzip_name,
+	/// set filename and inzipName.
-		bool embedded, Inset const * inset);
+	void set(std::string const & filename, std::string const & buffer_path);
 	
-	/// filename in the zip file, usually the relative path
+	/// filename in the zip file, which is the relative path
 	std::string inzipName() const { return inzip_name_; }
-	void setInzipName(std::string name) { inzip_name_ = name; }
+
 	/// embedded file, equals to temppath()/inzipName()
 	std::string embeddedFile(Buffer const * buf) const;
 	/// embeddedFile() or absFilename() depending on embedding status
@@ -111,9 +115,6 @@ public:
 	/// add an inset that refers to this file
 	void addInset(Inset const * inset);
 	Inset const * inset(int idx) const;
-	/// save the location of this inset as bookmark so that
-	/// it can be located using LFUN_BOOKMARK_GOTO
-	void saveBookmark(Buffer const * buf, int idx) const;
 	/// Number of Insets this file item is referred
 	/// If refCount() == 0, this file must be manually inserted.
 	/// This fact is used by the update() function to skip updating
@@ -124,7 +125,9 @@ public:
 	bool embedded() const { return embedded_; }
 	/// set embedding status. updateFromExternal() should be called before this
 	/// to copy or sync the embedded file with external one.
-	void setEmbed(bool embed) { embedded_ = embed; }
+	void setEmbed(bool embed);
+	/// Only files in or under current document path is embeddable
+	bool embeddable() const { return inzip_name_ != ""; } 
 
 	/// extract file, does not change embedding status
 	bool extract(Buffer const * buf) const;
@@ -132,9 +135,10 @@ public:
 	bool updateFromExternalFile(Buffer const * buf) const;
 	///
 	/// After the embedding status is changed, update all insets related
-	/// to this file item.
+	/// to this file item. For example, a graphic inset may need to monitor
-	/// Because inset pointers may not be up to date, EmbeddedFiles::update()
+	/// embedded file instead of external file. To make sure inset pointers 
-	/// would better be called before this function is called.
+	/// are up to date, please make sure there is no modification to the
+	/// document between EmbeddedFiles::update() and this function.
 	void updateInsets(Buffer const * buf) const;
 
 private:
@@ -148,6 +152,10 @@ private:
 };
 
 
+bool operator==(EmbeddedFile const & lhs, EmbeddedFile const & rhs);
+bool operator!=(EmbeddedFile const & lhs, EmbeddedFile const & rhs);
+
+
 class EmbeddedFiles {
 public:
 	typedef std::vector<EmbeddedFile> EmbeddedFileList;
@@ -164,15 +172,10 @@ public:
 	void enable(bool flag);
 
 	/// add a file item. 
-	/* \param filename filename to add
+	/* \param file Embedded file to add
-	 * \param embed embedding status. For a new file item, this is always true.
-	 *    If the file already exists, this parameter is ignored.
 	 * \param inset Inset pointer
-	 * \param inzipName suggested inzipname
 	 */
-	EmbeddedFile & registerFile(std::string const & filename, bool embed = false,
+	EmbeddedFile & registerFile(EmbeddedFile const & file, Inset const * inset = 0);
-		Inset const * inset = 0,
-		std::string const & inzipName = std::string());
 
 	/// scan the buffer and get a list of EmbeddedFile
 	void update();
@@ -200,8 +203,6 @@ public:
 	/// update all insets to use embedded files when embedding status is changed
 	void updateInsets() const;
 private:
-	/// get a unique inzip name, a suggestion can be given.
-	std::string const getInzipName(std::string const & name, std::string const & inzipName);
 	/// list of embedded files
 	EmbeddedFileList file_list_;
 	///

src/insets/InsetGraphics.cpp

@@ -77,6 +77,7 @@ TODO
 
 #include "support/convert.h"
 #include "support/docstream.h"
+#include "support/ExceptionMessage.h"
 #include "support/filetools.h"
 #include "support/lyxlib.h"
 #include "support/lstrings.h"
@@ -94,6 +95,8 @@ using namespace lyx::support;
 
 namespace lyx {
 
+namespace Alert = frontend::Alert;
+
 namespace {
 
 /// Find the most suitable image format for images in \p format
@@ -168,9 +171,20 @@ void InsetGraphics::doDispatch(Cursor & cur, FuncRequest & cmd)
 		Buffer const & buffer = cur.buffer();
 		InsetGraphicsParams p;
 		InsetGraphicsMailer::string2params(to_utf8(cmd.argument()), buffer, p);
-		if (!p.filename.empty())
+		// when embedding is enabled, change of embedding status leads to actions
+		if (!p.filename.empty() && buffer.embeddedFiles().enabled() ) {
+			try {
+				if (p.filename.embedded())
+					p.filename.updateFromExternalFile(&buffer);
+				else
+					p.filename.extract(&buffer);
+			} catch (ExceptionMessage const & message) {
+				Alert::error(message.title_, message.details_);
+				// do not set parameter if an error happens
+				break;
+			}
 			setParams(p);
-		else
+		} else
 			cur.noUpdate();
 		break;
 	}
@@ -210,8 +224,7 @@ bool InsetGraphics::getStatus(Cursor & cur, FuncRequest const & cmd,
 void InsetGraphics::registerEmbeddedFiles(Buffer const &, 
 	EmbeddedFiles & files) const
 {
-	files.registerFile(params().filename.absFilename(), 
+	files.registerFile(params().filename, this);
-		false, this);
 }
 
 

src/support/FileName.h

@@ -209,7 +209,7 @@ public:
 	 *  \param buffer_path if \c filename has a relative path, generate
 	 *  the absolute path using this.
 	 */
-	void set(std::string const & filename, std::string const & buffer_path);
+	virtual void set(std::string const & filename, std::string const & buffer_path);
 
 	void erase();