Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
doc: gh markdown style code blocks#4733
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: gh markdown style code blocks #4733
Uh oh!
There was an error while loading. Please reload this page.
Conversation
rvagg commented Jan 18, 2016
re the Regarding renaming, I don't think you're going to get any support on this, it's unnecessary churn that makes the history too hard to follow. In general churn without strong justification gets a -1 around here for this reason. Regarding unindending code blocks, I'm +1 on that, I've never liked the indent code block formatting and being explicit about We need @nodejs/documentation in this discussion too though. @eljefedelrodeodeljefe in case you weren't aware, there's an active call for involvement in the proposed Docs WG that might interest you: nodejs/docs#2 |
doc/api/child_process.md Outdated
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.
Since there's churn here anyway, maybe make the indentation consistent while you're at it?
Qard commented Jan 18, 2016
There's a few spots missing the js tag. I'm overall +0 on this. Any other docs folks have thought on it? |
bengl commented Jan 18, 2016
From a @nodejs/documentation perspective, with respect to de-indenting and using language-aware code fences, this change is consistent with our new style guide (see the bottom). While renaming from |
stevemao commented Jan 18, 2016
This bug should be fixed. |
eljefedelrodeodeljefe commented Jan 18, 2016
Added @Qard's remarks in be6d6e8. @rvagg I agree on the churn. However maybe this will be the only time this can be touched. So please ignore the commit, while rebasing if no-one is in favor. @stevemao good link. I was looking at it and it seems to render okay, @rvagg . I have no strong opinion on bash. However you like it. @rvagg thanks for welcoming me on the other PR and pointing to the docs wg. It's fun so far. :) |
eljefedelrodeodeljefe commented Jan 18, 2016
It's really unfortunate that |
rvagg commented Jan 18, 2016
re |
rvagg commented Jan 18, 2016
oh, and regarding the rename of the files, it's making the commit diff really large and hard to review so you may get more traction towards landing if you clean it up by removing that commit (let us know if you need any hints on the git-fu for that). |
eljefedelrodeodeljefe commented Jan 18, 2016
I will try something like this gist later https://gist.github.com/emiller/6769886 But this is probably too hard, since it's likely rewriting the history (into the new files). I will see. Thanks for the help! |
stevemao commented Jan 18, 2016
Hi @rvagg Could you link |
rvagg commented Jan 18, 2016
@stevemao sorry, just being colloquial, I was just meaning advanced git usage. In this case to remove a commit I'd use |
stevemao commented Jan 18, 2016
Oh yeah true... Thanks for the clarification. |
Fishrock123 commented Jan 18, 2016
@eljefedelrodeodeljefe Could you please move the |
eljefedelrodeodeljefe commented Jan 18, 2016
@Fishrock123 removed the commit by rebase and force pushing. Think the other PR got a little whacked. I tell you....after that |
doc/api/repl.markdown Outdated
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.
Note for this PR or the future, should probably remove mjr:~
silverwind commented Jan 18, 2016
More comparision of bash$ NODE_DEBUG=cluster node server.js 23521,Master Worker 23524 online 23521,Master Worker 23526 online 23521,Master Worker 23523 online 23521,Master Worker 23528 onlineconsole$ NODE_DEBUG=cluster node server.js23521,Master Worker 23524 online23521,Master Worker 23526 online23521,Master Worker 23523 online23521,Master Worker 23528 onlinebash$ node script.js 2> error.log | tee info.logconsole$ node script.js 2> error.log | tee info.logbash% node debug myscript.js < debugger listening on port 5858 connecting... ok breakin /home/indutny/Code/git/indutny/myscript.js:1 1 x = 5; 2 setTimeout(function (){3 debugger; debug>console% node debug myscript.js< debugger listening on port 5858connecting... okbreak in /home/indutny/Code/git/indutny/myscript.js:1 1 x = 5; 2 setTimeout(function (){ 3 debugger;debug>bash$ node debug myscript.js < debugger listening on port 5858 connecting... ok breakin /home/indutny/Code/git/indutny/myscript.js:1 1 x = 5; 2 setTimeout(function (){3 debugger; debug> cont < hello breakin /home/indutny/Code/git/indutny/myscript.js:3 1 x = 5; 2 setTimeout(function (){3 debugger; 4 console.log('world'); 5 }, 1000); debug> next breakin /home/indutny/Code/git/indutny/myscript.js:4 2 setTimeout(function (){3 debugger; 4 console.log('world'); 5 }, 1000); 6 console.log('hello'); debug> repl Press Ctrl + C to leave debug repl > x 5 > 2+2 4 debug> next < world breakin /home/indutny/Code/git/indutny/myscript.js:5 3 debugger; 4 console.log('world'); 5 }, 1000); 6 console.log('hello'); 7 debug> quit $console$ node debug myscript.js< debugger listening on port 5858connecting... okbreak in /home/indutny/Code/git/indutny/myscript.js:1 1 x = 5; 2 setTimeout(function (){ 3 debugger;debug> cont< hellobreak in /home/indutny/Code/git/indutny/myscript.js:3 1 x = 5; 2 setTimeout(function (){ 3 debugger; 4 console.log('world'); 5 }, 1000);debug> nextbreak in /home/indutny/Code/git/indutny/myscript.js:4 2 setTimeout(function (){ 3 debugger; 4 console.log('world'); 5 }, 1000); 6 console.log('hello');debug> replPress Ctrl + C to leave debug repl > x5 > 2+24debug> next< worldbreak in /home/indutny/Code/git/indutny/myscript.js:5 3 debugger; 4 console.log('world'); 5 }, 1000); 6 console.log('hello'); 7debug> quit $I'd say we use plain ````` fences. None of |
Qard commented Jan 18, 2016
Your effort in this PR is greatly appreciated, however I think it's currently too big to properly review. I think if this was split up into a separate PR for each markdown file, only changing the indentation and fencing, I would be good to merge it. The other miscellaneous things should also be split out to separate PRs. |
silverwind commented Jan 19, 2016
For the purpose of viewing all changes on GitHub, it should suffice to do one commit per file. And yeah, let's focus on getting the fencing done first and do additional changes that have been suggested in seperate PRs. |
Qard commented Jan 19, 2016
You could split to a commit for each file, but that still requires that each file is validated entirely before anything in the PR can be merged. By splitting it to separate PRs, more easily consumable chunks can be validated and merged in isolation. Either way works, but I feel like it might go smoother if there are separate PRs. |
silverwind commented Jan 19, 2016
To keep things simple, I suggest eliminate "extra" changes (typo fixes) except the fencing from the current diff. We can then review it locally ( |
eljefedelrodeodeljefe commented Jan 19, 2016
I stand corrected. Checking out from @Qard, @silverwind I know it's churn. But the idea was to have churn in just one PR instead of three. Rod was right though, that especially renaming needs consensus and therefore needs to be separated. Anyhow the first commit (just plain code blocks) is probably as hard to review as the whole PR with all the changes. :) |
This changes the code blocks from 4-space indentation to ``` fences for better syntax highlighting and future linting support. Minor On-the-fly changes for typos and highlight breaking markdown have been made. JSON-Style objects have been changed so their closing bracket is on the same line as the opening one. Known issues: * Not every JSON / object notation has been improved. Should make another run for this. * Some example functions break hightlighting due to various combinations of brackets. However changing them means leaving the code style. Fixes: #4726 PR-URL: #4733 Reviewed-By: Roman Reiss <[email protected]>
silverwind commented Jan 21, 2016
Thanks! Landed in e436272. Feel free to follow up with more changes that were discussed here. |
This changes the code blocks from 4-space indentation to ``` fences for better syntax highlighting and future linting support. Minor On-the-fly changes for typos and highlight breaking markdown have been made. JSON-Style objects have been changed so their closing bracket is on the same line as the opening one. Known issues: * Not every JSON / object notation has been improved. Should make another run for this. * Some example functions break hightlighting due to various combinations of brackets. However changing them means leaving the code style. Fixes: #4726 PR-URL: #4733 Reviewed-By: Roman Reiss <[email protected]>
MylesBorins commented Jan 28, 2016
There are a number of changes to docs that have not yet landed that need to be back ported. Once those land this commit will likely have to be manually backported to LTS |
stevemao commented Feb 10, 2016
Aside: (don't wanna make this sound like an advertisement but it's actually useful) |
eljefedelrodeodeljefe commented Feb 10, 2016
@stevemao@silverwind had something similar with HTML output. In either form it's superuseful, thanks. |
stevemao commented Feb 10, 2016
I tried it. Another good tool :) |
This changes the code blocks from 4-space indentation to ``` fences for better syntax highlighting and future linting support. Minor On-the-fly changes for typos and highlight breaking markdown have been made. JSON-Style objects have been changed so their closing bracket is on the same line as the opening one. Known issues: * Not every JSON / object notation has been improved. Should make another run for this. * Some example functions break hightlighting due to various combinations of brackets. However changing them means leaving the code style. Fixes: #4726 PR-URL: #4733 Reviewed-By: Roman Reiss <[email protected]>
This changes the code blocks from 4-space indentation to ``` fences for better syntax highlighting and future linting support. Minor On-the-fly changes for typos and highlight breaking markdown have been made. JSON-Style objects have been changed so their closing bracket is on the same line as the opening one. Known issues: * Not every JSON / object notation has been improved. Should make another run for this. * Some example functions break hightlighting due to various combinations of brackets. However changing them means leaving the code style. Fixes: #4726 PR-URL: #4733 Reviewed-By: Roman Reiss <[email protected]>
This changes the code blocks from 4-space indentation to ``` fences for better syntax highlighting and future linting support. Minor On-the-fly changes for typos and highlight breaking markdown have been made. JSON-Style objects have been changed so their closing bracket is on the same line as the opening one. Known issues: * Not every JSON / object notation has been improved. Should make another run for this. * Some example functions break hightlighting due to various combinations of brackets. However changing them means leaving the code style. Fixes: #4726 PR-URL: #4733 Reviewed-By: Roman Reiss <[email protected]>
This changes the code blocks from 4-space indentation to ``` fences for better syntax highlighting and future linting support. Minor On-the-fly changes for typos and highlight breaking markdown have been made. JSON-Style objects have been changed so their closing bracket is on the same line as the opening one. Known issues: * Not every JSON / object notation has been improved. Should make another run for this. * Some example functions break hightlighting due to various combinations of brackets. However changing them means leaving the code style. Fixes: nodejs#4726 PR-URL: nodejs#4733 Reviewed-By: Roman Reiss <[email protected]>
These are likely left over from backporting nodejs#4733.
These are likely left over from backporting #4733. PR-URL: #7681 Reviewed-By: Myles Borins <[email protected]>
These are likely left over from backporting #4733. PR-URL: #7681 Reviewed-By: Myles Borins <[email protected]>
These are likely left over from backporting #4733. PR-URL: #7681 Reviewed-By: Myles Borins <[email protected]>
These are likely left over from backporting #4733. PR-URL: #7681 Reviewed-By: Myles Borins <[email protected]>

As per discussion on #4726
This changes the code blocks from indentation to ``` for better syntax highlighting.
On the fly changes for typos and highlight breaking markdown have been made. Amongst others this includes:
javascript tojsImproved:
JSON / object notation wasn't consistent. Replaced most with:
Known issues:
make another run for this.
I would regard 9df0ba4 as optional. It would be a little nicer to have .md though. The choice of extension is from somewhen in 2010, when I see this right, here.doc: renamed doc files to .md, adapted build scriptsmoved to second PRThis is for consistency and a little less noise for the eye
as in vm.markdown (2 against 8 chars)