1292035 - Enable the space-before-blocks rule for eslint

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

This rule requires that blocks have a space before them, for example:
```
function(){
  return true;
}
```
is wrong

```
function() {
  return true;
}
```
is right

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1292035 - Enable the space-before-blocks rule for eslint. This patch was generated using 'eslint --fix'.

Review commit: https://reviewboard.mozilla.org/r/69186/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69186/

Comment 2

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8777671 [details]
Bug 1292035 - Enable the space-before-blocks rule for eslint. This patch was generated using 'eslint --fix'.

https://reviewboard.mozilla.org/r/69186/#review66280

::: browser/extensions/pocket/content/panels/js/signup.js:173
(Diff revision 1)
>      }
>  }
>  
>  $(function()
>  {
> -    if(!window.thePKT_SIGNUP){
> +    if(!window.thePKT_SIGNUP) {

I think we should add a space after the |if| here while we are touching the file (is there an eslint rule for that?) - this file doesn't seem to consistently avoid it.

::: toolkit/components/microformats/microformat-shiv.js:81
(Diff revision 1)
>              options = (options)? options : {};
>              this.mergeOptions(options);
>              this.getDOMContext( options );
>  
>              // if we do not have any context create error
> -            if(!this.rootNode || !this.document){
> +            if(!this.rootNode || !this.document) {

this file (and a couple that follow) also leaves the spaces before the |if|, but this file seems to hate spaces *everywhere*, so it doesn't weem worth fixing :( These files actually becomes less consistent after this patch, but I guess it's a small step towards more consistency in general, so probably helps more than it hurts...

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Mark Hammond [:markh] from comment #2)
> Comment on attachment 8777671 [details]
> Bug 1292035 - Enable the space-before-blocks rule for eslint. This patch was
> generated using 'eslint --fix'.
> 
> https://reviewboard.mozilla.org/r/69186/#review66280
> 
> ::: browser/extensions/pocket/content/panels/js/signup.js:173
> (Diff revision 1)
> >      }
> >  }
> >  
> >  $(function()
> >  {
> > -    if(!window.thePKT_SIGNUP){
> > +    if(!window.thePKT_SIGNUP) {
> 
> I think we should add a space after the |if| here while we are touching the
> file (is there an eslint rule for that?) - this file doesn't seem to
> consistently avoid it.

Yes, that rule is "keyword-spacing" and I am almost finished with that patch.
 
> ::: toolkit/components/microformats/microformat-shiv.js:81
> (Diff revision 1)
> >              options = (options)? options : {};
> >              this.mergeOptions(options);
> >              this.getDOMContext( options );
> >  
> >              // if we do not have any context create error
> > -            if(!this.rootNode || !this.document){
> > +            if(!this.rootNode || !this.document) {
> 
> this file (and a couple that follow) also leaves the spaces before the |if|,
> but this file seems to hate spaces *everywhere*, so it doesn't weem worth
> fixing :( These files actually becomes less consistent after this patch, but
> I guess it's a small step towards more consistency in general, so probably
> helps more than it hurts...

After a few more of these rules land, and eventually if we can get the "indent" rule to land, then this file will be much closer to conformity with the rest of our codebase. Baby steps.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8777671 [details]
Bug 1292035 - Enable the space-before-blocks rule for eslint. This patch was generated using 'eslint --fix'.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69186/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd861f275e09
Enable the space-before-blocks rule for eslint. This patch was generated using 'eslint --fix'. r=markh

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fd861f275e09