build: support lint-js-fix in vcbuild.bat

[RedYetiDev]

This PR adds support for lint-js-fix in the vcbuild.bat file.

[anonrig]

cc @nodejs/platform-windows

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59293/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59298/

[huseyinacacak-janea]

Hey @RedYetiDev, the labels you created should be called from other command in order to be executed. Feel free to use the patch I'm sending.

Another thing, this is not as important, I think lint-md-fix and lint-js-fix should go before lint-md and lint-js, just in case someone tries to run both commands at the same time.

From e45abb7e84c32d7403a196303e406ed775580261 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?H=C3=BCseyin=20A=C3=A7acak?= <[email protected]>
Date: Mon, 20 May 2024 14:24:40 +0300
Subject: [PATCH 1/1] build: fix lint-md-fix and lint-js-fix

---
 vcbuild.bat | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/vcbuild.bat b/vcbuild.bat
index f07711fab7..1b71db935d 100644
--- a/vcbuild.bat
+++ b/vcbuild.bat
@@ -707,11 +707,11 @@ goto lint-js
 goto lint-js
 
 :lint-js
-if not defined lint_js goto lint-md-build
+if not defined lint_js goto lint-js-fix
 if not exist tools\node_modules\eslint goto no-lint
 echo running lint-js
 %node_exe% tools\node_modules\eslint\bin\eslint.js --cache --max-warnings=0 --report-unused-disable-directives --rule "linebreak-style: 0" .eslintrc.js benchmark doc lib test tools
-goto lint-md-build
+goto lint-js-fix
 
 :lint-js-fix
 if not defined lint_js_fix goto lint-md-build
@@ -731,7 +731,7 @@ echo "Deprecated no-op target 'lint_md_build'"
 goto lint-md
 
 :lint-md
-if not defined lint_md goto format-md
+if not defined lint_md goto lint-md-fix
 echo Running Markdown linter on docs...
 SETLOCAL ENABLEDELAYEDEXPANSION
 set lint_md_files=
@@ -742,7 +742,7 @@ for /D %%D IN (doc\*) do (
 )
 %node_exe% tools\lint-md\lint-md.mjs %lint_md_files%
 ENDLOCAL
-goto format-md
+goto lint-md-fix
 
 :lint-md-fix
 if not defined lint_md_fix goto format-md
-- 
2.43.0.windows.1

[RedYetiDev]

I disagree, there are times that people want to lint without fixes

[RedYetiDev]

Another thing, this is not as important, I think lint-md-fix and lint-js-fix should go before lint-md and lint-js, just in case someone tries to run both commands at the same time.

That won't happen, as there can only be one %1 argument (IIUC)

[huseyinacacak-janea]

That won't happen, as there can only be one %1 argument (IIUC)

Theoretically, if you call vcbuild.bat lint-js lint-js-fix it will do both. The value of %1 is changed with the usage of shift command here. As I said, this part is a suggestion.

The patch file I provided enables the 2 new commands. Running vcbuild.bat lint-js-fix does nothing for me locally unless the patch is applied. I do not see running lint-js-fix which should be there.

[RedYetiDev]

That won't happen, as there can only be one %1 argument (IIUC)

Theoretically, if you call vcbuild.bat lint-js lint-js-fix it will do both. The value of %1 is changed with the usage of shift command here. As I said, this part is a suggestion.

The patch file I provided enables the 2 new commands. Running vcbuild.bat lint-js-fix does nothing for me locally unless the patch is applied. I do not see running lint-js-fix which should be there.

Good to know, thanks (I'm not a batch file expert)

[RedYetiDev]

I've made your suggestions. Sorry I brushed your comment off initially, I'm not a batch file expert and I didn't quite understand how goto worked.

[huseyinacacak-janea]

No problem, glad I could help.

[RedYetiDev]

To summarize, I darn goofed and --fix isn't supported by the markdown linter.

[aduh95]

Could someone with a Windows machine validate that this works so this can land?

[H4ad]

The script is being called correctly, so this PR is good.

---

But I'm getting an error (which is not related to this PR) that is:

$ node.exe tools\node_modules\eslint\bin\eslint.js --cache --max-warnings=0 --report-unused-disable-directives --rule "linebreak-style: 0" .eslintrc.js benchmark doc lib test tools

Oops! Something went wrong! :(

ESLint: 8.57.0

C:\Users\vinic\Desktop\Projects\node\tools\node_modules\eslint\node_modules\eslint:1
..
^

SyntaxError: Unexpected token '.'
    at internalCompileFunction (node:internal/vm:128:18)
    at wrapSafe (node:internal/modules/cjs/loader:1279:20)
    at Module._compile (node:internal/modules/cjs/loader:1331:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1426:10)
    at Module.load (node:internal/modules/cjs/loader:1205:32)
    at Module._load (node:internal/modules/cjs/loader:1021:12)
    at Module.require (node:internal/modules/cjs/loader:1230:19)
    at require (node:internal/modules/helpers:179:18)
    at Object.<anonymous> (C:\Users\vinic\Desktop\Projects\node\tools\node_modules\eslint\node_modules\eslint-plugin-jsdoc\dist\rules\checkExamples.cjs:8:39)
    at Module._compile (node:internal/modules/cjs/loader:1368:14)

Any recommendations on what to do here?

[huseyinacacak-janea]

@H4ad eslint is a symlink that doesn't work by default in Git. To fix this issue, the repo should be cloned as follows
git clone -c core.symlinks=true <repository_url>

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/53046
✔  Done loading data for nodejs/node/pull/53046
----------------------------------- PR info ------------------------------------
Title      build: support `lint-js-fix` in `vcbuild.bat` (#53046)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     RedYetiDev:patch-32 -> nodejs:main
Labels     windows, build, author ready, needs-ci
Commits    4
 - build: support `lint-md-fix` and `lint-js-fix` in `vcbuild.bat`
 - Update vcbuild.bat
 - Update vcbuild.bat
 - Update vcbuild.bat
Committers 1
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/53046
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Moshe Atlow 
Reviewed-By: Vinícius Lourenço Claro Cardoso 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53046
Reviewed-By: Yagiz Nizipli 
Reviewed-By: Moshe Atlow 
Reviewed-By: Vinícius Lourenço Claro Cardoso 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 18 May 2024 13:18:20 GMT
   ✔  Approvals: 3
   ✔  - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/53046#pullrequestreview-2064964306
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/53046#pullrequestreview-2065008304
   ✔  - Vinícius Lourenço Claro Cardoso (@H4ad): https://github.com/nodejs/node/pull/53046#pullrequestreview-2073766737
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-05-19T07:10:15Z: https://ci.nodejs.org/job/node-test-pull-request/59298/
   ⚠  Commits were pushed after the last Full PR CI run:
   ⚠  - Update vcbuild.bat
   ⚠  - Update vcbuild.bat
- Querying data for job/node-test-pull-request/59298/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/9208503129

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59371/

[H4ad]

Nothing more sad than a green CI and a conflict, @RedYetiDev can you rebase?

[RedYetiDev]

Fingers crossed nothing broke

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59402/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59405/

[aduh95]

@RedYetiDev can you please rebase to get rid of the merge commit that the CI is choking on?

[RedYetiDev]

😌 Rebase complete

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59411/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59695/

[aduh95]

This needs another rebase.

[RedYetiDev]

@aduh95 done :-)

[RedYetiDev]

The merge conflicts won't go away :/, I might need to open a new PR.