From 99c1a11218c41472eb27cc37ac14201a9c50b84c Mon Sep 17 00:00:00 2001
From: Daniel Demmel <hello@danieldemmel.me>
Date: Mon, 29 Jun 2026 22:53:03 +0100
Subject: [PATCH] Fix #58: highlight script attributes appearing before type

The inline_script rule matched the opening <script> tag with a greedy
`.*type=` that consumed every character up to the type attribute. Any
attribute placed before type (e.g. <script id="foo" type="...">) was
swallowed into a single unscoped token and lost its highlighting.

Replace the consuming `.*type=...` with a non-consuming lookahead that
only asserts a handlebars type exists somewhere in the tag, so the begin
match ends right after <script and the existing #tag-stuff rules
highlight every attribute regardless of order. The now-unused type/value
beginCaptures are removed.

Applied to all three grammar files. Adds regression tests covering both
attribute orders plus a guard that a plain text/javascript script is not
treated as a handlebars embedding.
---
 grammars/Handlebars.json           |  8 +-------
 grammars/Handlebars.sublime-syntax |  4 +---
 grammars/Handlebars.tmLanguage     | 12 +-----------
 test/embedding.test.js             | 30 ++++++++++++++++++++++++++++++
 4 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/grammars/Handlebars.json b/grammars/Handlebars.json
index 2a5ff3c..19c47b5 100644
--- a/grammars/Handlebars.json
+++ b/grammars/Handlebars.json
@@ -693,19 +693,13 @@
             ]
         },
         "inline_script": {
-            "begin": "(?:^\\s+)?(<)((?i:script))\\b(?:.*(type)=([\"'](?:text/x-handlebars-template|text/x-handlebars|text/template|x-tmpl-handlebars)[\"']))(?![^>]*/>)",
+            "begin": "(?:^\\s+)?(<)((?i:script))\\b(?=[^>]*\\stype=[\"'](?:text/x-handlebars-template|text/x-handlebars|text/template|x-tmpl-handlebars)[\"'])(?![^>]*/>)",
             "beginCaptures": {
                 "1": {
                     "name": "punctuation.definition.tag.html"
                 },
                 "2": {
                     "name": "entity.name.tag.script.html"
-                },
-                "3": {
-                    "name": "entity.other.attribute-name.html"
-                },
-                "4": {
-                    "name": "string.quoted.double.html"
                 }
             },
             "end": "(?<=</(script|SCRIPT))(>)(?:\\s*\\n)?",
diff --git a/grammars/Handlebars.sublime-syntax b/grammars/Handlebars.sublime-syntax
index ba2d35d..9fe0c61 100644
--- a/grammars/Handlebars.sublime-syntax
+++ b/grammars/Handlebars.sublime-syntax
@@ -336,12 +336,10 @@ contexts:
     - match: <
       scope: invalid.illegal.bad-angle-bracket.html
   inline_script:
-    - match: '(?:^\s+)?(<)((?i:script))\b(?:.*(type)=(["''](?:text/x-handlebars-template|text/x-handlebars|text/template|x-tmpl-handlebars)["'']))(?![^>]*/>)'
+    - match: '(?:^\s+)?(<)((?i:script))\b(?=[^>]*\stype=(["''](?:text/x-handlebars-template|text/x-handlebars|text/template|x-tmpl-handlebars)["'']))(?![^>]*/>)'
       captures:
         1: punctuation.definition.tag.html
         2: entity.name.tag.script.html
-        3: entity.other.attribute-name.html
-        4: string.quoted.double.html
       push:
         - meta_scope: source.handlebars.embedded.html
         - match: (?<=</(script|SCRIPT))(>)(?:\s*\n)?
diff --git a/grammars/Handlebars.tmLanguage b/grammars/Handlebars.tmLanguage
index 9d0c1f5..c09e0c8 100644
--- a/grammars/Handlebars.tmLanguage
+++ b/grammars/Handlebars.tmLanguage
@@ -928,7 +928,7 @@
 		<key>inline_script</key>
 		<dict>
 			<key>begin</key>
-			<string>(?:^\s+)?(&lt;)((?i:script))\b(?:.*(type)=(["'](?:text/x-handlebars-template|text/x-handlebars|text/template|x-tmpl-handlebars)["']))(?![^&gt;]*/&gt;)</string>
+			<string>(?:^\s+)?(&lt;)((?i:script))\b(?=[^&gt;]*\stype=(["'](?:text/x-handlebars-template|text/x-handlebars|text/template|x-tmpl-handlebars)["']))(?![^&gt;]*/&gt;)</string>
 			<key>beginCaptures</key>
 			<dict>
 				<key>1</key>
@@ -941,16 +941,6 @@
 					<key>name</key>
 					<string>entity.name.tag.script.html</string>
 				</dict>
-				<key>3</key>
-				<dict>
-					<key>name</key>
-					<string>entity.other.attribute-name.html</string>
-				</dict>
-				<key>4</key>
-				<dict>
-					<key>name</key>
-					<string>string.quoted.double.html</string>
-				</dict>
 			</dict>
 			<key>end</key>
 			<string>(?&lt;=&lt;/(script|SCRIPT))(&gt;)(?:\s*\n)?</string>
diff --git a/test/embedding.test.js b/test/embedding.test.js
index d6d2f65..808e138 100644
--- a/test/embedding.test.js
+++ b/test/embedding.test.js
@@ -54,6 +54,36 @@ test('inline <script> template embeds Handlebars', async () => {
   await assertScope(src, 'title', 'variable.parameter.handlebars');
 });
 
+// Regression for #58: an `id` (or any attribute) appearing BEFORE `type` in the
+// opening tag must still be highlighted. The old grammar consumed everything up
+// to `type=` with a greedy `.*`, swallowing the leading attributes into one
+// unscoped token. The fix asserts the handlebars `type` via a lookahead and lets
+// the #tag-stuff rules highlight every attribute regardless of order.
+test('#58: id attribute before type is still highlighted', async () => {
+  const src = '<script id="t" type="text/x-handlebars-template"></script>';
+  await assertScope(src, 'id', 'entity.other.attribute-name.id.html');
+  await assertScope(src, 't', 'string.quoted.double.handlebars');
+  // The script body is still recognised as an embedded handlebars template.
+  await assertScope(src, 'script', 'entity.name.tag.script.html');
+});
+
+test('#58: id attribute after type is still highlighted (no regression)', async () => {
+  const src = '<script type="text/x-handlebars-template" id="t"></script>';
+  await assertScope(src, 'id', 'entity.other.attribute-name.id.html');
+  await assertScope(src, 't', 'string.quoted.double.handlebars');
+});
+
+test('#58: a plain text/javascript script is NOT a handlebars template', async () => {
+  // The lookahead must not fire for a non-handlebars type; the body should fall
+  // through to source.js, not the handlebars embedding.
+  const src = '<script id="x" type="text/javascript">';
+  const idTok = (await tokenizeLines(src)).flat().find((t) => t.text === 'id');
+  assert.ok(
+    !idTok.scopes.includes('source.handlebars.embedded.html'),
+    `plain JS script should not be a handlebars embedding: ${JSON.stringify(idTok.scopes)}`
+  );
+});
+
 test('plain HTML outside any script tag is NOT treated as embedded', async () => {
   const src = '<p>{{body}}</p>';
   const tok = (await tokenizeLines(src)).flat().find((t) => t.text === 'p');