Dear all,

When some encoding error happens, it is extremely difficult to locate
the offending character. LyX gives an error message "Some characters
of your document are probably not representable in the chosen
encoding." with no hint of which and where the character is. This is,
IMHO, a major issue with 1.5.3.

In the attached patch, I try to

1. define encoding_exception
+class encoding_exception : public std::exception {
+public:
+       virtual ~encoding_exception() throw() {}
+       virtual const char * what() const throw();
+};

2.  Throw this exception in Encoding::Encoding. It is strange to me
that we do nothing here when we know ".\nLaTeX export will fail.".

Encoding::Encoding(string const & n, string const & l, string const & i,
                   bool f, Encoding::Package p)
        : Name_(n), LatexName_(l), iconvName_(i), fixedwidth_(f), package_(p)
@@ -325,10 +331,7 @@
                // c cannot be encoded in this encoding
                CharInfoMap::const_iterator const it = unicodesymbols.find(c);
                if (it == unicodesymbols.end())
-                       lyxerr << "Could not find LaTeX command for character 
0x"
-                              << std::hex << c << std::dec
-                              << ".\nLaTeX export will fail."
-                              << endl;
+                       throw encoding_exception();

3. catch this exception in TeXOnePar

-       bool need_par = pit->simpleTeXOnePar(buf, bparams, outerfont,
+       bool need_par = false;
+       try {
+               need_par = pit->simpleTeXOnePar(buf, bparams, outerfont,
                                             os, texrow, runparams);
+       } catch (encoding_exception & e) {
+               //      
+               errorList.push_back(ErrorItem(_("Encoding error"),
+                       _(" "), -1, 0, 0));
+       }

HOWEVER, passing errorList to this function means interface change to
a lot of functions, which makes this patch somewhat large.

With this proof-of-concept patch, when encoding error happens, a
errorlist dialog will display. Clicking on the error item will (not
yet) move the cursor to the offending character. I would like to ask

1. Is this in general welcome?
2. Any chance for 1.5.3?
3. Any better idea?

Cheers,
Bo
Index: src/insets/InsetEnvironment.cpp
===================================================================
--- src/insets/InsetEnvironment.cpp	(revision 22160)
+++ src/insets/InsetEnvironment.cpp	(working copy)
@@ -14,6 +14,7 @@
 
 #include "BufferParams.h"
 #include "gettext.h"
+#include "ErrorList.h"
 #include "OutputParams.h"
 #include "output_latex.h"
 #include "TexRow.h"
@@ -71,8 +72,10 @@
 	// FIXME UNICODE
 	os << from_utf8(layout_->latexheader);
 	TexRow texrow;
+	ErrorList errorList;
 	latexParagraphs(buf, paragraphs(), os, texrow, runparams,
-			layout_->latexparagraph);
+			errorList, layout_->latexparagraph);
+	// FIXME handle err.
 	// FIXME UNICODE
 	os << from_utf8(layout_->latexfooter);
 	return texrow.rows();
Index: src/insets/InsetText.cpp
===================================================================
--- src/insets/InsetText.cpp	(revision 22160)
+++ src/insets/InsetText.cpp	(working copy)
@@ -287,7 +287,9 @@
 		     OutputParams const & runparams) const
 {
 	TexRow texrow;
-	latexParagraphs(buf, paragraphs(), os, texrow, runparams);
+	ErrorList errorList;
+	latexParagraphs(buf, paragraphs(), os, texrow, runparams, errorList);
+	// FIXME: handle errorList?
 	return texrow.rows();
 }
 
Index: src/output_latex.h
===================================================================
--- src/output_latex.h	(revision 22160)
+++ src/output_latex.h	(working copy)
@@ -26,6 +26,7 @@
 class ParagraphList;
 class OutputParams;
 class TexRow;
+class ErrorList;
 
 /// Export up to \p number optarg insets
 int latexOptArgInsets(Buffer const & buf, Paragraph const & par,
@@ -42,6 +43,7 @@
 		     odocstream & ofs,
 		     TexRow & texrow,
 		     OutputParams const &,
+			 ErrorList &,
 		     std::string const & everypar = std::string());
 
 /// Switch the encoding of \p os from \p oldEnc to \p newEnc if needed.
Index: src/graphics/PreviewLoader.cpp
===================================================================
--- src/graphics/PreviewLoader.cpp	(revision 22160)
+++ src/graphics/PreviewLoader.cpp	(working copy)
@@ -19,6 +19,7 @@
 #include "Converter.h"
 #include "debug.h"
 #include "Encoding.h"
+#include "ErrorList.h"
 #include "Format.h"
 #include "InsetIterator.h"
 #include "Color.h"
@@ -704,7 +705,9 @@
 	runparams.nice = true;
 	runparams.moving_arg = true;
 	runparams.free_spacing = true;
-	tmp.writeLaTeXSource(os, buffer_.filePath(), runparams, true, false);
+	ErrorList err;
+	tmp.writeLaTeXSource(os, buffer_.filePath(), runparams, err, true, false);
+	/// FIXME err should be handled here.
 
 	// FIXME! This is a HACK! The proper fix is to control the 'true'
 	// passed to WriteStream below:
Index: src/Buffer.h
===================================================================
--- src/Buffer.h	(revision 22160)
+++ src/Buffer.h	(working copy)
@@ -202,6 +202,7 @@
 	void writeLaTeXSource(odocstream & os,
 			   std::string const & original_path,
 			   OutputParams const &,
+			   ErrorList &,
 			   bool output_preamble = true,
 			   bool output_body = true);
 	///
Index: src/output_latex.cpp
===================================================================
--- src/output_latex.cpp	(revision 22160)
+++ src/output_latex.cpp	(working copy)
@@ -16,6 +16,8 @@
 #include "BufferParams.h"
 #include "debug.h"
 #include "Encoding.h"
+#include "ErrorList.h"
+#include "gettext.h"
 #include "Language.h"
 #include "LyXRC.h"
 #include "OutputParams.h"
@@ -48,7 +50,8 @@
 	       ParagraphList const & paragraphs,
 	       ParagraphList::const_iterator pit,
 	       odocstream & os, TexRow & texrow,
-	       OutputParams const & runparams);
+	       OutputParams const & runparams,
+		   ErrorList & errorList);
 
 ParagraphList::const_iterator
 TeXOnePar(Buffer const & buf,
@@ -56,6 +59,7 @@
 	  ParagraphList::const_iterator pit,
 	  odocstream & os, TexRow & texrow,
 	  OutputParams const & runparams,
+	  ErrorList & errorList,
 	  string const & everypar = string());
 
 
@@ -64,7 +68,8 @@
 	  ParagraphList const & paragraphs,
 	  ParagraphList::const_iterator pit,
 	  odocstream & os, TexRow & texrow,
-	  OutputParams const & runparams)
+	  OutputParams const & runparams,
+	  ErrorList & errorList)
 {
 	LYXERR(Debug::LATEX) << "TeXDeeper...     " << &*pit << endl;
 	ParagraphList::const_iterator par = pit;
@@ -73,10 +78,10 @@
 		     par->params().depth() == pit->params().depth()) {
 		if (par->layout()->isEnvironment()) {
 			par = TeXEnvironment(buf, paragraphs, par,
-					     os, texrow, runparams);
+					     os, texrow, runparams, errorList);
 		} else {
 			par = TeXOnePar(buf, paragraphs, par,
-					     os, texrow, runparams);
+					     os, texrow, runparams, errorList);
 		}
 	}
 	LYXERR(Debug::LATEX) << "TeXDeeper...done " << endl;
@@ -90,7 +95,8 @@
 	       ParagraphList const & paragraphs,
 	       ParagraphList::const_iterator pit,
 	       odocstream & os, TexRow & texrow,
-	       OutputParams const & runparams)
+	       OutputParams const & runparams,
+		   ErrorList & errorList)
 {
 	LYXERR(Debug::LATEX) << "TeXEnvironment...     " << &*pit << endl;
 
@@ -163,7 +169,7 @@
 	}
 	ParagraphList::const_iterator par = pit;
 	do {
-		par = TeXOnePar(buf, paragraphs, par, os, texrow, runparams);
+		par = TeXOnePar(buf, paragraphs, par, os, texrow, runparams, errorList);
 
 		if (par == paragraphs.end()) {
 			// Make sure that the last paragraph is
@@ -192,7 +198,7 @@
 				texrow.newline();
 			}
 			par = TeXDeeper(buf, paragraphs, par, os, texrow,
-					runparams);
+					runparams, errorList);
 		}
 	} while (par != paragraphs.end()
 		 && par->layout() == pit->layout()
@@ -244,6 +250,7 @@
 	  ParagraphList::const_iterator pit,
 	  odocstream & os, TexRow & texrow,
 	  OutputParams const & runparams_in,
+	  ErrorList & errorList,
 	  string const & everypar)
 {
 	LYXERR(Debug::LATEX) << "TeXOnePar...     " << &*pit << " '"
@@ -442,8 +449,15 @@
 
 	// FIXME UNICODE
 	os << from_utf8(everypar);
-	bool need_par = pit->simpleTeXOnePar(buf, bparams, outerfont,
+	bool need_par = false;
+	try {
+		need_par = pit->simpleTeXOnePar(buf, bparams, outerfont,
 					     os, texrow, runparams);
+	} catch (encoding_exception & e) {
+		//	
+		errorList.push_back(ErrorItem(_("Encoding error"),
+			_(" "), -1, 0, 0));
+	}
 
 	// Make sure that \\par is done with the font of the last
 	// character if this has another size as the default.
@@ -605,6 +619,7 @@
 		     odocstream & os,
 		     TexRow & texrow,
 		     OutputParams const & runparams,
+			 ErrorList & errorList,
 		     string const & everypar)
 {
 	bool was_title = false;
@@ -665,18 +680,18 @@
 
 			if (layout->is_environment) {
 				par = TeXOnePar(buf, paragraphs, par, os, texrow,
-						runparams, everypar);
+						runparams, errorList, everypar);
 			} else if (layout->isEnvironment() ||
 				   !par->params().leftIndent().zero()) {
 				par = TeXEnvironment(buf, paragraphs, par, os,
-						     texrow, runparams);
+						     texrow, runparams, errorList);
 			} else {
 				par = TeXOnePar(buf, paragraphs, par, os, texrow,
-						runparams, everypar);
+						runparams, errorList, everypar);
 			}
 		} else {
 			par = TeXOnePar(buf, paragraphs, par, os, texrow,
-					runparams, everypar);
+					runparams, errorList, everypar);
 		}
 		if (std::distance(lastpar, par) >= std::distance(lastpar, endpar))
 			break;
Index: src/Encoding.cpp
===================================================================
--- src/Encoding.cpp	(revision 22160)
+++ src/Encoding.cpp	(working copy)
@@ -251,6 +251,12 @@
 } // namespace anon
 
 
+const char * encoding_exception::what() const throw()
+{
+	return "Could not find LaTeX command for a character";
+}
+
+
 Encoding::Encoding(string const & n, string const & l, string const & i,
 		   bool f, Encoding::Package p)
 	: Name_(n), LatexName_(l), iconvName_(i), fixedwidth_(f), package_(p)
@@ -325,10 +331,7 @@
 		// c cannot be encoded in this encoding
 		CharInfoMap::const_iterator const it = unicodesymbols.find(c);
 		if (it == unicodesymbols.end())
-			lyxerr << "Could not find LaTeX command for character 0x"
-			       << std::hex << c << std::dec
-			       << ".\nLaTeX export will fail."
-			       << endl;
+			throw encoding_exception();
 		else
 			return it->second.command;
 	}
Index: src/Buffer.cpp
===================================================================
--- src/Buffer.cpp	(revision 22160)
+++ src/Buffer.cpp	(working copy)
@@ -938,45 +938,49 @@
 	if (!openFileWrite(ofs, fname))
 		return false;
 
+	ErrorList & errorList = pimpl_->errorLists["Export"];
+	errorList.clear();
 	bool failed_export = false;
 	try {
 		writeLaTeXSource(ofs, original_path,
-		      runparams, output_preamble, output_body);
+		      runparams, errorList, output_preamble, output_body);
 	}
 	catch (iconv_codecvt_facet_exception & e) {
-		lyxerr << "Caught iconv exception: " << e.what() << endl;
+		errorList.push_back(ErrorItem(_("IConv exception"),
+			_(e.what()), -1, 0, 0));
 		failed_export = true;
 	}
 	catch (std::exception const & e) {
-		lyxerr << "Caught \"normal\" exception: " << e.what() << endl;
+		errorList.push_back(ErrorItem(_("Normal exception"),
+			_(e.what()), -1, 0, 0));
 		failed_export = true;
 	}
 	catch (...) {
-		lyxerr << "Caught some really weird exception..." << endl;
+		errorList.push_back(ErrorItem(_("Unknown exception"),
+			_("Unknown"), -1, 0, 0));
 		LyX::cref().emergencyCleanup();
 		abort();
 	}
 
 	ofs.close();
 	if (ofs.fail()) {
+		errorList.push_back(ErrorItem(_("File Close"),
+			_("File " + fname.toFilesystemEncoding() + " was not closed properly"), -1, 0, 0));
 		failed_export = true;
 		lyxerr << "File '" << fname << "' was not closed properly." << endl;
 	}
 
-	if (failed_export) {
-		Alert::error(_("Encoding error"),
-			_("Some characters of your document are probably not "
-			"representable in the chosen encoding.\n"
-			"Changing the document encoding to utf8 could help."));
-		return false;
-	}
-	return true;
+	busy(false);
+	errors("Export");
+
+	return !failed_export;
 }
 
 
 void Buffer::writeLaTeXSource(odocstream & os,
 			   string const & original_path,
 			   OutputParams const & runparams_in,
+			   ErrorList & errorList,
 			   bool const output_preamble, bool const output_body)
 {
 	OutputParams runparams = runparams_in;
@@ -1074,7 +1078,7 @@
 	}
 
 	// the real stuff
-	latexParagraphs(*this, paragraphs(), os, texrow(), runparams);
+	latexParagraphs(*this, paragraphs(), os, texrow(), runparams, errorList);
 
 	// Restore the parenthood if needed
 	if (output_preamble)
@@ -1822,10 +1826,11 @@
 	// No side effect of file copying and image conversion
 	runparams.dryrun = true;
 
+	ErrorList & errorList = pimpl_->errorLists["Export"];
 	if (full_source) {
 		os << "% " << _("Preview source code") << "\n\n";
 		if (isLatex())
-			writeLaTeXSource(os, filePath(), runparams, true, true);
+			writeLaTeXSource(os, filePath(), runparams, errorList, true, true);
 		else {
 			writeDocBookSource(os, fileName(), runparams, false);
 		}
@@ -1845,12 +1850,13 @@
 		// output paragraphs
 		if (isLatex()) {
 			texrow().reset();
-			latexParagraphs(*this, paragraphs(), os, texrow(), runparams);
+			latexParagraphs(*this, paragraphs(), os, texrow(), runparams, errorList);
 		} else {
 			// DocBook
 			docbookParagraphs(paragraphs(), *this, os, runparams);
 		}
 	}
+	errors("Export");
 }
 
 
Index: src/Encoding.h
===================================================================
--- src/Encoding.h	(revision 22160)
+++ src/Encoding.h	(working copy)
@@ -24,7 +24,13 @@
 
 class LaTeXFeatures;
 
+class encoding_exception : public std::exception {
+public:
+	virtual ~encoding_exception() throw() {}
+	virtual const char * what() const throw();
+};
 
+
 ///
 class Encoding {
 public:

Reply via email to