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

[ESLint] Why are padded blocks not permitted? #483

Closed
chrisngobanh opened this issue Aug 22, 2015 · 9 comments
Closed

[ESLint] Why are padded blocks not permitted? #483

chrisngobanh opened this issue Aug 22, 2015 · 9 comments

Comments

@chrisngobanh
Copy link
Contributor

I don't quite understand why the ESLint config file prohibits padded blocks. What would happen if you needed to make a comment after a block opens:

// ESLint Error: "Block must not be padded by blank lines. (padded-blocks)"
$document.ready(() => {

  // Initialize/Render React
  React.render(React.createElement(Index), $react.get(0));
  ...
}

// Violates 17.2 of the style guide: "Put an empty line before the [single-line] comment."
$document.ready(() => {
  // Initialize/Render React
  React.render(React.createElement(Index), $react.get(0));
  ...
}

In addition, I don't see anywhere in the style guide where padded blocks are prohibited.

@justjake
Copy link
Collaborator

Yeah, the eslint config is out of sync with our actual styleguide in a number of places. I am working on a test suite to bring them together here:

#480

@justjake
Copy link
Collaborator

However in this case I think we should amend the styleguide to say that a comment on the first line of a block does not need a preceding line.

@chrisngobanh
Copy link
Contributor Author

I don't see anywhere in the style guide that mentions padded blocks, so maybe we should make a change in the styleguide to address it as well.

chrisngobanh added a commit to chrisngobanh/javascript that referenced this issue Oct 16, 2015
chrisngobanh added a commit to chrisngobanh/javascript that referenced this issue Oct 16, 2015
In airbnb#483, justjake said that we should update the style guide to allow
single line comments in the first line of a block.
@chrisngobanh
Copy link
Contributor Author

#547, which was recently merged, addresses this issue by adding a section to the style guide that prohibits padded blocks.

dustinmartin pushed a commit to dustinmartin/javascript that referenced this issue Jan 4, 2016
dustinmartin pushed a commit to dustinmartin/javascript that referenced this issue Jan 4, 2016
In airbnb#483, justjake said that we should update the style guide to allow
single line comments in the first line of a block.
gilbox pushed a commit to gilbox/javascript that referenced this issue Mar 21, 2016
gilbox pushed a commit to gilbox/javascript that referenced this issue Mar 21, 2016
In airbnb#483, justjake said that we should update the style guide to allow
single line comments in the first line of a block.
@jbcpollak
Copy link

I know this question is closed, but I am wondering if there is a justification for why padded blocks aren't allowed? I noticed both AirBnB and Google's styles disallow it, so we are following the same convention, but we are wondering why.

@ljharb
Copy link
Collaborator

ljharb commented Mar 10, 2017

Whitespace is beneficial when it adds readability, but not when it creates visual noise. With syntax highlighting, lines padding blocks add zero benefit, but increase the vertical length of code, and reduce the amount you can see at once.

@jbcpollak
Copy link

@ljharb - good enough for me, thanks! Would be awesome to see that in the style guide (I often wish style guides had more justifications) but that is very helpful.

jaylaw81 pushed a commit to appirio-digital/ads-best-practices that referenced this issue Sep 19, 2017
jaylaw81 pushed a commit to appirio-digital/ads-best-practices that referenced this issue Sep 19, 2017
In airbnb#483, justjake said that we should update the style guide to allow
single line comments in the first line of a block.
@dietergeerts
Copy link

dietergeerts commented Jun 9, 2018

@ljharb , that's not true for all cases. In several cases, adding a blank line add a HUGE readability improvement to the code, and it outweighs the fact that the code gets longer by 1 single line. For example in a test file:

import { withJsonDefault } from './with-json-default';

describe('withJsonDefault', () => {

  it(`must use the serializer of the given schema directly,
    as defaults are only meant to be used on deserialization,
    where we have no control over the actual json coming through.`, () => {
    const primitive = _clone(PRIMITIVE);
    sinon.spy(primitive, 'serialize');
    const schema = withJsonDefault('B', primitive);

vs

import { withJsonDefault } from './with-json-default';

describe('withJsonDefault', () => {
  it(`must use the serializer of the given schema directly,
    as defaults are only meant to be used on deserialization,
    where we have no control over the actual json coming through.`, () => {
    const primitive = _clone(PRIMITIVE);
    sinon.spy(primitive, 'serialize');
    const schema = withJsonDefault('B', primitive);

So what's the real reason for not allowing the first line of a block to be empty? Myself, I never pad a block at the end, but doing it in the beginning give way more readable code in such cases.

If you want to reduce visual noise, then there also should be no spaces inside object declarations as well imho... So I believe that those both rules are conflicting with the reason behind them. Imho, a styleguide should have some general principles, and use them throughout, unless there is a very good reason not to adhere it.

@ljharb
Copy link
Collaborator

ljharb commented Jun 9, 2018

Personally i find the latter case much more readable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants