-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
doc: fix Windows code snippet tags #48100
Conversation
Those were wrongly marked as `bash`, when the text around it was referencing PowerShell or Command Prompt.
Review requested:
|
Fast-track has been requested by @aduh95. Please 👍 to approve. |
cp (Get-Command node).Source hello.exe | ||
``` | ||
|
||
Using Command Prompt: | ||
|
||
```bash | ||
```text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
```text | |
```cmd |
Shouldn't this be cmd?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linter won’t let me do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so here's the total list:
node/tools/lint-md/lint-md.mjs
Lines 20828 to 20843 in 260092e
"bash", | |
"c", | |
"cjs", | |
"coffee", | |
"console", | |
"cpp", | |
"diff", | |
"http", | |
"js", | |
"json", | |
"markdown", | |
"mjs", | |
"powershell", | |
"r", | |
"text", | |
"ts", |
I think
console
still makes more sense than text
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree, console
is meant for showing the output of commands (with optionally the command themselves when prefixed with $
or >
), so it wouldn’t apply here. If you feel strongly that text
is not correct, let’s add cmd
to the list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel strongly about it :)
Landed in 30870d1 |
Those were wrongly marked as `bash`, when the text around it was referencing PowerShell or Command Prompt. PR-URL: nodejs#48100 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Mestery <[email protected]>
Those were wrongly marked as `bash`, when the text around it was referencing PowerShell or Command Prompt. PR-URL: nodejs#48100 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Mestery <[email protected]>
Those were wrongly marked as `bash`, when the text around it was referencing PowerShell or Command Prompt. PR-URL: #48100 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Mestery <[email protected]>
Blocked by #47125 |
Those were wrongly marked as `bash`, when the text around it was referencing PowerShell or Command Prompt. PR-URL: nodejs#48100 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Mestery <[email protected]>
Those were wrongly marked as `bash`, when the text around it was referencing PowerShell or Command Prompt. PR-URL: nodejs#48100 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Mestery <[email protected]>
Those were wrongly marked as
bash
(that's my bad, it's coming from #48082), when the text around it was referencing PowerShell or Command Prompt.