Skip to content

Commit a97000b

Browse files
committed
fix(highlight): address Copilot review comments on PR #80
- detectLang: perform extension map lookup before Dockerfile check to prevent false positives (e.g. Dockerfile.ts correctly detected as TypeScript, Dockerfile.php as PHP) - PHP tokenizer: use /^#(?!\[)[^\n]*/ for hash comments to avoid dimming PHP 8+ attribute syntax (#[Route], #[ORM\Entity], etc.) - Test: assert \x1b[35m magenta on Terraform boolean/null literals (was only checking text preservation via strip()) - Test: add MAINTAINER to Dockerfile instructions test (present in regex but was missing from test list) - Test: add regression for Dockerfile.ts → typescript detection - Test: add PHP 8+ attribute non-comment assertion
1 parent f714b5b commit a97000b

2 files changed

Lines changed: 37 additions & 10 deletions

File tree

src/render/highlight.test.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,15 @@ describe("PHP tokenizer", () => {
525525
expect(strip(highlightFragment("# comment", [], "f.php").join(""))).toBe("# comment");
526526
});
527527

528+
it("does NOT dim PHP 8+ attribute syntax #[...] as a comment", () => {
529+
// Without the fix, '#[Route(...)]' is consumed as a single dim comment token.
530+
// With the fix, '#' falls through to the default dim rule, but 'Route'
531+
// is recognised as a PascalCase identifier → cyan (not part of a comment).
532+
const out = highlightFragment("#[Route('/')]", [], "f.php").join("");
533+
expect(out).toContain("\x1b[36m"); // cyan for PascalCase 'Route' — proves it's NOT swallowed by the comment rule
534+
expect(strip(out)).toBe("#[Route('/')]");
535+
});
536+
528537
it("colorizes block comments", () => {
529538
expect(strip(highlightFragment("/* block */", [], "f.php").join(""))).toBe("/* block */");
530539
});
@@ -679,7 +688,9 @@ describe("Terraform/HCL tokenizer", () => {
679688

680689
it("colorizes boolean literals", () => {
681690
for (const kw of ["true", "false", "null"]) {
682-
expect(strip(highlightFragment(kw, [], "f.tf").join(""))).toBe(kw);
691+
const out = highlightFragment(kw, [], "f.tf").join("");
692+
expect(out).toContain("\x1b[35m"); // magenta for boolean/null literals
693+
expect(strip(out)).toBe(kw);
683694
}
684695
});
685696

@@ -706,6 +717,14 @@ describe("Dockerfile tokenizer", () => {
706717
expect(out).toContain("\x1b[35m"); // pc.magenta for instructions
707718
});
708719

720+
it("does NOT mis-detect 'Dockerfile.ts' as Dockerfile — extension wins", () => {
721+
// 'const' is a TypeScript keyword → magenta via TS rules, not Dockerfile rules.
722+
// The key assertion is that the extension-based lookup takes priority.
723+
const out = highlightFragment("const", [], "Dockerfile.ts").join("");
724+
expect(out).toContain("\x1b[35m"); // magenta via TypeScript keywords, not Dockerfile
725+
expect(strip(out)).toBe("const");
726+
});
727+
709728
it("colorizes all standard Dockerfile instructions in magenta", () => {
710729
for (const instr of [
711730
"FROM",
@@ -725,6 +744,7 @@ describe("Dockerfile tokenizer", () => {
725744
"STOPSIGNAL",
726745
"HEALTHCHECK",
727746
"SHELL",
747+
"MAINTAINER", // deprecated but still present in the regex
728748
]) {
729749
const out = highlightFragment(instr, [], "Dockerfile").join("");
730750
expect(out).toContain("\x1b[35m"); // magenta

src/render/highlight.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,8 @@ type Lang =
2323
| "text";
2424

2525
function detectLang(filePath: string): Lang {
26-
// Dockerfile detection by filename (no extension)
27-
const base = filePath.split("/").pop() ?? "";
28-
const baseLower = base.toLowerCase();
29-
if (baseLower === "dockerfile" || baseLower.startsWith("dockerfile.")) {
30-
return "dockerfile";
31-
}
32-
26+
// Extension-based detection takes priority to avoid false positives
27+
// (e.g. Dockerfile.ts must be detected as typescript, not dockerfile).
3328
const ext = filePath.split(".").pop()?.toLowerCase() ?? "";
3429
const map: Record<string, Lang> = {
3530
ts: "typescript",
@@ -71,7 +66,18 @@ function detectLang(filePath: string): Lang {
7166
tf: "terraform",
7267
hcl: "terraform",
7368
};
74-
return map[ext] ?? "text";
69+
const langFromExt = map[ext];
70+
if (langFromExt) return langFromExt;
71+
72+
// Dockerfile detection by filename — only reached when no extension matched,
73+
// so Dockerfile.ts / Dockerfile.php are never misidentified.
74+
const base = filePath.split("/").pop() ?? "";
75+
const baseLower = base.toLowerCase();
76+
if (baseLower === "dockerfile" || baseLower.startsWith("dockerfile.")) {
77+
return "dockerfile";
78+
}
79+
80+
return "text";
7581
}
7682

7783
// ─── Syntax token rules ───────────────────────────────────────────────────────
@@ -199,7 +205,8 @@ const tokenRules: Partial<Record<Lang, TokenRule[]>> & {
199205
],
200206
php: [
201207
[/^\/\/[^\n]*/, (s) => pc.dim(s)],
202-
[/^#[^\n]*/, (s) => pc.dim(s)],
208+
// Fix: exclude PHP 8+ attribute syntax (#[Route(...)], #[ORM\Entity]) from being dimmed as comments
209+
[/^#(?!\[)[^\n]*/, (s) => pc.dim(s)],
203210
[/^\/\*[\s\S]*?\*\//, (s) => pc.dim(s)],
204211
[/^"(?:[^"\\]|\\.)*"/, (s) => pc.green(s)],
205212
[/^'(?:[^'\\]|\\.)*'/, (s) => pc.green(s)],

0 commit comments

Comments
 (0)