Skip to content
This repository has been archived by the owner on Nov 10, 2022. It is now read-only.

Can we show auto-fix results in the demo? #322

Closed
platinumazure opened this issue Jan 4, 2017 · 12 comments
Closed

Can we show auto-fix results in the demo? #322

platinumazure opened this issue Jan 4, 2017 · 12 comments
Assignees
Labels
demo Relates to the ESLint demo page (https://eslint.org/demo) enhancement This change enhances an existing feature of the website

Comments

@platinumazure
Copy link
Member

Now that the demo has been rewritten, it would be awesome if we could show auto-fix results! (The motivation for this is to provide a quick way to verify incoming bugs that focus on auto-fix issues.)

It would be awesome if we could show auto-fix results without clobbering the original code, too (or at least allow the user to show fixed/broken code via a toggle maybe?).

Possible tasks (may be inaccurate):

  • Ensure source-code-fixer, rule-fixer, etc. are included in the browserify'd ESLint so the web demo can access those modules (might require creating a WebEngine API as noted in other discussions?)
  • Ensure fix:true is passed to the linting call so fix info and output are available
  • Add control to demo for showing fixed output
@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jan 4, 2017
@platinumazure platinumazure added demo Relates to the ESLint demo page (https://eslint.org/demo) enhancement This change enhances an existing feature of the website and removed triage An ESLint team member will look at this issue soon labels Jan 4, 2017
@ilyavolodin
Copy link
Member

I looked into it while rewriting the demo. As you mentioned, it's not something we can do without changes to the core, since CLIEngine is not browserified (nor should it). I think approach with WebEngine API is incorrect. We would need to add WebEngine API into the core, and that doesn't make much sense to me, since we specifically telling our customers that the only platform that we support is Node.
I think the more correct approach would be to split auto-fix API into two parts, fixing and saving to a file. Saving to a file part should stay in CLIEngine, but fixing should be moved into linter object. I'm still not 100% sure why @nzakas put auto-fixing into CLIEngine.

@nzakas
Copy link
Member

nzakas commented Jan 6, 2017

Only writing to a file is in CLIEngine. The main functionality is in SourceCodeFixer.

@ilyavolodin
Copy link
Member

Ah, you are correct. In that case, we should expose SourceCodeFixer in the linter object (if it's not already exposed. But I don't think it is).

@not-an-aardvark
Copy link
Member

not-an-aardvark commented Jan 7, 2017

The functionality for applying a single round of fixes is in SourceCodeFixer, but the multipass logic is in CLIEngine (here). As far as the demo is concerned, it would be nice to be able to look at multipass autofix results, since the intermediate autofix results aren't really relevant to the user most of the time.

@nzakas
Copy link
Member

nzakas commented Jan 10, 2017

I don't think we should be tacking things onto linter just to expose them to the demo. Id much rather go the WebEngine route and do this is an organized way.

@gyandeeps
Copy link
Member

I will work on this once eslint/eslint#8631 is merged.

@gyandeeps gyandeeps self-assigned this May 22, 2017
@gyandeeps
Copy link
Member

gyandeeps commented Jun 19, 2017

WIP - Once 4.1.0 is released then I can update the demo site

image

I am not a designer so plz feel free to give some suggestions. 😄

@gyandeeps
Copy link
Member

@eslint/eslint-team any thoughts on the design ☝️ ?

@ilyavolodin
Copy link
Member

Why do we need to show fixed code in separate window? Let's just replace original code with fixed (need to add some sort of button to activate fix mode).

@platinumazure
Copy link
Member Author

platinumazure commented Jun 22, 2017 via email

@gyandeeps
Copy link
Member

gyandeeps commented Jun 22, 2017

I like side by side also as I maybe testing some rule and interactively trying things. Otherwise it will be so annoying to see my code vanish as soon as i stop typing. Its easy to see the changes applied (for context sake).

@ilyavolodin
Copy link
Member

In that case, current design looks good to me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
demo Relates to the ESLint demo page (https://eslint.org/demo) enhancement This change enhances an existing feature of the website
Projects
None yet
Development

No branches or pull requests

6 participants