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

feedback re: recent developments (ast branch et al) #8

Closed
AntonioMeireles opened this issue Dec 10, 2019 · 5 comments
Closed

feedback re: recent developments (ast branch et al) #8

AntonioMeireles opened this issue Dec 10, 2019 · 5 comments

Comments

@AntonioMeireles
Copy link

@helixbass

First of all let me give you an huge THANK YOU for your epic effort on this (plus ast branch of coffeescript and prettier plugin)! It's a much needed thing that will IMHO inject new life in the coffeescript ecosystem and make it much more viable long term :-)

Now, two small bugs ...

  • at sight, global vars are not being recognized by eslint and end always being set as undefined.
  • setting coffee/keyword-spacing: 1 breaks with ...
[Error - 11:47:20 AM] TypeError: Cannot read property 'before' of undefined
Occurred while linting /Users/am/hacks/csirt-reporter/lib/commands/parser.coffee:446
    at checkSpacingBefore (/Users/am/hacks/csirt-reporter/node_modules/eslint-plugin-coffee/lib/rules/keyword-spacing.js:374:44)
    at checkSpacingAround (/Users/am/hacks/csirt-reporter/node_modules/eslint-plugin-coffee/lib/rules/keyword-spacing.js:380:9)
    at checkSpacingAroundFirstToken (/Users/am/hacks/csirt-reporter/node_modules/eslint-plugin-coffee/lib/rules/keyword-spacing.js:387:18)
    at /Users/am/hacks/csirt-reporter/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>:null:null)
    at Object.emit (/Users/am/hacks/csirt-reporter/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/Users/am/hacks/csirt-reporter/node_modules/eslint/lib/linter/node-event-generator.js:254:26)
    at NodeEventGenerator.applySelectors (/Users/am/hacks/csirt-reporter/node_modules/eslint/lib/linter/node-event-generator.js:283:22)
    at NodeEventGenerator.enterNode (/Users/am/hacks/csirt-reporter/node_modules/eslint/lib/linter/node-event-generator.js:297:14)
    at CodePathAnalyzer.enterNode (/Users/am/hacks/csirt-reporter/node_modules/eslint-plugin-coffee/lib/code-path-analysis/code-path-analyzer.js:573:21)
    at /Users/am/hacks/csirt-reporter/node_modules/eslint/lib/linter/linter.js:936:32
    at Array.forEach (<anonymous>:null:null)
    at runRules (/Users/am/hacks/csirt-reporter/node_modules/eslint/lib/linter/linter.js:931:15)
    at Linter._verifyWithoutProcessors (/Users/am/hacks/csirt-reporter/node_modules/eslint/lib/linter/linter.js:1157:31)
    at Linter._verifyWithConfigArray (/Users/am/hacks/csirt-reporter/node_modules/eslint/lib/linter/linter.js:1255:21)
    at Linter.verify (/Users/am/hacks/csirt-reporter/node_modules/eslint/lib/linter/linter.js:1210:25)
    at Linter.verifyAndFix (/Users/am/hacks/csirt-reporter/node_modules/eslint/lib/linter/linter.js:1400:29)
    at verifyText (/Users/am/hacks/csirt-reporter/node_modules/eslint/lib/cli-engine/cli-engine.js:230:48)
    at CLIEngine.executeOnText (/Users/am/hacks/csirt-reporter/node_modules/eslint/lib/cli-engine/cli-engine.js:885:26)
    at /Users/am/.vscode/extensions/dbaeumer.vscode-eslint-1.9.1/server/out/eslintServer.js:1:60107
    at /Users/am/.vscode/extensions/dbaeumer.vscode-eslint-1.9.1/server/out/eslintServer.js:1:61116

relevant stuff in package.json is ...

"devDependencies": {
    "coffeescript": "github:jashkenas/coffeescript#265b251a",
    "docco": "^0.8.0",
    "eslint": "^6.7.2",
    "eslint-config-prettier": "^6.7.0",
    "eslint-plugin-coffee": "^0.1.1",
    "eslint-plugin-import": "^2.19.1",
    "eslint-plugin-jest": "^23.1.1",
    "eslint-plugin-prettier": "^3.1.1",
    "eslint-plugin-react": "^7.17.0",
    "prettier": "github:helixbass/prettier#9105f5f1",
    "prettier-plugin-coffeescript": "^0.1.1"
  }

all the best,

António

@helixbass
Copy link
Owner

First of all let me give you an huge THANK YOU

You're welcome! I hope that it will improve quality-of-life for developing in Coffeescript, I know I've been enjoying using ESLint + Prettier 👍

Glad you figured out how to get the plugin running in your project. I'd started writing the README spec'ing the dependency as coffeescript@2.5.0 which doesn't exist yet, but I updated the installation instructions to specify current ast branch

I'd recommend updating your Coffeescript dependency to latest ast branch though: github:jashkenas/coffeescript#05d45e9b. I updated this README and the Prettier plugin README to reflect this

I'm curious from your stack trace, did you get ESLint running inside VSCode?

at sight, global vars are not being recognized by eslint and end always being set as undefined.

Could you give an example of code/ESLint config that is failing? I can do a deeper dive on how global vars work in ESLint but will be helpful to have a failing example

setting coffee/keyword-spacing: 1 breaks with

Ok I hadn't seen this breaking while "dogfooding" the ESLint plugin since this rule was turned off for Prettier. But turned it on and was able to recreate some failures of keyword-spacing

So that should be fixed, I released as eslint-plugin-coffee@0.1.2, can you try?

Thanks for the early feedback!

@AntonioMeireles
Copy link
Author

@helixbass

  • can confirm that eslint-plugin-coffee@0.1.2 fixes the coffee/keyword-spacing: 1 as i couldn't reproduce it anymore :-) THANKS!
  • regarding the globals ...
     global.foo = 'bar'
     x = () -> console.log foo
    just throws eslint(no-undef) for foo
  • re: vsCode. using it with the stock prettier and eslint plugins without major issues.
    only needed tweaks are in vscodes' settings.json...
"eslint.validate": [
       (...) 
        {
            "language": "coffeescript",
            "autoFix": true
        },
    ],

(...) 
    "[coffeescript]": {
        "editor.defaultFormatter": "esbenp.prettier-vscode"
    },
 

IMHO getting top notch integration with vscode (the emacs of this century - like it or not) may be all that is needed to...

All the Best,

António

@helixbass
Copy link
Owner

regarding the globals

Ok based on reading through eslint/eslint#11333 it looks like ESLint isn't tracking assignments to global (but that in theory it could be an option)

So looks like as of now the recommended solution would be eg:

### global foo ###
global.foo = 'bar'
x = () -> console.log foo

re: vsCode

Cool I will install VSCode and try adjusting settings.json per your description. Then I can add those instructions to the README

IMHO getting top notch integration with vscode

Yes I agree VSCode is # 1 on the list. Let me know if you have any idea of what a top notch integration with VSCode would probably look like (ie if there's a better option than just adjusting settings.json manually)

@helixbass
Copy link
Owner

Wow ya that was really easy to get ESLint and Prettier working in VSCode. Added instructions to this README and the Prettier plugin README, thanks!

@AntonioMeireles
Copy link
Author

@helixbass fwiw / fyi just filled microsoft/vscode-eslint#825 to plead for fix of only the 'major' issue i found so far in vscode's side && thanks again :-)

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

2 participants