From f2a833dd131151d7b08fa757eeeb81725de6445d Mon Sep 17 00:00:00 2001 From: Michael Hackner Date: Tue, 18 Aug 2026 18:07:19 -0700 Subject: [PATCH] feat(diff): syntax highlighting for Bazel/Starlark Starlark has no grammar of its own, so map it to Python the way GitHub Linguist does. Reaching the renderer needed two lookup fixes as well: named extensionless files (`pkg/BUILD`, `docker/Dockerfile`) now resolve below the repository root, and the language Hunk resolves is pinned onto the metadata Pierre renders, whose own derivation cannot see mappings keyed by whole filenames. Co-authored-by: Claude Opus 5 --- .../nested-extensionless-highlighting.md | 5 +++ .changeset/starlark-highlighting.md | 5 +++ src/core/changeset/diffFile.test.ts | 18 ++++++++ src/core/changeset/diffFile.ts | 11 +++-- src/core/changeset/fileLanguage.test.ts | 36 ++++++++++++++++ src/core/changeset/fileLanguage.ts | 41 ++++++++++++++++--- src/core/changeset/fileLanguageLookup.ts | 20 +++++++-- 7 files changed, 124 insertions(+), 12 deletions(-) create mode 100644 .changeset/nested-extensionless-highlighting.md create mode 100644 .changeset/starlark-highlighting.md diff --git a/.changeset/nested-extensionless-highlighting.md b/.changeset/nested-extensionless-highlighting.md new file mode 100644 index 000000000..982d5d0d9 --- /dev/null +++ b/.changeset/nested-extensionless-highlighting.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Highlight extensionless named files such as `Dockerfile` and `Makefile` when they sit in a subdirectory, not only at the repository root. diff --git a/.changeset/starlark-highlighting.md b/.changeset/starlark-highlighting.md new file mode 100644 index 000000000..b6c5fd97e --- /dev/null +++ b/.changeset/starlark-highlighting.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": minor +--- + +Highlight Bazel and Starlark files with the Python grammar, matching how GitHub classifies Starlark. diff --git a/src/core/changeset/diffFile.test.ts b/src/core/changeset/diffFile.test.ts index 4312fe4d7..58a41d6c0 100644 --- a/src/core/changeset/diffFile.test.ts +++ b/src/core/changeset/diffFile.test.ts @@ -44,6 +44,24 @@ describe("buildDiffFile", () => { expect(ctsFile.language).toBe("typescript"); }); + test("pins the resolved language onto the metadata Pierre renders", () => { + // Pierre re-derives the language from the path unless the metadata names one, and its own + // derivation cannot resolve a whole filename nested in a directory. Without the override a + // nested `BUILD` renders as plain text even though `language` says otherwise. + const nested = buildDiffFile( + metadataFor("a\n", "b\n", "pkg/nested/BUILD"), + "P", + 0, + "src", + null, + ); + expect(nested.language).toBe("python"); + expect(nested.metadata.lang).toBe("python"); + + const plain = buildDiffFile(metadataFor("a\n", "b\n", "notes"), "P", 1, "src", null); + expect(plain.metadata.lang).toBe("text"); + }); + test("infers binary status from the patch when not given explicitly", () => { const binary = buildDiffFile(metadata, "Binary files a/x and b/x differ\n", 0, "src", null); expect(binary.isBinary).toBe(true); diff --git a/src/core/changeset/diffFile.ts b/src/core/changeset/diffFile.ts index cd944c9aa..5c1707ec1 100644 --- a/src/core/changeset/diffFile.ts +++ b/src/core/changeset/diffFile.ts @@ -1,4 +1,4 @@ -import { type FileDiffMetadata } from "@pierre/diffs"; +import { setLanguageOverride, type FileDiffMetadata } from "@pierre/diffs"; import { findSidecarFileContext } from "./sidecar"; import { patchLooksBinary } from "./binary"; import { fileLanguageForPath } from "./fileLanguageLookup"; @@ -68,6 +68,11 @@ export function buildDiffFile( ? (previousPath ?? normalizedMetadata.prevName) : (normalizeDiffPath(previousPath) ?? normalizedMetadata.prevName); const resolvedIsBinary = isBinary ?? patchLooksBinary(patch); + // Pierre's renderers re-derive the language from `metadata.name` unless the metadata carries an + // override, and that derivation cannot see Hunk's extension registrations for whole filenames + // (`pkg/BUILD`) or its basename fallback. Pinning the language Hunk already resolved keeps the + // renderer, plain-text detection, and source expansion agreeing on one answer. + const language = fileLanguageForPath(path); const sourceFetcher = sourceFetcherBuilder?.({ path, previousPath: resolvedPreviousPath, @@ -81,9 +86,9 @@ export function buildDiffFile( path, previousPath: resolvedPreviousPath, patch, - language: fileLanguageForPath(path) ?? undefined, + language, stats: stats ?? countDiffStats(normalizedMetadata), - metadata: normalizedMetadata, + metadata: setLanguageOverride(normalizedMetadata, language), lineMoveKinds, agent: findSidecarFileContext(sidecar, path, resolvedPreviousPath), isUntracked, diff --git a/src/core/changeset/fileLanguage.test.ts b/src/core/changeset/fileLanguage.test.ts index 5105a8841..3bdf94c32 100644 --- a/src/core/changeset/fileLanguage.test.ts +++ b/src/core/changeset/fileLanguage.test.ts @@ -20,6 +20,42 @@ describe("custom file language registration", () => { expect(BUILT_IN_FILE_LANGUAGE_EXTENSIONS.has("mts")).toBe(true); expect(BUILT_IN_FILE_LANGUAGE_EXTENSIONS.has("cts")).toBe(true); expect(BUILT_IN_FILE_LANGUAGE_EXTENSIONS.has("ts")).toBe(false); + // Python is a default for Starlark, not a claim, so an extension may replace it. + expect(BUILT_IN_FILE_LANGUAGE_EXTENSIONS.has("bzl")).toBe(false); + }); + + test("highlights Bazel and Starlark files as Python", () => { + expect(fileLanguageForPath("defs.bzl")).toBe("python"); + expect(fileLanguageForPath("tools/defs.bzl")).toBe("python"); + expect(fileLanguageForPath("rules.star")).toBe("python"); + expect(fileLanguageForPath("copy.bara.sky")).toBe("python"); + expect(fileLanguageForPath("BUILD.bazel")).toBe("python"); + expect(fileLanguageForPath("pkg/nested/MODULE.bazel")).toBe("python"); + expect(fileLanguageForPath("WORKSPACE.bzlmod")).toBe("python"); + expect(fileLanguageForPath("Tiltfile")).toBe("python"); + }); +}); + +describe("extensionless filename lookups", () => { + test("resolves Bazel package files at any depth", () => { + expect(fileLanguageForPath("BUILD")).toBe("python"); + expect(fileLanguageForPath("pkg/BUILD")).toBe("python"); + expect(fileLanguageForPath("a/b/c/WORKSPACE")).toBe("python"); + expect(fileLanguageForPath("third_party/BUCK")).toBe("python"); + }); + + test("resolves Pierre's own special filenames at any depth", () => { + expect(fileLanguageForPath("Dockerfile")).toBe("dockerfile"); + expect(fileLanguageForPath("docker/Dockerfile")).toBe("dockerfile"); + // Windows-style separators reach Hunk from user-supplied paths, not from VCS output. + expect(fileLanguageForPath("build\\tools\\Makefile")).toBe("makefile"); + }); + + test("leaves paths with no matching grammar as plain text", () => { + expect(fileLanguageForPath("notes")).toBe("text"); + expect(fileLanguageForPath("path/to/notes")).toBe("text"); + // `.bazelrc` is a flag file rather than Starlark, so it stays unhighlighted. + expect(fileLanguageForPath(".bazelrc")).toBe("text"); }); }); diff --git a/src/core/changeset/fileLanguage.ts b/src/core/changeset/fileLanguage.ts index 6f360defa..bfd0f6cde 100644 --- a/src/core/changeset/fileLanguage.ts +++ b/src/core/changeset/fileLanguage.ts @@ -1,7 +1,8 @@ import type { SupportedLanguages } from "@pierre/diffs"; /** - * Records file-extension → highlight-language mappings without loading the diff engine. + * Records file-extension and filename → highlight-language mappings without loading the diff + * engine. * * Registration happens during startup, on every invocation, while the mappings are only read * when a changeset is built. Applying them eagerly would pull the whole diff engine — and its @@ -13,26 +14,54 @@ import type { SupportedLanguages } from "@pierre/diffs"; */ // Pierre omits these TypeScript extensions, so Hunk registers them itself. -const HUNK_CUSTOM_EXTENSIONS: Record = { +const HUNK_RESERVED_EXTENSIONS: Record = { mts: "typescript", cts: "typescript", }; /** - * Extensions Hunk itself registers, in Pierre's dotless lowercase form. + * Maps Bazel and Starlark files to Python, keyed by extension and by exact filename. + * + * Starlark has no grammar of its own and is not waiting for one: GitHub Linguist classifies it + * with `tm_scope: source.python`, so Python is the intended rendering rather than a stand-in + * Hunk invented. The `bazel` and `bzlmod` extensions cover `BUILD.bazel`, `MODULE.bazel`, + * `WORKSPACE.bzlmod`, and Bazel's newer `REPO.bazel`/`VENDOR.bazel` without naming each one. + * + * These stay out of `BUILT_IN_FILE_LANGUAGE_EXTENSIONS` on purpose: an extension shipping a + * genuine Starlark grammar should be able to replace a Python approximation. + */ +const STARLARK_FILE_LANGUAGES: Record = { + bzl: "python", + star: "python", + bazel: "python", + bzlmod: "python", + // Legacy Skylark spelling, still current for Copybara's `copy.bara.sky`. + sky: "python", + BUILD: "python", + WORKSPACE: "python", + BUCK: "python", + Tiltfile: "python", +}; + +/** + * Extensions Hunk refuses to yield to an extension, in Pierre's dotless lowercase form. * * Extension-contributed mappings are skipped rather than allowed to shadow * these, so a third-party language pack cannot silently break TypeScript * highlighting for everyone. */ export const BUILT_IN_FILE_LANGUAGE_EXTENSIONS: ReadonlySet = new Set( - Object.keys(HUNK_CUSTOM_EXTENSIONS), + Object.keys(HUNK_RESERVED_EXTENSIONS), ); // Hunk's own mappings are seeded here rather than applied on import, so they land in the same // pass as extension-contributed ones. `apply.ts` refuses extension mappings that collide with -// BUILT_IN_FILE_LANGUAGE_EXTENSIONS, so seeding first cannot lose to a later registration. -const pendingFileLanguages = new Map(Object.entries(HUNK_CUSTOM_EXTENSIONS)); +// BUILT_IN_FILE_LANGUAGE_EXTENSIONS, so the reserved ones cannot lose to a later registration; +// the Starlark defaults intentionally can. +const pendingFileLanguages = new Map([ + ...Object.entries(HUNK_RESERVED_EXTENSIONS), + ...Object.entries(STARLARK_FILE_LANGUAGES), +]); /** * Map one dotless, lowercased file extension to a highlight language. diff --git a/src/core/changeset/fileLanguageLookup.ts b/src/core/changeset/fileLanguageLookup.ts index 36e9e440d..c03877d54 100644 --- a/src/core/changeset/fileLanguageLookup.ts +++ b/src/core/changeset/fileLanguageLookup.ts @@ -21,8 +21,22 @@ function applyPendingFileLanguages() { } } -/** Return the highlight language for one path, or undefined when no grammar matches. */ -export function fileLanguageForPath(path: string) { +/** + * Return the highlight language for one path, or `"text"` when no grammar matches. + * + * Pierre keys extensionless names (`BUILD`, `Dockerfile`, `Makefile`) on the whole lookup string, + * but Hunk looks up repo-relative paths, so a nested `pkg/BUILD` would otherwise render as plain + * text while a root-level `BUILD` highlighted. Retrying on the basename is reachable only once + * the full path has matched nothing, so a real extension still wins over a filename that merely + * happens to appear deeper in the tree. + */ +export function fileLanguageForPath(path: string): SupportedLanguages { applyPendingFileLanguages(); - return getFiletypeFromFileName(path); + const language = getFiletypeFromFileName(path); + if (language !== "text") { + return language; + } + + const basename = path.split(/[/\\]/).pop() ?? path; + return basename === path ? language : getFiletypeFromFileName(basename); }