fix(syntax/#2985): PHP syntax highlights not showing (#2997)

__Issue:__ PHP syntax highlights are no longer showing by default __Defect:__ #2889 upgraded the extensions, but the PHP extension has a new dependency on `embeddedLanguages`, which our current textmate implementation does not handle correctly __Fix:__ Remove the `embeddedLanguages` for now - add a regression / canary test to ensure we don't break this in the future. Fixes #2985 Tracking `embeddedLanguage` support in #2998
onivim · Jan 15, 2021 · a4661f7 · a4661f7
1 parent 06493ff
commit a4661f7
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 29 deletions.
diff --git a/CHANGES_CURRENT.md b/CHANGES_CURRENT.md
@@ -57,6 +57,7 @@
 - #2990 - Signature Help: Fix blocking `esc` key press back to normal mode
 - #2991 - OSX: Fix shortcut keys double-triggering events
 - #2993 - CodeLens: Handle null command id & label icons
+- #2997 - Syntax: Fix regression in syntax highlighting for PHP (fixes #2985)
 - #2995 - Extensions: Fix bug with 3-param http/https request (fixes #2981)
 
 ### Performance

diff --git a/extensions/php/package.json b/extensions/php/package.json
@@ -35,20 +35,6 @@
 				"language": "php",
 				"scopeName": "source.php",
 				"path": "./syntaxes/php.tmLanguage.json"
-			},
-			{
-				"language": "php",
-				"scopeName": "text.html.php",
-				"path": "./syntaxes/html.tmLanguage.json",
-				"embeddedLanguages": {
-					"text.html": "html",
-					"source.php": "php",
-					"source.sql": "sql",
-					"text.xml": "xml",
-					"source.js": "javascript",
-					"source.json": "json",
-					"source.css": "css"
-				}
 			}
 		],
 		"snippets": [

diff --git a/integration_test/SyntaxHighlightPhpTest.re b/integration_test/SyntaxHighlightPhpTest.re
@@ -0,0 +1,60 @@
+open Oni_Core;
+open Oni_Model;
+open Oni_IntegrationTestLib;
+
+let uniqueColorCount: list(ThemeToken.t) => int =
+  tokens => {
+    let rec loop = (uniqueCount, maybeLastForeground, tokens) => {
+      switch (tokens) {
+      | [] => uniqueCount
+      | [(hd: ThemeToken.t), ...tail] =>
+        switch (maybeLastForeground) {
+        | None => loop(uniqueCount + 1, Some(hd.foregroundColor), tail)
+        | Some(color) =>
+          if (Revery.Color.equals(color, hd.foregroundColor)) {
+            loop(uniqueCount, Some(hd.foregroundColor), tail);
+          } else {
+            loop(uniqueCount + 1, Some(hd.foregroundColor), tail);
+          }
+        }
+      };
+    };
+
+    loop(0, None, tokens);
+  };
+
+// Integration test - verify we get highlights for PHP
+// Regression test for #2985
+runTest(~name="SyntaxHighlightPhpTest", ({dispatch, wait, _}) => {
+  wait(~name="Capture initial state", (state: State.t) =>
+    Selectors.mode(state) |> Vim.Mode.isNormal
+  );
+  wait(~name="Wait for syntax server", ~timeout=10.0, (state: State.t) => {
+    Feature_Syntax.isSyntaxServerRunning(state.syntaxHighlights)
+  });
+
+  let testFile = getAssetPath("test-syntax.php");
+
+  // Create a buffer
+  dispatch(Actions.OpenFileByPath(testFile, None, None));
+
+  // Wait for highlights to show up
+  wait(~name="Verify we get syntax highlights", (state: State.t) => {
+    state
+    |> Selectors.getActiveBuffer
+    |> Option.map(Buffer.getId)
+    |> Option.map(bufferId => {
+         let tokens =
+           Feature_Syntax.getTokens(
+             ~bufferId,
+             ~line=EditorCoreTypes.LineNumber.(zero),
+             state.syntaxHighlights,
+           );
+
+         let uniqueTokenColors = tokens |> uniqueColorCount;
+
+         uniqueTokenColors > 1;
+       })
+    |> Option.value(~default=false)
+  });
+});
diff --git a/integration_test/dune b/integration_test/dune
@@ -19,11 +19,11 @@
    RegressionCommandLineNoCompletionTest RegressionFontFallbackTest
    RegressionFileModifiedIndicationTest RegressionNonExistentDirectory
    RegressionVspEmptyInitialBufferTest RegressionVspEmptyExistingBufferTest
-   SCMGitTest SyntaxHighlightTextMateTest SyntaxHighlightTreesitterTest
-   AddRemoveSplitTest TerminalSetPidTitleTest TerminalConfigurationTest
-   TypingBatchedTest TypingUnbatchedTest VimSimpleRemapTest
-   VimlRemapCmdlineTest ClipboardChangeTest VimScriptLocalFunctionTest
-   ZenModeSingleFileModeTest ZenModeSplitTest)
+   SCMGitTest SyntaxHighlightPhpTest SyntaxHighlightTextMateTest
+   SyntaxHighlightTreesitterTest AddRemoveSplitTest TerminalSetPidTitleTest
+   TerminalConfigurationTest TypingBatchedTest TypingUnbatchedTest
+   VimSimpleRemapTest VimlRemapCmdlineTest ClipboardChangeTest
+   VimScriptLocalFunctionTest ZenModeSingleFileModeTest ZenModeSplitTest)
  (libraries Oni_CLI Oni_IntegrationTestLib reason-native-crash-utils.asan))
 
 (install
@@ -54,12 +54,12 @@
    SearchShowClearHighlightsTest.exe SyntaxHighlightTextMateTest.exe
    SyntaxHighlightTreesitterTest.exe SyntaxServer.exe
    SyntaxServerCloseTest.exe SyntaxServerMessageExceptionTest.exe
-   SyntaxServerParentPidTest.exe SyntaxServerParentPidCorrectTest.exe
-   SyntaxServerReadExceptionTest.exe TerminalSetPidTitleTest.exe
-   TerminalConfigurationTest.exe AddRemoveSplitTest.exe TypingBatchedTest.exe
-   TypingUnbatchedTest.exe VimScriptLocalFunctionTest.exe
-   VimSimpleRemapTest.exe VimlRemapCmdlineTest.exe
-   ZenModeSingleFileModeTest.exe ZenModeSplitTest.exe ClipboardChangeTest.exe
-   large-c-file.c lsan.supp some-test-file.json some-test-file.txt test.crlf
-   test.lf utf8.txt utf8-test-file.htm Inconsolata-Regular.ttf
-   PlugScriptLocal.vim))
+   SyntaxHighlightPhpTest.exe SyntaxServerParentPidTest.exe
+   SyntaxServerParentPidCorrectTest.exe SyntaxServerReadExceptionTest.exe
+   TerminalSetPidTitleTest.exe TerminalConfigurationTest.exe
+   AddRemoveSplitTest.exe TypingBatchedTest.exe TypingUnbatchedTest.exe
+   VimScriptLocalFunctionTest.exe VimSimpleRemapTest.exe
+   VimlRemapCmdlineTest.exe ZenModeSingleFileModeTest.exe
+   ZenModeSplitTest.exe ClipboardChangeTest.exe large-c-file.c lsan.supp
+   some-test-file.json some-test-file.txt test.crlf test.lf test-syntax.php
+   utf8.txt utf8-test-file.htm Inconsolata-Regular.ttf PlugScriptLocal.vim))
diff --git a/integration_test/test-syntax.php b/integration_test/test-syntax.php
@@ -0,0 +1 @@
+<?php echo "Hello, world!" ?>
diff --git a/src/Core/ThemeToken.re b/src/Core/ThemeToken.re
@@ -30,8 +30,8 @@ let toString =
   Printf.sprintf(
     "ColorizedToken - index: %d foreground: %s background: %s scope: %s bold: %b italic: %b",
     index,
-    backgroundColor |> Revery.Color.toString,
     foregroundColor |> Revery.Color.toString,
+    backgroundColor |> Revery.Color.toString,
     syntaxScope |> SyntaxScope.toString,
     bold,
     italic,