Skip to content
Merged
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
40 changes: 35 additions & 5 deletions src/CommandTreeProvider.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import * as vscode from "vscode";
import { isPhonyTask, isPrivateTask } from "./models/TaskItem";
import type { CommandItem, CategoryDef } from "./models/TaskItem";
import type { CommandTreeItem } from "./models/TaskItem";
import type { DiscoveryResult } from "./discovery";
import { discoverAllTasks, flattenTasks, getExcludePatterns, CATEGORY_DEFS } from "./discovery";
import { TagConfig } from "./config/TagConfig";
import { logger } from "./utils/logger";
import { buildNestedFolderItems } from "./tree/folderTree";
import { createCommandNode, createCategoryNode } from "./tree/nodeFactory";
import { createCategoryNode, createTaskNodes } from "./tree/nodeFactory";
import { getAllRows } from "./db/db";
import type { CommandRow } from "./db/db";
import { getDbOrThrow } from "./db/lifecycle";
Expand Down Expand Up @@ -166,7 +167,7 @@ export class CommandTreeProvider implements vscode.TreeDataProvider<CommandTreeI

private buildFlatCategory(def: CategoryDef, tasks: CommandItem[]): CommandTreeItem {
const sorted = this.sortTasks(tasks);
const children = sorted.map((t) => createCommandNode(t));
const children = createTaskNodes(sorted);
return createCategoryNode({
label: `${def.label} (${tasks.length})`,
children,
Expand All @@ -183,15 +184,44 @@ export class CommandTreeProvider implements vscode.TreeDataProvider<CommandTreeI
return [...tasks].sort(comparator);
}

private comparePrivateTasks(a: CommandItem, b: CommandItem): number {
return Number(isPrivateTask(a)) - Number(isPrivateTask(b));
}

private compareHelpTasks(a: CommandItem, b: CommandItem): number {
const isHelpA = a.type === "make" && a.label === "help";
const isHelpB = b.type === "make" && b.label === "help";
return Number(isHelpB) - Number(isHelpA);
}

private comparePhonyTasks(a: CommandItem, b: CommandItem): number {
return Number(isPhonyTask(b)) - Number(isPhonyTask(a));
}

private compareMakeTaskPriority(a: CommandItem, b: CommandItem): number {
if (a.type !== "make" || b.type !== "make") {
return 0;
}
return this.compareHelpTasks(a, b) || this.comparePhonyTasks(a, b);
}

private getComparator(): (a: CommandItem, b: CommandItem) => number {
const order = this.getSortOrder();
if (order === "folder") {
return (a, b) => a.category.localeCompare(b.category) || a.label.localeCompare(b.label);
return (a, b) =>
a.category.localeCompare(b.category) ||
this.comparePrivateTasks(a, b) ||
this.compareMakeTaskPriority(a, b) ||
a.label.localeCompare(b.label);
}
if (order === "type") {
return (a, b) => a.type.localeCompare(b.type) || a.label.localeCompare(b.label);
return (a, b) =>
a.type.localeCompare(b.type) ||
this.comparePrivateTasks(a, b) ||
this.compareMakeTaskPriority(a, b) ||
a.label.localeCompare(b.label);
}
return (a, b) => a.label.localeCompare(b.label);
return (a, b) => this.comparePrivateTasks(a, b) || this.compareMakeTaskPriority(a, b) || a.label.localeCompare(b.label);
}

private applyTagFilter(tasks: CommandItem[]): CommandItem[] {
Expand Down
61 changes: 58 additions & 3 deletions src/discovery/make.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as vscode from "vscode";
import * as path from "path";
import type { CommandItem, IconDef, CategoryDef } from "../models/TaskItem";
import type { CommandItem, MutableCommandItem, IconDef, CategoryDef } from "../models/TaskItem";
import { generateCommandId, simplifyPath } from "../models/TaskItem";
import { readFileContent } from "../utils/fileUtils";

Expand Down Expand Up @@ -28,6 +28,7 @@ export async function discoverMakeTargets(workspaceRoot: string, excludePatterns

for (const file of allFiles) {
const content = await readFileContent(file);
const phonyTargets = parsePhonyTargets(content);
const targets = parseMakeTargets(content);
const makeDir = path.dirname(file.fsPath);
const category = simplifyPath(file.fsPath, workspaceRoot);
Expand All @@ -38,7 +39,7 @@ export async function discoverMakeTargets(workspaceRoot: string, excludePatterns
continue;
}

commands.push({
const command: MutableCommandItem = {
id: generateCommandId("make", file.fsPath, name),
label: name,
type: "make",
Expand All @@ -48,7 +49,13 @@ export async function discoverMakeTargets(workspaceRoot: string, excludePatterns
filePath: file.fsPath,
tags: [],
line,
});
};

if (phonyTargets.has(name)) {
command.isPhony = true;
}

commands.push(command);
}
}

Expand All @@ -60,6 +67,54 @@ interface MakeTarget {
readonly line: number;
}

function addPhonyTargets(line: string, phonyTargets: Set<string>): void {
for (const name of line.split(/\s+/)) {
if (name !== "") {
phonyTargets.add(name);
}
}
}

function trimContinuation(line: string): string {
return line.endsWith("\\") ? line.slice(0, -1).trim() : line;
}

function isContinuationLine(line: string): boolean {
return line.endsWith("\\");
}

function readPhonyLine(line: string): string | undefined {
const trimmed = line.trim();
if (!trimmed.startsWith(".PHONY:")) {
return undefined;
}
return trimmed.slice(".PHONY:".length).trim();
}

function parsePhonyTargets(content: string): ReadonlySet<string> {
const phonyTargets = new Set<string>();
let collecting = false;

for (const line of content.split("\n")) {
const trimmed = line.trim();
if (collecting) {
addPhonyTargets(trimContinuation(trimmed), phonyTargets);
collecting = isContinuationLine(trimmed);
continue;
}

const phonyLine = readPhonyLine(line);
if (phonyLine === undefined) {
continue;
}

addPhonyTargets(trimContinuation(phonyLine), phonyTargets);
collecting = isContinuationLine(phonyLine);
}

return phonyTargets;
}

/**
* Parses Makefile to extract target names and their line numbers.
*/
Expand Down
14 changes: 14 additions & 0 deletions src/models/TaskItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export interface CommandItem {
readonly description?: string;
readonly summary?: string;
readonly securityWarning?: string;
readonly isPhony?: boolean;
readonly line?: number;
}

Expand All @@ -115,6 +116,7 @@ export interface MutableCommandItem {
description?: string;
summary?: string;
securityWarning?: string;
isPhony?: boolean;
line?: number;
}

Expand Down Expand Up @@ -217,3 +219,15 @@ export function simplifyPath(filePath: string, workspaceRoot: string): string {
export function generateCommandId(type: CommandType, filePath: string, name: string): string {
return `${type}:${filePath}:${name}`;
}

function supportsPrivateTaskStyling(type: CommandType): boolean {
return type === "make" || type === "mise";
}

export function isPrivateTask(task: CommandItem): boolean {
return supportsPrivateTaskStyling(task.type) && task.label.startsWith("_");
}

export function isPhonyTask(task: CommandItem): boolean {
return task.type === "make" && task.isPhony === true;
}
117 changes: 117 additions & 0 deletions src/test/e2e/treeview.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ suite("TreeView E2E Tests", () => {
suite("Private Make And Mise Tasks", () => {
const makeRelativePath = "private-targets/Makefile";
const miseRelativePath = "private-targets/mise.toml";
const privateDivider = "---------------- private ----------------";
const publicLabels = ["alpha_public", "zeta_public"];
const privateLabels = ["_beta_private", "_omega_private"];

Expand All @@ -269,6 +270,19 @@ suite("TreeView E2E Tests", () => {
);
}

async function getFolderChildrenForCategory(categoryLabel: string, folderLabel: string): Promise<CommandTreeItem[]> {
const provider = getCommandTreeProvider();
const categories = await provider.getChildren();
const category = categories.find((item) => getLabelString(item.label).includes(categoryLabel));
assert.ok(category !== undefined, `Should find category ${categoryLabel}`);

const children = await provider.getChildren(category);
const folder = children.find((item) => getLabelString(item.label) === folderLabel);
assert.ok(folder !== undefined, `Should find folder ${folderLabel}`);

return await provider.getChildren(folder);
}

setup(async function () {
this.timeout(15000);

Expand Down Expand Up @@ -321,6 +335,14 @@ suite("TreeView E2E Tests", () => {

const items = await getItemsForFile("make", makeRelativePath);
const labels = items.map((item) => getLabelString(item.label));
const folderChildren = await getFolderChildrenForCategory("Make Targets", "private-targets");
const folderLabels = folderChildren.map((item) => getLabelString(item.label));

assert.deepStrictEqual(
folderLabels,
[...publicLabels, privateDivider, ...privateLabels],
"Make targets should insert a divider between public and _-prefixed private targets"
);

assert.deepStrictEqual(
labels,
Expand Down Expand Up @@ -350,6 +372,14 @@ suite("TreeView E2E Tests", () => {

const items = await getItemsForFile("mise", miseRelativePath);
const labels = items.map((item) => getLabelString(item.label));
const folderChildren = await getFolderChildrenForCategory("Mise Tasks", "private-targets");
const folderLabels = folderChildren.map((item) => getLabelString(item.label));

assert.deepStrictEqual(
folderLabels,
[...publicLabels, privateDivider, ...privateLabels],
"Mise tasks should insert a divider between public and _-prefixed private tasks"
);

assert.deepStrictEqual(
labels,
Expand All @@ -374,4 +404,91 @@ suite("TreeView E2E Tests", () => {
}
});
});

suite("Make Target Conventions", () => {
const makeRelativePath = "make-conventions/Makefile";
const privateDivider = "---------------- private ----------------";

async function getFolderChildrenForCategory(categoryLabel: string, folderLabel: string): Promise<CommandTreeItem[]> {
const provider = getCommandTreeProvider();
const categories = await provider.getChildren();
const category = categories.find((item) => getLabelString(item.label).includes(categoryLabel));
assert.ok(category !== undefined, `Should find category ${categoryLabel}`);

const children = await provider.getChildren(category);
const folder = children.find((item) => getLabelString(item.label) === folderLabel);
assert.ok(folder !== undefined, `Should find folder ${folderLabel}`);

return await provider.getChildren(folder);
}

async function getMakeItemsForFile(relativePath: string): Promise<CommandTreeItem[]> {
const provider = getCommandTreeProvider();
const items = await collectLeafItems(provider);
return items.filter(
(item) => isCommandItem(item.data) && item.data.type === "make" && item.data.filePath.endsWith(relativePath)
);
}

setup(async function () {
this.timeout(15000);

writeFile(
makeRelativePath,
[
".PHONY: help build _private",
"",
"aaa_file:",
'\t@echo "file target"',
"",
"help:",
'\t@echo "help target"',
"",
"build:",
'\t@echo "build target"',
"",
"%.o: %.c",
'\t@echo "pattern rule"',
"",
".DEFAULT:",
'\t@echo "special target"',
"",
"_private:",
'\t@echo "private target"',
].join("\n")
);

await refreshTasks();
});

teardown(async function () {
this.timeout(15000);
deleteFile(makeRelativePath);
await refreshTasks();
});

test("make help is pinned to the top, phony targets sort before non-phony ones, and special targets stay hidden", async function () {
this.timeout(15000);

const folderChildren = await getFolderChildrenForCategory("Make Targets", "make-conventions");
const folderLabels = folderChildren.map((item) => getLabelString(item.label));
const items = await getMakeItemsForFile(makeRelativePath);
const labels = items.map((item) => getLabelString(item.label));

assert.deepStrictEqual(
folderLabels,
["help", "build", "aaa_file", privateDivider, "_private"],
"Make targets should pin help first, prefer phony public targets over non-phony ones, and separate private targets"
);

assert.deepStrictEqual(
labels,
["help", "build", "aaa_file", "_private"],
"Only invokable make targets should remain after hiding special and pattern rules"
);

assert.ok(!labels.includes("%.o"), "Pattern rules should be hidden from Make discovery");
assert.ok(!labels.includes(".DEFAULT"), "Dot-prefixed special targets should be hidden from Make discovery");
});
});
});
8 changes: 4 additions & 4 deletions src/tree/folderTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { CommandItem } from "../models/TaskItem";
import type { CommandTreeItem } from "../models/TaskItem";
import type { DirNode } from "./dirTree";
import { groupByFullDir, buildDirTree, needsFolderWrapper, getFolderLabel } from "./dirTree";
import { createCommandNode, createFolderNode } from "./nodeFactory";
import { createFolderNode, createTaskNodes } from "./nodeFactory";

/**
* Renders a DirNode as a folder CommandTreeItem.
Expand All @@ -20,7 +20,7 @@ function renderFolder({
}): CommandTreeItem {
const label = getFolderLabel(node.dir, parentDir);
const folderId = `${parentTreeId}/${label}`;
const taskItems = sortTasks(node.tasks).map((t) => createCommandNode(t));
const taskItems = createTaskNodes(sortTasks(node.tasks));
const subItems = node.subdirs.map((sub) =>
renderFolder({
node: sub,
Expand Down Expand Up @@ -66,7 +66,7 @@ export function buildNestedFolderItems({
})
);
}
result.push(...sortTasks(node.tasks).map((t) => createCommandNode(t)));
result.push(...createTaskNodes(sortTasks(node.tasks)));
} else if (needsFolderWrapper(node, rootNodes.length)) {
result.push(
renderFolder({
Expand All @@ -77,7 +77,7 @@ export function buildNestedFolderItems({
})
);
} else {
result.push(...sortTasks(node.tasks).map((t) => createCommandNode(t)));
result.push(...createTaskNodes(sortTasks(node.tasks)));
}
}

Expand Down
Loading
Loading