Merge pull request #13280 from jordan-woyak/input-expressions-highlighting

InputCommon/DolphinQt: Fix sometimes broken syntax highlighting in IOWindow.

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

@@ -19,6 +19,7 @@
 #include <QSlider>
 #include <QSpinBox>
 #include <QTableWidget>
+#include <QTextBlock>
 #include <QTimer>
 #include <QVBoxLayout>
 
@@ -111,8 +112,9 @@ QTextCharFormat GetCommentCharFormat()
 }  // namespace
 
 ControlExpressionSyntaxHighlighter::ControlExpressionSyntaxHighlighter(QTextDocument* parent)
-    : QSyntaxHighlighter(parent)
+    : QObject(parent)
 {
+  connect(parent, &QTextDocument::contentsChanged, this, [this, parent]() { Highlight(parent); });
 }
 
 void QComboBoxWithMouseWheelDisabled::wheelEvent(QWheelEvent* event)
@@ -120,39 +122,32 @@ void QComboBoxWithMouseWheelDisabled::wheelEvent(QWheelEvent* event)
   // Do nothing
 }
 
-void ControlExpressionSyntaxHighlighter::highlightBlock(const QString&)
+void ControlExpressionSyntaxHighlighter::Highlight(QTextDocument* document)
 {
-  // TODO: This is going to result in improper highlighting with non-ascii characters:
-  ciface::ExpressionParser::Lexer lexer(document()->toPlainText().toStdString());
+  // toLatin1 converts multi-byte unicode characters to a single-byte character,
+  // so Token string_position values are the character counts that Qt's FormatRange expects.
+  ciface::ExpressionParser::Lexer lexer(document->toPlainText().toLatin1().toStdString());
 
   std::vector<ciface::ExpressionParser::Token> tokens;
   const auto tokenize_status = lexer.Tokenize(tokens);
 
-  using ciface::ExpressionParser::TokenType;
-
-  const auto set_block_format = [this](int start, int count, const QTextCharFormat& format) {
-    if (start + count <= currentBlock().position() ||
-        start >= currentBlock().position() + currentBlock().length())
+  if (ciface::ExpressionParser::ParseStatus::Successful == tokenize_status)
   {
-      // This range is not within the current block.
-      return;
+    const auto parse_status = ciface::ExpressionParser::ParseTokens(tokens);
+    if (ciface::ExpressionParser::ParseStatus::Successful != parse_status.status)
+    {
+      auto token = *parse_status.token;
+      // Add invalid version of token where parsing failed for appropriate error-highlighting.
+      token.type = ciface::ExpressionParser::TOK_INVALID;
+      tokens.emplace_back(token);
+    }
   }
 
-    int block_start = start - currentBlock().position();
-
-    if (block_start < 0)
-    {
-      count += block_start;
-      block_start = 0;
-    }
-
-    setFormat(block_start, count, format);
-  };
-
-  for (auto& token : tokens)
-  {
+  auto get_token_char_format = [](const ciface::ExpressionParser::Token& token) {
     std::optional<QTextCharFormat> char_format;
 
+    using ciface::ExpressionParser::TokenType;
+
     switch (token.type)
     {
     case TokenType::TOK_INVALID:
@@ -186,22 +181,50 @@ void ControlExpressionSyntaxHighlighter::highlightBlock(const QString&)
       break;
     }
 
+    return char_format;
+  };
+
+  // FYI, formatting needs to be done at the block level to prevent altering of undo/redo history.
+  for (QTextBlock block = document->begin(); block.isValid(); block = block.next())
+  {
+    block.layout()->clearFormats();
+
+    const int block_position = block.position();
+    const int block_length = block_position + block.length();
+
+    QList<QTextLayout::FormatRange> format_ranges;
+
+    for (auto& token : tokens)
+    {
+      int token_length = int(token.string_length);
+      int token_start = int(token.string_position) - block_position;
+      if (token_start < 0)
+      {
+        token_length += token_start;
+        token_start = 0;
+      }
+
+      if (token_length <= 0)
+      {
+        // Token is in a previous block.
+        continue;
+      }
+
+      if (token_start >= block_length)
+      {
+        // Token is in a following block.
+        break;
+      }
+
+      const auto char_format = get_token_char_format(token);
       if (char_format.has_value())
-      set_block_format(int(token.string_position), int(token.string_length), *char_format);
+      {
+        format_ranges.emplace_back(QTextLayout::FormatRange{
+            .start = token_start, .length = token_length, .format = *char_format});
+      }
     }
 
-  // This doesn't need to be run for every "block", but it works.
-  if (ciface::ExpressionParser::ParseStatus::Successful == tokenize_status)
-  {
-    ciface::ExpressionParser::RemoveInertTokens(&tokens);
-    const auto parse_status = ciface::ExpressionParser::ParseTokens(tokens);
-
-    if (ciface::ExpressionParser::ParseStatus::Successful != parse_status.status)
-    {
-      const auto token = *parse_status.token;
-      set_block_format(int(token.string_position), int(token.string_length),
-                       GetInvalidCharFormat());
-    }
+    block.layout()->setFormats(format_ranges);
   }
 }
 

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

@@ -10,7 +10,6 @@
 #include <QComboBox>
 #include <QDialog>
 #include <QString>
-#include <QSyntaxHighlighter>
 
 #include "InputCommon/ControllerInterface/CoreDevice.h"
 
@@ -26,6 +25,7 @@ class QPlainTextEdit;
 class QPushButton;
 class QSlider;
 class QSpinBox;
+class QTextDocument;
 
 namespace ControllerEmu
 {
@@ -34,14 +34,14 @@ class EmulatedController;
 
 class InputStateLineEdit;
 
-class ControlExpressionSyntaxHighlighter final : public QSyntaxHighlighter
+class ControlExpressionSyntaxHighlighter final : public QObject
 {
   Q_OBJECT
 public:
   explicit ControlExpressionSyntaxHighlighter(QTextDocument* parent);
 
-protected:
-  void highlightBlock(const QString& text) final override;
+private:
+  void Highlight(QTextDocument* text_edit);
 };
 
 class QComboBoxWithMouseWheelDisabled : public QComboBox

Source/Core/InputCommon/ControlReference/ExpressionParser.cpp

@@ -188,7 +188,19 @@ Token Lexer::NextToken()
   case '*':
     return Token(TOK_MUL);
   case '/':
+  {
+    // Handle /* */ style comments.
+    if (it != expr.end() && *it == '*')
+    {
+      ++it;
+      const auto end_of_comment = expr.find("*/", it - expr.begin());
+      if (end_of_comment == std::string::npos)
+        return Token(TOK_INVALID);
+      it = expr.begin() + end_of_comment + 2;
+      return Token(TOK_COMMENT);
+    }
     return Token(TOK_DIV);
+  }
   case '%':
     return Token(TOK_MOD);
   case '=':
@@ -221,26 +233,10 @@ ParseStatus Lexer::Tokenize(std::vector<Token>& tokens)
 {
   while (true)
   {
-    const std::size_t string_position = it - expr.begin();
+    const std::string::iterator prev_it = it;
     Token tok = NextToken();
-
-    tok.string_position = string_position;
-    tok.string_length = it - expr.begin();
-
-    // Handle /* */ style comments.
-    if (tok.type == TOK_DIV && PeekToken().type == TOK_MUL)
-    {
-      const auto end_of_comment = expr.find("*/", it - expr.begin());
-
-      if (end_of_comment == std::string::npos)
-        return ParseStatus::SyntaxError;
-
-      tok.type = TOK_COMMENT;
-      tok.string_length = end_of_comment + 4;
-
-      it = expr.begin() + end_of_comment + 2;
-    }
-
+    tok.string_position = prev_it - expr.begin();
+    tok.string_length = it - prev_it;
     tokens.push_back(tok);
 
     if (tok.type == TOK_INVALID)
@@ -682,6 +678,11 @@ ParseResult ParseResult::MakeErrorResult(Token token, std::string description)
   return result;
 }
 
+bool IsInertToken(const Token& tok)
+{
+  return tok.type == TOK_COMMENT || tok.type == TOK_WHITESPACE;
+}
+
 class Parser
 {
 public:
@@ -711,7 +712,12 @@ private:
     return tok;
   }
 
-  Token Peek() { return *m_it; }
+  Token Peek()
+  {
+    while (IsInertToken(*m_it))
+      ++m_it;
+    return *m_it;
+  }
 
   bool Expects(TokenType type)
   {
@@ -1000,18 +1006,9 @@ static ParseResult ParseComplexExpression(const std::string& str)
   if (tokenize_status != ParseStatus::Successful)
     return ParseResult::MakeErrorResult(Token(TOK_INVALID),
                                         Common::GetStringT("Tokenizing failed."));
-
-  RemoveInertTokens(&tokens);
   return ParseTokens(tokens);
 }
 
-void RemoveInertTokens(std::vector<Token>* tokens)
-{
-  std::erase_if(*tokens, [](const Token& tok) {
-    return tok.type == TOK_COMMENT || tok.type == TOK_WHITESPACE;
-  });
-}
-
 static std::unique_ptr<Expression> ParseBarewordExpression(const std::string& str)
 {
   ControlQualifier qualifier;

Source/Core/InputCommon/ControlReference/ExpressionParser.h

@@ -197,6 +197,5 @@ private:
 
 ParseResult ParseExpression(const std::string& expr);
 ParseResult ParseTokens(const std::vector<Token>& tokens);
-void RemoveInertTokens(std::vector<Token>* tokens);
 
 }  // namespace ciface::ExpressionParser