Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/nested-extensionless-highlighting.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions .changeset/starlark-highlighting.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"hunkdiff": minor
---

Highlight Bazel and Starlark files with the Python grammar, matching how GitHub classifies Starlark.
18 changes: 18 additions & 0 deletions src/core/changeset/diffFile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 8 additions & 3 deletions src/core/changeset/diffFile.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
36 changes: 36 additions & 0 deletions src/core/changeset/fileLanguage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});

Expand Down
41 changes: 35 additions & 6 deletions src/core/changeset/fileLanguage.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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<string, SupportedLanguages> = {
const HUNK_RESERVED_EXTENSIONS: Record<string, SupportedLanguages> = {
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<string, SupportedLanguages> = {
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<string> = 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<string, string>(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<string, string>([
...Object.entries(HUNK_RESERVED_EXTENSIONS),
...Object.entries(STARLARK_FILE_LANGUAGES),
]);

/**
* Map one dotless, lowercased file extension to a highlight language.
Expand Down
20 changes: 17 additions & 3 deletions src/core/changeset/fileLanguageLookup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Loading