Skip to content

Commit 7ff1fc5

Browse files
authored
Resolve Task from PATH in the comment-lint hook windows (Stirling-Tools#7793)
What was wrong in scripts/lint/comment-lint-hook.mjs: The win32 fallback hardcoded task.cmd. Scoop ships only task.exe, so cmd.exe returned exit 1. The hook maps exit 1 to FOUND, so a missing executable became "findings" with an empty report. Task's stderr was captured and discarded, so the actual error never surfaced. The fix (commit b285c0d, 17 insertions): Fall back to bare task on win32 too, still shell: true, so PATHEXT finds task.exe / .cmd / .bat. FOUND with an empty report is now treated as "did not run", exit 1 (non-blocking) instead of exit 2. Keep Task's stderr and print it with that message. ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] Every comment I added says something the code does not ([guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/CODE_COMMENTS.md)) - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have run `task check` to verify linters, typechecks, and tests pass - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details.
1 parent 77bbf40 commit 7ff1fc5

1 file changed

Lines changed: 17 additions & 6 deletions

File tree

scripts/lint/comment-lint-hook.mjs

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,15 @@ const result = run();
5454
if (result.status === 0) process.exit(0);
5555

5656
if (result.status === ENGINE_BROKEN) {
57-
process.stderr.write("comment-lint could not run, so comments in this turn were not checked.\n");
57+
process.stderr.write(`comment-lint could not run, so comments in this turn were not checked.${reason(result)}\n`);
5858
process.exit(1);
5959
}
6060

61-
// Anything else is Task itself failing, which means the check did not happen.
62-
if (result.status !== FOUND) {
63-
process.stderr.write(`comment-lint did not run (task exit ${result.status}), so comments in this turn were not checked.\n`);
61+
// Anything else is Task itself failing, which means the check did not happen. A
62+
// findings exit carrying no findings is the same case: Task failed before the
63+
// linter ran, so blocking on it would name comments nobody can go and read.
64+
if (result.status !== FOUND || !result.output.trim()) {
65+
process.stderr.write(`comment-lint did not run (task exit ${result.status}), so comments in this turn were not checked.${reason(result)}\n`);
6466
process.exit(1);
6567
}
6668

@@ -69,6 +71,13 @@ if (result.status !== FOUND) {
6971
process.stderr.write(`${result.output.trim()}\n\nFix these before finishing.\n`);
7072
process.exit(2);
7173

74+
// Task's stderr names the real failure, such as an executable missing from PATH,
75+
// which the captured pipe would otherwise swallow.
76+
function reason(result) {
77+
const message = (result.error ?? "").trim();
78+
return message ? `\n${message}` : "";
79+
}
80+
7281
function readStdin() {
7382
try {
7483
return JSON.parse(readFileSync(0, "utf8"));
@@ -92,7 +101,9 @@ function taskCommand() {
92101
for (const candidate of candidates) {
93102
if (existsSync(candidate)) return { command: candidate, shell: false };
94103
}
95-
return { command: process.platform === "win32" ? "task.cmd" : "task", shell: process.platform === "win32" };
104+
// Bare name on win32 too: the shell resolves it through PATHEXT, so it finds
105+
// task.exe, task.cmd or task.bat, whichever the installer shipped.
106+
return { command: "task", shell: process.platform === "win32" };
96107
}
97108

98109
function run() {
@@ -114,6 +125,6 @@ function run() {
114125
});
115126
return { status: 0, output };
116127
} catch (error) {
117-
return { status: error.status ?? -1, output: error.stdout ?? "" };
128+
return { status: error.status ?? -1, output: error.stdout ?? "", error: error.stderr ?? "" };
118129
}
119130
}

0 commit comments

Comments
 (0)