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

Add descriptions of enforced rules #1798

Merged
merged 1 commit into from
May 4, 2018
Merged

Conversation

sangaman
Copy link
Contributor

@sangaman sangaman commented May 2, 2018

This commit adds brief descriptions and examples of rules enforced by the guide that were not previously mentioned in the README.

This is related to #1797, although this is not exhaustive and there are still a number of rules not covered in the README nor in this PR.

On a side note, I noticed that some of the HTML <a> anchor links have numbering that's offset by 1, for example the whitespace rules are numbered starting 19 but the anchor links are numbered starting 18. I just continued this pattern in this PR even though it seems incorrect, but I'd be happy to fix these links in a separate PR. It may be possible to just get rid of them too, I'm not sure if they're necessary.

This commit adds brief descriptions and examples of rules enforced by the guide that were not previously mentioned in the README.
@sangaman
Copy link
Contributor Author

sangaman commented May 2, 2018

Hmm looks like it failed the linting due to multiple blank lines, which I put in intentionally for an example of bad code. I can just drop the offending code snippet, but if there's an easy way to keep it without failing linting I'd gladly do that, just let me know.

@ljharb
Copy link
Collaborator

ljharb commented May 2, 2018

The numbering is entirely correct; those are legacy numbered links that were initially correct and are now frozen forever, even as we renumber things moving forward.

You can use specific HTML comments to bypass linting on specific code blocks; see elsewhere in the readme for examples.

README.md Outdated
@@ -1053,6 +1053,22 @@ Other Style Guides
};
```

<a name="whitespace--implicit-arrow-linebreak"></a><a name="8.6"></a>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thus, numbered links are never to be added - only preserved forever on legacy items.

README.md Outdated
@@ -2664,6 +2680,95 @@ Other Style Guides
.fail(() => console.log('You have failed this city.'));
```

<a name="whitespace--block-spacing"></a><a name="18.13"></a>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also here

README.md Outdated
if (foo) { bar = 0; }
```

<a name="whitespace--comma-spacing"></a><a name="18.14"></a>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

README.md Outdated
var arr = [1, 2];
```

<a name="whitespace--computed-property-spacing"></a><a name="18.15"></a>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here


// good
(foo) => bar;
(foo) => (bar);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also good:

(foo) => (
  bar
)

README.md Outdated
// good
obj[foo]
obj['foo']
var x = {[b]: a}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be spaces inside the curly braces here for it to be "good"

README.md Outdated
obj[foo[bar]]
```

<a name="whitespace--func-call-spacing"></a><a name="18.16"></a>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also remove the name here

@sangaman
Copy link
Contributor Author

sangaman commented May 2, 2018

@ljharb Thanks. I'm not finding examples of HTML comments to ignore linting in the readme or elsewhere in the project, do you happen to know an example off the top of your head? If I put the entire block in an html comment, it seems like it would just disappear.

@ljharb
Copy link
Collaborator

ljharb commented May 2, 2018

Oops, you're right - so in this case, it's an MD012 error.

You can surround the section with <!-- markdownlint-disable MD012 --> and <!-- markdownlint-enable MD012 -->

@sangaman sangaman force-pushed the patch-1 branch 2 times, most recently from 92ad4b7 to ea6ca7f Compare May 2, 2018 06:54
@sangaman
Copy link
Contributor Author

sangaman commented May 2, 2018

@ljharb Thanks, looks like that worked.

README.md Outdated
func();
```

<a name="whitespace--key-spacing"></a><a name="18.17"></a>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this numbered anchor also needs to be removed

README.md Outdated
var obj = { "foo": 42 };
```

<a name="whitespace--no-trailing-spaces"></a><a name="18.18"></a>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this numbered anchor also needs to be removed

README.md Outdated
<a name="whitespace--no-trailing-spaces"></a><a name="18.18"></a>
- [19.18](#whitespace--no-trailing-spaces) Avoid trailing spaces at the end of lines. eslint: [`no-trailing-spaces`](https://eslint.org/docs/rules/no-trailing-spaces)

<a name="whitespace--no-multiple-empty-lines"></a><a name="18.19"></a>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this numbered anchor also needs to be removed

@sangaman
Copy link
Contributor Author

sangaman commented May 2, 2018

@ljharb Done, looks like that's all of them.

Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this seems good

@ljharb ljharb requested a review from sharmilajesupaul May 2, 2018 07:15
README.md Outdated
- [19.19](#whitespace--no-multiple-empty-lines) Avoid multiple empty lines and only allow one newline at the end of files. eslint: [`no-multiple-empty-lines`](https://eslint.org/docs/rules/no-multiple-empty-lines)

<!-- markdownlint-disable MD012 -->
```javascript
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments and triple backticks need to be de-indented, it's causing a formatting issue.
See: https://github.com/sangaman/javascript/blob/6a44cf694fc771a87afa564d98d2e797fdede9b2/README.md#whitespace--no-multiple-empty-lines

@sangaman
Copy link
Contributor Author

sangaman commented May 2, 2018

@sharmilajesupaul Good catch. De-indenting made the code block stretch to the left unlike the other blocks. But I tried indenting the html comments in line with the backticks, and everything looks good.

@ljharb ljharb merged commit edf942e into airbnb:master May 4, 2018
@sangaman sangaman deleted the patch-1 branch May 4, 2018 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants