Skip to content
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: strip trailing whitespace #9620

Merged

Conversation

sam-github
Copy link
Contributor

@sam-github sam-github commented Nov 15, 2016

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

lines should not end in trailing whitespace

@sam-github sam-github added the doc Issues and PRs related to the documentations. label Nov 15, 2016
@sam-github
Copy link
Contributor Author

#9611 indicated that trailing whitespace had crept into the docs.

@Fishrock123
Copy link
Contributor

Fishrock123 commented Nov 15, 2016

Ugh. See https://github.com/nodejs/node/blob/master/.editorconfig#L12-L13 -- the problem is, for whatever silly reason, that this actually means something in markdown.

I think it makes a newline break, but I forget.

FWIW I find that incredibly silly and think we should ignore that bit of markdown.

@sam-github
Copy link
Contributor Author

Ugh, indeed. I will review the docs visually for every change here to ensure that the whitespace was semantic, not accidental.

@silverwind
Copy link
Contributor

silverwind commented Nov 15, 2016

+1 for dropping that option. I was initially for it, but I'm not aware that anyone really uses it. There's always <br> to force a linebreak if necessary (which is essentially what it does).

@gibfahn
Copy link
Member

gibfahn commented Nov 18, 2016

@Fishrock123 I'm as hazy on Markdown as everyone else, but I think the newline is only for two spaces at the end of the line. I don't think a single space at the end of a file achieves anything.

Ref

PR-URL: nodejs#9620
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@sam-github sam-github force-pushed the strip-trailing-whitespace-from-docs branch from 50c528b to a03074a Compare November 18, 2016 17:28
@sam-github sam-github merged commit a03074a into nodejs:master Nov 18, 2016
sam-github added a commit to sam-github/node that referenced this pull request Nov 18, 2016
markdown had a dispensation because 2 or more trailing spaces triggers a
new paragraph. There are no examples of that usage in Node, all trailing
whitespace found were mistakes, and the dispensation is now removed.

See: nodejs#9620
@sam-github
Copy link
Contributor Author

I manually reviewed this, no trailing whitespace was semantic, and would have to have been 2 or more spaces to be semantic, thanks @gib for the reference. I PRed a removal of the .editorconfig section that you pointed out, @Fishrock123: #9676

@sam-github sam-github deleted the strip-trailing-whitespace-from-docs branch November 18, 2016 17:48
sam-github added a commit that referenced this pull request Nov 21, 2016
markdown had a dispensation because 2 or more trailing spaces triggers a
new paragraph. There are no examples of that usage in Node, all trailing
whitespace found were mistakes, and the dispensation is now removed.

See: #9620
PR-URL: #9676
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
addaleax pushed a commit that referenced this pull request Nov 22, 2016
PR-URL: #9620
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Nov 22, 2016
markdown had a dispensation because 2 or more trailing spaces triggers a
new paragraph. There are no examples of that usage in Node, all trailing
whitespace found were mistakes, and the dispensation is now removed.

See: #9620
PR-URL: #9676
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 13, 2016
PR-URL: #9620
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 20, 2016
markdown had a dispensation because 2 or more trailing spaces triggers a
new paragraph. There are no examples of that usage in Node, all trailing
whitespace found were mistakes, and the dispensation is now removed.

See: #9620
PR-URL: #9676
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
PR-URL: #9620
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
markdown had a dispensation because 2 or more trailing spaces triggers a
new paragraph. There are no examples of that usage in Node, all trailing
whitespace found were mistakes, and the dispensation is now removed.

See: #9620
PR-URL: #9676
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Dec 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants