Closed Bug 1292035 Opened 3 years ago Closed 3 years ago

Enable the space-before-blocks rule for eslint

Categories

(Toolkit :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: jaws, Assigned: jaws)

Details

Attachments

(1 file)

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

```
function() {
  return true;
}
```
is right
Attachment #8777671 - Flags: review?(markh) → review+
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...
(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 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/
Pushed by jwein@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/fd861f275e09
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.