1217922 - eslint head.js plugin does not seem to work

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Tom Tromey :tromey]

I have the eslint plugins installed and I have emacs set up to run
eslint.  This works in most cases.

However, the head.js plugin does not seem to work.
I opened

devtools/client/styleinspector/test/browser_computedview_keybindings_01.js

and I see multiple eslint errors; here's a brief (and mildly cryptic) summary:

   19   9 error    no-... "addTab" is not defined.... (javascript-eslint)
   20  33 error    no-... "openComputedView" is not defined.... (javascript-eslint)
   21   9 error    no-... "selectNode" is not defined.... (javascript-eslint)
   28  32 error    no-... "once" is not defined.... (javascript-eslint)

Comment 1

[Patrick Brosset <:pbro>]

Interestingly, it didn't work for me on Windows, but after I fixed the path as described in bug 1217851 it did work just fine. All globals like addTab were being defined.

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

@Tom: Was this on a Windows box?

Comment 3

[Tom Tromey :tromey]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #2)
> @Tom: Was this on a Windows box?

Nope, Linux.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

It does not appear to work for me on Mac either.

Comment 5

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Can you try again with the latest version and report any errors?

Comment 6

[Tom Tromey :tromey]

I tried it today.

It still seems to fail.  I opened devtools/client/webconsole/test/browser_webconsole_output_02.js
I get some warnings which are correct, and then also:

  181  25 error    no-... "loadTab" is not defined.... (javascript-eslint)
  182  23 error    no-... "openConsole" is not defined.... (javascript-eslint)
  183  11 error    no-... "checkOutputForInputs" is not defined.... (javascript-eslint)
  185  11 error    no-... "finishTest" is not defined.... (javascript-eslint)

All of these are defined in the corresponding head.js.

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

This should hopefully be fixed by bug 1225289.

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

We are getting the process's working directory but on some OSes that is not necessarily the same as the target file's directory.

I believe that I have a simple fix.

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: MozReview Request: Bug 1217922 - eslint head.js plugin does not seem to work r?=pbrosset

Bug 1217922 - eslint head.js plugin does not seem to work r?=pbrosset

(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #12)
> Comment on attachment 8690134 [details]
> MozReview Request: Bug 1217922 - eslint head.js plugin does not seem to work
> r?=pbrosset
>
> https://reviewboard.mozilla.org/r/25771/#review23289
>
> ::: testing/eslint-plugin-mozilla/lib/rules/import-headjs-globals.js:95
> (Diff revision 1)
> > -      checkFile([testPath, headjs], context);
> > +      checkFile([fullTestPath, fullHeadjsPath]);
>
> You're not passing context anymore here as the 2nd argument. Is this
> intentional?
> The checkFile function's signature wasn't changed in this commit:
> `function checkFile(fileArray, context)`
> so it will still expect a `context` parameter.

Well spotted... this is a simple mistake caused by breaking apart a much larger patch.

Comment 10

[Patrick Brosset <:pbro>]

Comment on attachment 8690773 [details]
MozReview Request: Bug 1217922 - eslint head.js plugin does not seem to work r?=pbrosset

https://reviewboard.mozilla.org/r/25943/#review23333

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8739d8da1b0d

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8739d8da1b0d