ExpressionParser/DolphinQt: Added parse results to UI.

Source/Core/DolphinQt/Config/Mapping/IOWindow.cpp

@@ -4,6 +4,7 @@
 
 #include "DolphinQt/Config/Mapping/IOWindow.h"
 
+#include <optional>
 #include <thread>
 
 #include <QComboBox>
@@ -11,13 +12,13 @@
 #include <QGroupBox>
 #include <QHBoxLayout>
 #include <QLabel>
+#include <QLineEdit>
 #include <QListWidget>
 #include <QMessageBox>
 #include <QPlainTextEdit>
 #include <QPushButton>
 #include <QSlider>
 #include <QSpinBox>
-#include <QSyntaxHighlighter>
 #include <QVBoxLayout>
 
 #include "Core/Core.h"
@@ -35,6 +36,7 @@ constexpr int SLIDER_TICK_COUNT = 100;
 
 namespace
 {
+// TODO: Make sure these functions return colors that will be visible in the current theme.
 QTextCharFormat GetSpecialCharFormat()
 {
   QTextCharFormat format;
@@ -85,56 +87,78 @@ QTextCharFormat GetFunctionCharFormat()
   format.setForeground(QBrush{Qt::darkCyan});
   return format;
 }
+}  // namespace
 
-class SyntaxHighlighter : public QSyntaxHighlighter
+ControlExpressionSyntaxHighlighter::ControlExpressionSyntaxHighlighter(QTextDocument* parent,
+                                                                       QLineEdit* result)
+    : QSyntaxHighlighter(parent), m_result_text(result)
 {
-public:
+}
-  SyntaxHighlighter(QTextDocument* parent) : QSyntaxHighlighter(parent) {}
 
-  void highlightBlock(const QString& text) final override
+void ControlExpressionSyntaxHighlighter::highlightBlock(const QString& text)
+{
+  // TODO: This is going to result in improper highlighting with non-ascii characters:
+  ciface::ExpressionParser::Lexer lexer(text.toStdString());
+
+  std::vector<ciface::ExpressionParser::Token> tokens;
+  const auto tokenize_status = lexer.Tokenize(tokens);
+
+  using ciface::ExpressionParser::TokenType;
+
+  for (auto& token : tokens)
   {
-    // TODO: This is going to result in improper highlighting with non-ascii characters:
+    std::optional<QTextCharFormat> char_format;
-    ciface::ExpressionParser::Lexer lexer(text.toStdString());
 
-    std::vector<ciface::ExpressionParser::Token> tokens;
+    switch (token.type)
-    lexer.Tokenize(tokens);
-
-    using ciface::ExpressionParser::TokenType;
-
-    for (auto& token : tokens)
     {
-      switch (token.type)
+    case TokenType::TOK_INVALID:
-      {
+      char_format = GetInvalidCharFormat();
-      case TokenType::TOK_INVALID:
+      break;
-        setFormat(token.string_position, token.string_length, GetInvalidCharFormat());
+    case TokenType::TOK_LPAREN:
-        break;
+    case TokenType::TOK_RPAREN:
-      case TokenType::TOK_LPAREN:
+    case TokenType::TOK_COMMA:
-      case TokenType::TOK_RPAREN:
+      char_format = GetSpecialCharFormat();
-      case TokenType::TOK_COMMA:
+      break;
-        setFormat(token.string_position, token.string_length, GetSpecialCharFormat());
+    case TokenType::TOK_LITERAL:
-        break;
+      char_format = GetLiteralCharFormat();
-      case TokenType::TOK_LITERAL:
+      break;
-        setFormat(token.string_position, token.string_length, GetLiteralCharFormat());
+    case TokenType::TOK_CONTROL:
-        break;
+      char_format = GetControlCharFormat();
-      case TokenType::TOK_CONTROL:
+      break;
-        setFormat(token.string_position, token.string_length, GetControlCharFormat());
+    case TokenType::TOK_FUNCTION:
-        break;
+      char_format = GetFunctionCharFormat();
-      case TokenType::TOK_FUNCTION:
+      break;
-        setFormat(token.string_position, token.string_length, GetFunctionCharFormat());
+    case TokenType::TOK_VARIABLE:
-        break;
+      char_format = GetVariableCharFormat();
-      case TokenType::TOK_VARIABLE:
+      break;
-        setFormat(token.string_position, token.string_length, GetVariableCharFormat());
+    default:
-        break;
+      if (token.IsBinaryOperator())
-      default:
+        char_format = GetOperatorCharFormat();
-        if (token.IsBinaryOperator())
+      break;
-          setFormat(token.string_position, token.string_length, GetOperatorCharFormat());
+    }
-        break;
+
-      }
+    if (char_format.has_value())
+      setFormat(int(token.string_position), int(token.string_length), *char_format);
+  }
+
+  if (ciface::ExpressionParser::ParseStatus::Successful != tokenize_status)
+  {
+    m_result_text->setText(tr("Invalid Token."));
+  }
+  else
+  {
+    const auto parse_status = ciface::ExpressionParser::ParseTokens(tokens);
+
+    m_result_text->setText(
+        QString::fromStdString(parse_status.description.value_or(_trans("Success."))));
+
+    if (ciface::ExpressionParser::ParseStatus::Successful != parse_status.status)
+    {
+      const auto token = *parse_status.token;
+      setFormat(int(token.string_position), int(token.string_length), GetInvalidCharFormat());
     }
   }
-};
+}
-
-}  // namespace
 
 IOWindow::IOWindow(QWidget* parent, ControllerEmu::EmulatedController* controller,
                    ControlReference* ref, IOWindow::Type type)
@@ -165,9 +189,12 @@ void IOWindow::CreateMainLayout()
   m_range_slider = new QSlider(Qt::Horizontal);
   m_range_spinbox = new QSpinBox();
 
+  m_parse_text = new QLineEdit();
+  m_parse_text->setReadOnly(true);
+
   m_expression_text = new QPlainTextEdit();
   m_expression_text->setFont(QFontDatabase::systemFont(QFontDatabase::FixedFont));
-  new SyntaxHighlighter(m_expression_text->document());
+  new ControlExpressionSyntaxHighlighter(m_expression_text->document(), m_parse_text);
 
   m_operators_combo = new QComboBox();
   m_operators_combo->addItem(tr("Operators"));
@@ -234,6 +261,7 @@ void IOWindow::CreateMainLayout()
 
   m_main_layout->addLayout(hbox, 2);
   m_main_layout->addWidget(m_expression_text, 1);
+  m_main_layout->addWidget(m_parse_text);
 
   // Button Box
   m_main_layout->addWidget(m_button_box);

Source/Core/DolphinQt/Config/Mapping/IOWindow.h

@@ -6,6 +6,7 @@
 
 #include <QDialog>
 #include <QString>
+#include <QSyntaxHighlighter>
 
 #include "Common/Flag.h"
 #include "InputCommon/ControllerInterface/Device.h"
@@ -14,6 +15,7 @@ class ControlReference;
 class QAbstractButton;
 class QComboBox;
 class QDialogButtonBox;
+class QLineEdit;
 class QListWidget;
 class QVBoxLayout;
 class QWidget;
@@ -27,6 +29,19 @@ namespace ControllerEmu
 class EmulatedController;
 }
 
+class ControlExpressionSyntaxHighlighter final : public QSyntaxHighlighter
+{
+  Q_OBJECT
+public:
+  ControlExpressionSyntaxHighlighter(QTextDocument* parent, QLineEdit* result);
+
+protected:
+  void highlightBlock(const QString& text) final override;
+
+private:
+  QLineEdit* const m_result_text;
+};
+
 class IOWindow final : public QDialog
 {
   Q_OBJECT
@@ -81,6 +96,7 @@ private:
 
   // Textarea
   QPlainTextEdit* m_expression_text;
+  QLineEdit* m_parse_text;
 
   // Buttonbox
   QDialogButtonBox* m_button_box;

Source/Core/InputCommon/ControlReference/ControlReference.cpp

@@ -54,7 +54,9 @@ std::string ControlReference::GetExpression() const
 void ControlReference::SetExpression(std::string expr)
 {
   m_expression = std::move(expr);
-  std::tie(m_parse_status, m_parsed_expression) = ParseExpression(m_expression);
+  auto parse_result = ParseExpression(m_expression);
+  m_parse_status = parse_result.status;
+  m_parsed_expression = std::move(parse_result.expr);
 }
 
 ControlReference::ControlReference() : range(1), m_parsed_expression(nullptr)

Source/Core/InputCommon/ControlReference/ExpressionParser.cpp

@@ -11,6 +11,7 @@
 #include <utility>
 #include <vector>
 
+#include "Common/Common.h"
 #include "Common/StringUtil.h"
 
 #include "InputCommon/ControlReference/ExpressionParser.h"
@@ -138,7 +139,7 @@ std::string Lexer::FetchWordChars()
     return "";
 
   // Valid word characters:
-  std::regex rx("[a-z0-9_]", std::regex_constants::icase);
+  std::regex rx(R"([a-z\d_])", std::regex_constants::icase);
 
   return FetchCharsWhile([&rx](char c) { return std::regex_match(std::string(1, c), rx); });
 }
@@ -180,7 +181,10 @@ Token Lexer::GetRealLiteral(char c)
   value += c;
   value += FetchCharsWhile([](char c) { return isdigit(c, std::locale::classic()) || ('.' == c); });
 
-  return Token(TOK_LITERAL, value);
+  if (std::regex_match(value, std::regex(R"(\d+(\.\d+)?)")))
+    return Token(TOK_LITERAL, value);
+
+  return Token(TOK_INVALID);
 }
 
 Token Lexer::NextToken()
@@ -267,8 +271,7 @@ ParseStatus Lexer::Tokenize(std::vector<Token>& tokens)
 class ControlExpression : public Expression
 {
 public:
-  // Keep a shared_ptr to the device so the control pointer doesn't become invalid
+  // Keep a shared_ptr to the device so the control pointer doesn't become invalid.
-  // TODO: This is causing devices to be destructed after backends are shutdown:
   std::shared_ptr<Device> m_device;
 
   explicit ControlExpression(ControlQualifier qualifier_) : qualifier(qualifier_) {}
@@ -384,7 +387,7 @@ public:
 
   operator std::string() const override
   {
-    return OpName(op) + "(" + (std::string)(*lhs) + ", " + (std::string)(*rhs) + ")";
+    return OpName(op) + "(" + std::string(*lhs) + ", " + std::string(*rhs) + ")";
   }
 };
 
@@ -422,12 +425,13 @@ private:
   const ControlState m_value{};
 };
 
-std::unique_ptr<LiteralExpression> MakeLiteralExpression(std::string name)
+ParseResult MakeLiteralExpression(Token token)
 {
-  // If TryParse fails we'll just get a Zero.
   ControlState val{};
-  TryParse(name, &val);
+  if (TryParse(token.data, &val))
-  return std::make_unique<LiteralReal>(val);
+    return ParseResult::MakeSuccessfulResult(std::make_unique<LiteralReal>(val));
+  else
+    return ParseResult::MakeErrorResult(token, _trans("Invalid literal."));
 }
 
 class VariableExpression : public Expression
@@ -520,45 +524,63 @@ ControlState* ControlEnvironment::GetVariablePtr(const std::string& name)
   return &m_variables[name];
 }
 
-struct ParseResult
+ParseResult ParseResult::MakeEmptyResult()
 {
-  ParseResult(ParseStatus status_, std::unique_ptr<Expression>&& expr_ = {})
+  ParseResult result;
-      : status(status_), expr(std::move(expr_))
+  result.status = ParseStatus::EmptyExpression;
-  {
+  return result;
-  }
+}
 
-  ParseStatus status;
+ParseResult ParseResult::MakeSuccessfulResult(std::unique_ptr<Expression>&& expr)
-  std::unique_ptr<Expression> expr;
+{
-};
+  ParseResult result;
+  result.status = ParseStatus::Successful;
+  result.expr = std::move(expr);
+  return result;
+}
+
+ParseResult ParseResult::MakeErrorResult(Token token, std::string description)
+{
+  ParseResult result;
+  result.status = ParseStatus::SyntaxError;
+  result.token = std::move(token);
+  result.description = std::move(description);
+  return result;
+}
 
 class Parser
 {
 public:
-  explicit Parser(std::vector<Token> tokens_) : tokens(tokens_) { m_it = tokens.begin(); }
+  explicit Parser(const std::vector<Token>& tokens_) : tokens(tokens_) { m_it = tokens.begin(); }
   ParseResult Parse()
   {
     ParseResult result = ParseToplevel();
 
+    if (ParseStatus::Successful != result.status)
+      return result;
+
     if (Peek().type == TOK_EOF)
       return result;
 
-    return {ParseStatus::SyntaxError};
+    return ParseResult::MakeErrorResult(Peek(), _trans("Expected EOF."));
   }
 
 private:
   struct FunctionArguments
   {
-    FunctionArguments(ParseStatus status_, std::vector<std::unique_ptr<Expression>>&& args_ = {})
+    FunctionArguments(ParseResult&& result_, std::vector<std::unique_ptr<Expression>>&& args_ = {})
-        : status(status_), args(std::move(args_))
+        : result(std::move(result_)), args(std::move(args_))
     {
     }
 
-    ParseStatus status;
+    // Note: expression member isn't being used.
+    ParseResult result;
+
     std::vector<std::unique_ptr<Expression>> args;
   };
 
-  std::vector<Token> tokens;
+  const std::vector<Token>& tokens;
-  std::vector<Token>::iterator m_it;
+  std::vector<Token>::const_iterator m_it;
 
   Token Chew()
   {
@@ -585,10 +607,10 @@ private:
       // Single argument with no parens (useful for unary ! function)
       auto arg = ParseAtom(Chew());
       if (ParseStatus::Successful != arg.status)
-        return {ParseStatus::SyntaxError};
+        return {std::move(arg)};
 
       args.emplace_back(std::move(arg.expr));
-      return {ParseStatus::Successful, std::move(args)};
+      return {ParseResult::MakeSuccessfulResult({}), std::move(args)};
     }
 
     // Chew the L-Paren
@@ -598,7 +620,7 @@ private:
     if (TOK_RPAREN == Peek().type)
     {
       Chew();
-      return {ParseStatus::Successful};
+      return {ParseResult::MakeSuccessfulResult({})};
     }
 
     while (true)
@@ -607,18 +629,18 @@ private:
       // Grab an expression, but stop at comma.
       auto arg = ParseBinary(BinaryOperatorPrecedence(TOK_COMMA));
       if (ParseStatus::Successful != arg.status)
-        return {ParseStatus::SyntaxError};
+        return {std::move(arg)};
 
       args.emplace_back(std::move(arg.expr));
 
       // Right paren is the end of our arguments.
       const Token tok = Chew();
       if (TOK_RPAREN == tok.type)
-        return {ParseStatus::Successful, std::move(args)};
+        return {ParseResult::MakeSuccessfulResult({}), std::move(args)};
 
       // Comma before the next argument.
       if (TOK_COMMA != tok.type)
-        return {ParseStatus::SyntaxError};
+        return {ParseResult::MakeErrorResult(tok, _trans("Expected comma."))};
     }
   }
 
@@ -629,29 +651,36 @@ private:
     case TOK_FUNCTION:
     {
       auto func = MakeFunctionExpression(tok.data);
+
+      if (!func)
+        return ParseResult::MakeErrorResult(tok, _trans("Unknown function."));
+
       auto args = ParseFunctionArguments();
 
-      if (ParseStatus::Successful != args.status)
+      if (ParseStatus::Successful != args.result.status)
-        return {ParseStatus::SyntaxError};
+        return std::move(args.result);
 
       if (!func->SetArguments(std::move(args.args)))
-        return {ParseStatus::SyntaxError};
+      {
+        // TODO: It would be nice to output how many arguments are expected.
+        return ParseResult::MakeErrorResult(tok, _trans("Wrong number of arguments."));
+      }
 
-      return {ParseStatus::Successful, std::move(func)};
+      return ParseResult::MakeSuccessfulResult(std::move(func));
     }
     case TOK_CONTROL:
     {
       ControlQualifier cq;
       cq.FromString(tok.data);
-      return {ParseStatus::Successful, std::make_unique<ControlExpression>(cq)};
+      return ParseResult::MakeSuccessfulResult(std::make_unique<ControlExpression>(cq));
     }
     case TOK_LITERAL:
     {
-      return {ParseStatus::Successful, MakeLiteralExpression(tok.data)};
+      return MakeLiteralExpression(tok);
     }
     case TOK_VARIABLE:
     {
-      return {ParseStatus::Successful, std::make_unique<VariableExpression>(tok.data)};
+      return ParseResult::MakeSuccessfulResult(std::make_unique<VariableExpression>(tok.data));
     }
     case TOK_LPAREN:
     {
@@ -661,10 +690,15 @@ private:
     {
       // An atom was expected but we got a subtraction symbol.
       // Interpret it as a unary minus function.
-      return ParseAtom(Token(TOK_FUNCTION, "minus"));
+
+      // Make sure to copy the existing string position values for proper error results.
+      Token func = tok;
+      func.type = TOK_FUNCTION;
+      func.data = "minus";
+      return ParseAtom(std::move(func));
     }
     default:
-      return {ParseStatus::SyntaxError};
+      return ParseResult::MakeErrorResult(tok, _trans("Expected start of expression."));
     }
   }
 
@@ -718,7 +752,7 @@ private:
       expr = std::make_unique<BinaryExpression>(tok.type, std::move(expr), std::move(rhs.expr));
     }
 
-    return {ParseStatus::Successful, std::move(expr)};
+    return ParseResult::MakeSuccessfulResult(std::move(expr));
   }
 
   ParseResult ParseParens()
@@ -728,9 +762,10 @@ private:
     if (result.status != ParseStatus::Successful)
       return result;
 
-    if (!Expects(TOK_RPAREN))
+    const auto rparen = Chew();
+    if (rparen.type != TOK_RPAREN)
     {
-      return {ParseStatus::SyntaxError};
+      return ParseResult::MakeErrorResult(rparen, _trans("Expected closing paren."));
     }
 
     return result;
@@ -739,15 +774,20 @@ private:
   ParseResult ParseToplevel() { return ParseBinary(); }
 };  // namespace ExpressionParser
 
+ParseResult ParseTokens(const std::vector<Token>& tokens)
+{
+  return Parser(tokens).Parse();
+}
+
 static ParseResult ParseComplexExpression(const std::string& str)
 {
   Lexer l(str);
   std::vector<Token> tokens;
-  ParseStatus tokenize_status = l.Tokenize(tokens);
+  const ParseStatus tokenize_status = l.Tokenize(tokens);
   if (tokenize_status != ParseStatus::Successful)
-    return {tokenize_status};
+    return ParseResult::MakeErrorResult(Token(TOK_INVALID), _trans("Tokenizing failed."));
 
-  return Parser(std::move(tokens)).Parse();
+  return ParseTokens(tokens);
 }
 
 static std::unique_ptr<Expression> ParseBarewordExpression(const std::string& str)
@@ -759,21 +799,24 @@ static std::unique_ptr<Expression> ParseBarewordExpression(const std::string& st
   return std::make_unique<ControlExpression>(qualifier);
 }
 
-std::pair<ParseStatus, std::unique_ptr<Expression>> ParseExpression(const std::string& str)
+ParseResult ParseExpression(const std::string& str)
 {
   if (StripSpaces(str).empty())
-    return std::make_pair(ParseStatus::EmptyExpression, nullptr);
+    return ParseResult::MakeEmptyResult();
 
   auto bareword_expr = ParseBarewordExpression(str);
   ParseResult complex_result = ParseComplexExpression(str);
 
   if (complex_result.status != ParseStatus::Successful)
   {
-    return std::make_pair(complex_result.status, std::move(bareword_expr));
+    // This is a bit odd.
+    // Return the error status of the complex expression with the fallback barewords expression.
+    complex_result.expr = std::move(bareword_expr);
+    return complex_result;
   }
 
-  auto combined_expr = std::make_unique<CoalesceExpression>(std::move(bareword_expr),
+  complex_result.expr = std::make_unique<CoalesceExpression>(std::move(bareword_expr),
-                                                            std::move(complex_result.expr));
+                                                             std::move(complex_result.expr));
-  return std::make_pair(complex_result.status, std::move(combined_expr));
+  return complex_result;
 }
 }  // namespace ciface::ExpressionParser

Source/Core/InputCommon/ControlReference/ExpressionParser.h

@@ -6,6 +6,7 @@
 
 #include <map>
 #include <memory>
+#include <optional>
 #include <string>
 
 #include "InputCommon/ControllerInterface/Device.h"
@@ -49,7 +50,7 @@ public:
   std::size_t string_position = 0;
   std::size_t string_length = 0;
 
-  Token(TokenType type_);
+  explicit Token(TokenType type_);
   Token(TokenType type_, std::string data_);
 
   bool IsBinaryOperator() const;
@@ -166,5 +167,26 @@ public:
   virtual operator std::string() const = 0;
 };
 
-std::pair<ParseStatus, std::unique_ptr<Expression>> ParseExpression(const std::string& expr);
+class ParseResult
+{
+public:
+  static ParseResult MakeEmptyResult();
+  static ParseResult MakeSuccessfulResult(std::unique_ptr<Expression>&& expr);
+  static ParseResult MakeErrorResult(Token token, std::string description);
+
+  ParseStatus status;
+  std::unique_ptr<Expression> expr;
+
+  // Used for parse errors:
+  // TODO: This should probably be moved elsewhere:
+  std::optional<Token> token;
+  std::optional<std::string> description;
+
+private:
+  ParseResult() = default;
+};
+
+ParseResult ParseExpression(const std::string& expr);
+ParseResult ParseTokens(const std::vector<Token>& tokens);
+
 }  // namespace ciface::ExpressionParser

Source/Core/InputCommon/ControlReference/FunctionExpression.cpp

@@ -17,19 +17,6 @@ constexpr ControlState CONDITION_THRESHOLD = 0.5;
 using Clock = std::chrono::steady_clock;
 using FSec = std::chrono::duration<ControlState>;
 
-// TODO: Return an oscillating value to make it apparent something was spelled wrong?
-class UnknownFunctionExpression : public FunctionExpression
-{
-private:
-  virtual bool ValidateArguments(const std::vector<std::unique_ptr<Expression>>& args) override
-  {
-    return false;
-  }
-  ControlState GetValue() const override { return 0.0; }
-  void SetValue(ControlState value) override {}
-  std::string GetFuncName() const override { return "unknown"; }
-};
-
 // usage: !toggle(toggle_state_input, [clear_state_input])
 class ToggleExpression : public FunctionExpression
 {
@@ -503,7 +490,7 @@ std::unique_ptr<FunctionExpression> MakeFunctionExpression(std::string name)
   else if ("pulse" == name)
     return std::make_unique<PulseExpression>();
   else
-    return std::make_unique<UnknownFunctionExpression>();
+    return nullptr;
 }
 
 int FunctionExpression::CountNumControls() const