improved error reporting for the keyword editor

data/en.mse-locale/locale

@@ -60,7 +60,7 @@ menu:
 	set info tab:		&Set Information	F6
 	style tab:			St&yle	F7
 	keywords tab:		&Keywords	F8
-	stats tab:			S&tatistics	F8
+	stats tab:			S&tatistics	F9
 	
 	help:			&Help
 	index:				&Index...	F1
@@ -331,6 +331,9 @@ label:
 	mode:				Mode
 	uses:				Uses
 	reminder:			Reminder text
+	standard keyword:
+		This is a standard %s keyword, you can not edit it.
+		If you make a copy of the keyword your copy will take precedent.
 	
 	# Open dialogs
 	all files			All files

src/data/action/keyword.cpp

@@ -84,27 +84,30 @@ void KeywordReminderTextValue::store() {
 		retrieve();
 		return;
 	}
-	// Re-highlight
+	// new value
 	String new_value = untag(value);
-	highlight(new_value);
 	// Try to parse the script
-	try {
-		ScriptP new_script = parse(new_value, true);
+	vector<ScriptParseError> parse_errors;
+	ScriptP new_script = parse(new_value, true, parse_errors);
+	if (parse_errors.empty()) {
 		// parsed okay, assign
 		errors.clear();
 		keyword.reminder.getScriptP()  = new_script;
 		keyword.reminder.getUnparsed() = new_value;
-	} catch (const Error& e) {
+	} else {
 		// parse errors, report
-		errors = e.what(); // TODO
+		errors = ScriptParseErrors(parse_errors).what();
 	}
+	// re-highlight input, show errors
+	highlight(new_value, parse_errors);
 }
 
 void KeywordReminderTextValue::retrieve() {
-	highlight(*underlying);
+	vector<ScriptParseError> no_errors;
+	highlight(*underlying, no_errors);
 }
 
-void KeywordReminderTextValue::highlight(const String& code) {
+void KeywordReminderTextValue::highlight(const String& code, const vector<ScriptParseError>& errors) {
 	// Add tags to indicate code / syntax highlight
 	// i.e.  bla {if code "x" } bla
 	// becomes:
@@ -112,7 +115,24 @@ void KeywordReminderTextValue::highlight(const String& code) {
 	String new_value;
 	int in_brace = 0;
 	bool in_string = true;
+	vector<ScriptParseError>::const_iterator error = errors.begin();
 	for (size_t pos = 0 ; pos < code.size() ; ) {
+		// error underlining
+		while (error != errors.end() && error->start == error->end) ++error;
+		if (error != errors.end()) {
+			if (error->start == pos) {
+				new_value += _("<error>");
+			}
+			if (error->end == pos) {
+				++error;
+				if (error == errors.end() || error->start > pos) {
+					new_value += _("</error>");
+				} else {
+					// immediatly open again
+				}
+			}
+		}
+		// process a character
 		Char c = code.GetChar(pos);
 		if (c == _('<')) {
 			new_value += _('\1'); // escape

src/data/action/keyword.hpp

@@ -16,6 +16,7 @@
 
 #include <util/prec.hpp>
 #include <util/action_stack.hpp>
+#include <util/error.hpp>
 #include <data/field/text.hpp>
 
 class Set;
@@ -93,7 +94,7 @@ class KeywordReminderTextValue : public KeywordTextValue {
 	virtual void retrieve();
 	
 	/// Syntax highlight, and store in value
-	void highlight(const String& code);
+	void highlight(const String& code, const vector<ScriptParseError>& errors);
 };
 
 // ----------------------------------------------------------------------------- : EOF

src/render/text/compound.cpp

@@ -33,3 +33,23 @@ void AtomTextElement::draw(RotatedDC& dc, double scale, const RealRect& rect, co
 	}
 	CompoundTextElement::draw(dc, scale, rect, xs, what, start, end);
 }
+
+// ----------------------------------------------------------------------------- : ErrorTextElement
+
+void ErrorTextElement::draw(RotatedDC& dc, double scale, const RealRect& rect, const double* xs, DrawWhat what, size_t start, size_t end) const {
+	// Draw wavy underline
+	dc.SetPen(*wxRED_PEN);
+	RealPoint pos = rect.bottomLeft() - dc.trInvS(RealSize(0,2));
+	RealSize  dx(dc.trInvS(2), 0), dy(0, dc.trInvS(1));
+	while (pos.x + 1 < rect.right()) {
+		dc.DrawLine(pos - dy, pos + dx + dy);
+		pos += dx;
+		dy  = -dy;
+	}
+	if (pos.x < rect.right()) {
+		// final piece
+		dc.DrawLine(pos - dy, pos + dx / 2);
+	}
+	// Draw the contents
+	CompoundTextElement::draw(dc, scale, rect, xs, what, start, end);
+}
\ No newline at end of file

src/render/text/element.cpp

@@ -73,18 +73,19 @@ struct TextElementsFromString {
 	// What formatting is enabled?
 	int bold, italic, symbol;
 	int soft, kwpph, param, line;
-	int code, code_kw, code_string, param_ref;
+	int code, code_kw, code_string, param_ref, error;
 	int param_id;
 	bool bracket;
 	
 	TextElementsFromString()
 		: bold(0), italic(0), symbol(0), soft(0), kwpph(0), param(0), line(0)
-		, code(0), code_kw(0), code_string(0), param_ref(0)
+		, code(0), code_kw(0), code_string(0), param_ref(0), error(0)
 		, param_id(0), bracket(false) {}
 	
 	// read TextElements from a string
 	void fromString(TextElements& te, const String& text, size_t start, size_t end, const TextStyle& style, Context& ctx) {
 		te.elements.clear();
+		end = min(end, text.size());
 		// for each character...
 		for (size_t pos = start ; pos < end ; ) {
 			Char c = text.GetChar(pos);
@@ -126,11 +127,18 @@ struct TextElementsFromString {
 				else if (is_substr(text, tag_start, _("</line")))       line        -= 1;
 				else if (is_substr(text, tag_start, _("<atom"))) {
 					// 'atomic' indicator
-					size_t end = match_close_tag(text, tag_start);
-					shared_ptr<AtomTextElement> e(new AtomTextElement(text, pos, end));
-					fromString(e->elements, text, pos, end, style, ctx);
+					size_t end_tag = min(end, match_close_tag(text, tag_start));
+					shared_ptr<AtomTextElement> e(new AtomTextElement(text, pos, end_tag));
+					fromString(e->elements, text, pos, end_tag, style, ctx);
 					te.elements.push_back(e);
-					pos = skip_tag(text, end);
+					pos = skip_tag(text, end_tag);
+				} else if (is_substr(text, tag_start, _( "<error"))) {
+					// error indicator
+					size_t end_tag = min(end, match_close_tag(text, tag_start));
+					shared_ptr<ErrorTextElement> e(new ErrorTextElement(text, pos, end_tag));
+					fromString(e->elements, text, pos, end_tag, style, ctx);
+					te.elements.push_back(e);
+					pos = skip_tag(text, end_tag);
 				} else {
 					// ignore other tags
 				}

src/render/text/element.hpp

@@ -164,6 +164,13 @@ class AtomTextElement : public CompoundTextElement {
 	virtual void draw(RotatedDC& dc, double scale, const RealRect& rect, const double* xs, DrawWhat what, size_t start, size_t end) const;
 };
 
+/// A TextElement drawn using a red wavy underline
+class ErrorTextElement : public CompoundTextElement {
+  public:
+	ErrorTextElement(const String& text, size_t start ,size_t end) : CompoundTextElement(text, start, end) {}
+	
+	virtual void draw(RotatedDC& dc, double scale, const RealRect& rect, const double* xs, DrawWhat what, size_t start, size_t end) const;
+};
 
 // ----------------------------------------------------------------------------- : Other text elements
 /*

src/script/parser.cpp

@@ -51,10 +51,11 @@ enum OpenBrace
 ,	BRACE_PAREN			// (, [, {
 };
 
-/// Iterator over a string, one token at a time
+/// Iterator over a string, one token at a time.
+/** Also stores errors found when tokenizing or parsing */
 class TokenIterator {
   public:
-	TokenIterator(const String& str, bool string_mode);
+	TokenIterator(const String& str, bool string_mode, vector<ScriptParseError>& errors);
 	
 	/// Peek at the next token, doesn't move to the one after that
 	/** Can peek further forward by using higher values of offset.
@@ -87,6 +88,14 @@ class TokenIterator {
 	void readToken();
 	/// Read the next token which is a string (after the opening ")
 	void readStringToken();
+	
+  public:
+	/// All errors found
+	vector<ScriptParseError>& errors;
+	/// Add an error message
+	void add_error(const String& message);
+	/// Expected some token instead of what was found
+	void expected(const String& exp);
 };
 
 // ----------------------------------------------------------------------------- : Characters
@@ -102,10 +111,11 @@ bool isLongOper(const String& s) { return s==_(":=") || s==_("==") || s==_("!=")
 
 // ----------------------------------------------------------------------------- : Tokenizing
 
-TokenIterator::TokenIterator(const String& str, bool string_mode)
+TokenIterator::TokenIterator(const String& str, bool string_mode, vector<ScriptParseError>& errors)
 	: input(str)
 	, pos(0)
 	, newline(false)
+	, errors(errors)
 {
 	if (string_mode) {
 		open_braces.push(BRACE_STRING_MODE);
@@ -221,7 +231,8 @@ void TokenIterator::readToken() {
 		// comment untill end of line
 		while (pos < input.size() && input[pos] != _('\n')) ++pos;
 	} else {
-		throw ScriptParseError(_("Unknown character in script: '") + String(1,c) + _("'"));
+		add_error(_("Unknown character in script: '") + String(1,c) + _("'"));
+		// just skip the character
 	}
 }
 
@@ -234,7 +245,10 @@ void TokenIterator::readStringToken() {
 				addToken(TOK_STRING, str);
 				return;
 			} else {
-				throw ScriptParseError(_("Unexpected end of input in string constant"));
+				add_error(_("Unexpected end of input in string constant"));
+				// fix up
+				addToken(TOK_STRING, str);
+				return;
 			}
 		}
 		Char c = input.GetChar(pos++);
@@ -246,7 +260,12 @@ void TokenIterator::readStringToken() {
 			return;
 		} else if (c == _('\\')) {
 			// escape
-			if (pos >= input.size()) throw ScriptParseError(_("Unexpected end of input in string constant"));
+			if (pos >= input.size()) {
+				add_error(_("Unexpected end of input in string constant"));
+				// fix up
+				addToken(TOK_STRING, str);
+				return;
+			}
 			c = input.GetChar(pos++);
 			if      (c == _('n')) str += _('\n');
 			else if (c == _('<')) str += _('\1'); // escape for <
@@ -264,8 +283,20 @@ void TokenIterator::readStringToken() {
 }
 
 
+void TokenIterator::add_error(const String& message) {
+	if (!errors.empty() && errors.back().start == pos) return; // already an error here
+	errors.push_back(ScriptParseError(pos, message));
+}
+void TokenIterator::expected(const String& expected) {
+	size_t error_pos = pos - peek(0).value.size();
+	if (!errors.empty() && errors.back().start == pos) return; // already an error here
+	errors.push_back(ScriptParseError(error_pos, expected, peek(0).value));
+}
+
+
 // ----------------------------------------------------------------------------- : Parsing
 
+
 /// Precedence levels for parsing, higher = tighter
 enum Precedence
 {	PREC_ALL
@@ -300,22 +331,43 @@ void parseExpr(TokenIterator& input, Script& script, Precedence minPrec);
  */
 void parseOper(TokenIterator& input, Script& script, Precedence minPrec, InstructionType closeWith = I_NOP, int closeWithData = 0);
 
-ScriptP parse(const String& s, bool string_mode) {
-	TokenIterator input(s, string_mode);
+
+ScriptP parse(const String& s, bool string_mode, vector<ScriptParseError>& errors_out) {
+	errors_out.clear();
+	// parse
+	TokenIterator input(s, string_mode, errors_out);
 	ScriptP script(new Script);
 	parseOper(input, *script, PREC_ALL, I_RET);
-	if (input.peek() != TOK_EOF) {
-		throw ScriptParseError(_("end of input"), input.peek().value);
-	} else {
+	Token eof = input.read();
+	if (eof != TOK_EOF) {
+		input.expected(_("end of input"));
+	}
+	// were there errors?
+	if (errors_out.empty()) {
 		return script;
+	} else {
+		return ScriptP();
+	}
+}
+
+ScriptP parse(const String& s, bool string_mode) {
+	vector<ScriptParseError> errors;
+	ScriptP script = parse(s, string_mode, errors);
+	if (!errors.empty()) {
+		throw ScriptParseErrors(errors);
 	}
+	return script;
 }
 
-// Expect a token, throws if it is not found
-void expectToken(TokenIterator& input, const Char* expect) {
+
+// Expect a token, adds an error if it is not found
+bool expectToken(TokenIterator& input, const Char* expect, const Char* name_in_error = nullptr) {
 	Token token = input.read();
-	if (token != expect) {
-		throw ScriptParseError(expect, token.value);
+	if (token == expect) {
+		return true;
+	} else {
+		input.expected(name_in_error ? name_in_error : expect);
+		return false;
 	}
 }
 
@@ -367,7 +419,9 @@ void parseExpr(TokenIterator& input, Script& script, Precedence minPrec) {
 					// for each AAA in BBB do CCC
 					input.read();										// each
 					Token name = input.read();							// AAA
-					if (name != TOK_NAME) throw ScriptParseError(_("name"), name.value);
+					if (name != TOK_NAME) {
+						input.expected(_("name"));
+					}
 					expectToken(input, _("in"));						// in
 					parseOper(input, script, PREC_SET);					// BBB
 					script.addInstruction(I_UNARY, I_ITERATOR_C);		//		iterator_collection
@@ -428,7 +482,8 @@ void parseExpr(TokenIterator& input, Script& script, Precedence minPrec) {
 		} else if (token == TOK_STRING) {
 			script.addInstruction(I_PUSH_CONST, to_script(token.value));
 		} else {
-			throw ScriptParseError(_("Unexpected token '") + token.value + _("'"));
+			input.expected(_("expression"));
+			return;
 		}
 		break;
 	}
@@ -459,12 +514,11 @@ void parseOper(TokenIterator& input, Script& script, Precedence minPrec, Instruc
 			// not an expression. Remove that instruction.
 			Instruction instr = script.getInstructions().back();
 			if (instr.instr != I_GET_VAR) {
-				throw ScriptParseError(_("Can only assign to variables"));
-			} else {
+				input.add_error(_("Can only assign to variables"));
+			}
 			script.getInstructions().pop_back();
 			parseOper(input, script, PREC_SET,  I_SET_VAR, instr.data);
 		}
-		}
 		else if (minPrec <= PREC_AND    && token==_("and"))   parseOper(input, script, PREC_CMP,   I_BINARY, I_AND);
 		else if (minPrec <= PREC_AND    && token==_("or" ))   parseOper(input, script, PREC_CMP,   I_BINARY, I_OR);
 		else if (minPrec <= PREC_CMP    && token==_("="))     parseOper(input, script, PREC_ADD,   I_BINARY, I_EQ);
@@ -484,7 +538,7 @@ void parseOper(TokenIterator& input, Script& script, Precedence minPrec, Instruc
 			if (token == TOK_NAME || token == TOK_INT || token == TOK_DOUBLE || token == TOK_STRING) {
 				script.addInstruction(I_MEMBER_C, token.value);
 			} else {
-				throw ScriptParseError(_("name"), input.peek().value);
+				input.expected(_("name"));
 			}
 		} else if (minPrec <= PREC_FUN && token==_("[")) { // get member by expr
 			parseOper(input, script, PREC_ALL, I_BINARY, I_MEMBER);
@@ -528,7 +582,7 @@ void parseOper(TokenIterator& input, Script& script, Precedence minPrec, Instruc
 			} else {
 				parseOper(input, script, PREC_ALL, I_BINARY, I_ADD);	// e
 			}
-			expectToken(input, _("}\""));
+			if (expectToken(input, _("}\""), _("}"))) {
 				parseOper(input, script, PREC_NONE);					// y
 				// optimize: e + ""  -> e
 				i = script.getInstructions().back();
@@ -537,6 +591,7 @@ void parseOper(TokenIterator& input, Script& script, Precedence minPrec, Instruc
 				} else {
 					script.addInstruction(I_BINARY, I_ADD);
 				}
+			}
 		} else if (minPrec <= PREC_NEWLINE && token.newline) {
 			// newline functions as ;
 			// only if we don't match another token!

src/script/parser.hpp

@@ -10,13 +10,25 @@
 // ----------------------------------------------------------------------------- : Includes
 
 #include <util/prec.hpp>
+#include <util/error.hpp>
 #include <script/script.hpp>
 
 // ----------------------------------------------------------------------------- : Parser
 
 /// Parse a String to a Script
 /** If string_mode then s is interpreted as a string,
- *  escaping to script mode can be done with {}
+ *  escaping to script mode can be done with {}.
+ *
+ *  Errors are stored in the output vector.
+ *  If there are errors, the result is a null pointer
+ */
+ScriptP parse(const String& s, bool string_mode, vector<ScriptParseError>& errors_out);
+
+/// Parse a String to a Script
+/** If string_mode then s is interpreted as a string,
+ *  escaping to script mode can be done with {}.
+ *
+ *  If an error is encountered, an exception is thrown.
  */
 ScriptP parse(const String& s, bool string_mode = false);
 

src/util/error.cpp

@@ -8,6 +8,8 @@
 
 #include <util/error.hpp>
 
+DECLARE_TYPEOF_COLLECTION(ScriptParseError);
+
 // ----------------------------------------------------------------------------- : Error types
 
 Error::Error(const String& message)
@@ -20,6 +22,32 @@ String Error::what() const {
 	return message;
 }
 
+// ----------------------------------------------------------------------------- : Parse errors
+
+ScriptParseError::ScriptParseError(size_t pos, const String& error)
+	: start(pos), end(pos)
+	, ParseError(error)
+{}
+ScriptParseError::ScriptParseError(size_t pos, const String& exp, const String& found)
+	: start(pos), end(pos + found.size())
+	, ParseError(_("Expected '") + exp + _("' instead of '") + found + _("'"))
+{}
+String ScriptParseError::what() const {
+	return String(_("(")) << (int)start << _("): ") << Error::what();
+}
+
+String concat(const vector<ScriptParseError>& errors) {
+	String total;
+	FOR_EACH_CONST(e, errors) {
+		if (!total.empty()) total += _("\n");
+		total += e.what();
+	}
+	return total;
+}
+ScriptParseErrors::ScriptParseErrors(const vector<ScriptParseError>& errors)
+	: ParseError(concat(errors))
+{}
+
 // ----------------------------------------------------------------------------- : Error handling
 
 // Errors for which a message box was already shown

src/util/error.hpp

@@ -75,9 +75,18 @@ class FileParseError : public ParseError {
 /// Parse error in a script
 class ScriptParseError : public ParseError {
   public:
-	inline ScriptParseError(const String& str) : ParseError(str) {}
-	inline ScriptParseError(const String& exp, const String& found)
-		: ParseError(_("Expected '") + exp + _("' instead of '") + found + _("'")) {}
+	ScriptParseError(size_t pos, const String& str);
+	ScriptParseError(size_t pos, const String& expected, const String& found);
+	/// Position of the error
+	size_t start, end;
+	/// Return the error message
+	virtual String what() const; 
+};
+
+/// Multiple parse errors in a script
+class ScriptParseErrors : public ParseError {
+  public:
+	ScriptParseErrors(const vector<ScriptParseError>& errors);
 };
 
 // ----------------------------------------------------------------------------- : Script errors