natesilva / javascript-eslint.tmbundle

Integrates the ESLint JavaScript validator with TextMate 2
BSD 3-Clause "New" or "Revised" License
46 stars 11 forks source link

ESlint is blocking UI #12

Open koenpunt opened 7 years ago

koenpunt commented 7 years ago

Currently when eslint is running, the editor stops responding. Would it be possible to run it asynchronous/in the background to prevent this lag?

natesilva commented 7 years ago

Not sure how to do this. I’ll keep this open in case I ever get a chance to try making it async.

One thing that may be happening is that, on large files, it can call the mate --set-mark command many times. If a common setting like quote marks or semicolons is different than the file you are looking at, it may need to set hundreds of marks. Currently we set them 10 at a time, so it can get called many times. Maybe this could be increased, I’d need to test that it’s not exceeding some maximum command-line length limit.

koenpunt commented 7 years ago

It actually doesn't matter if it is 1 or more markers, the linting itself is just slow, and while running I can't type anything. Maybe @sorbits can point us in the right direction on how to fix this/make it async?

noniq commented 7 years ago

Same here.

In my RuboCop bundle I’m using TextMate::Executor.run from TextMate’s “Ruby API”. This internally uses TextMate::Process.run, which does all the work of starting a subprocess so that it can run asynchronously while still being able to communicate with it: https://github.com/textmate/bundle-support.tmbundle/blob/master/Support/shared/lib/tm/process.rb

So this is exactly what would be needed here, but unfortunately it’s in Ruby and this bundles code is written in Python. I don’t think TextMate’s (rather limited) Python API has a similar function, but I’m not really familiar with this.

meyer commented 7 years ago

@noniq very cool, thanks for sharing. I had no idea the TextMate Ruby API was a thing.

natesilva commented 7 years ago

So right now it doesn’t use any API, it shells out to the mate command to set the gutter marks, and that’s slow. Can the Ruby API do that? I’m not a Ruby programmer, but if anyone knows a better way to set the gutter marks, or maybe write a wrapper script to the Ruby API, that would help a lot.

noniq commented 7 years ago

To be clear: What I called “Ruby API” is nothing more than a set of Ruby classes and functions, bundled with TextMate (see the “Bundle Support” bundle). 😉 So in principle it would be totally possible to reimplement TextMate::Process.run in a language like Python – no special magic here. I’m not proficient enough in Python to do this, though. But maybe someone else is?

sorbits commented 7 years ago

Maybe @sorbits can point us in the right direction on how to fix this/make it async?

I see there are two ESLint commands in this bundle, which of them is the one that is slow?

sorbits commented 7 years ago

Currently we set them 10 at a time, so it can get called many times. Maybe this could be increased, I’d need to test that it’s not exceeding some maximum command-line length limit.

I think the maximum command line length is 64KB, so this is probably not something to worry about.

For the SCM Diff bundle we set all the +/- gutter marks with only a single call to mate.

koenpunt commented 7 years ago

I see there are two ESLint commands in this bundle, which of them is the one that is slow?

"Save & Validate" blocks any interaction after save, sometimes even for multiple seconds. This means no cursor movement, or anything. The other might not be too quick either, but that one doesn't block anything so that's acceptable.

sorbits commented 7 years ago

For that command, try change output to HTML / New Window.

Then TextMate should run it async automatically, and as TextMate delays opening the HTML output window until there actually is something to show, the command’s use of exit_discard and exit_show_tool_tip should still work.

On 12 Jun 2017, at 21:56, Koen Punt wrote:

I see there are two ESLint commands in this bundle, which of them is the one that is slow?

"Save & Validate" can block any interaction after save for multiple seconds. This means no cursor movement, or anything. The other might not be too quick either, but that one doesn't block anything so that's acceptable.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/natesilva/javascript-eslint.tmbundle/issues/12#issuecomment-307905855

koenpunt commented 7 years ago

For that command, try change output to HTML / New Window.

When I do that, the cursor moves its focus to the new window when the command completes.

natesilva commented 7 years ago

@koenpunt Another quick fix to try is to change main.py at line 176 -- change the number 10 to a much larger number. Based on what @sorbits said, I’m not sure it will solve the problem, but it’s worth a try.

sorbits commented 7 years ago

The new window is a HTML window, correct?

And before changing, did it also open a HTML window, but without moving focus? Because behavior should be the same.

On 12 Jun 2017, at 22:22, Koen Punt wrote:

For that command, try change output to HTML / New Window.

When I do that, the cursor moves its focus to the new window when the command completes.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/natesilva/javascript-eslint.tmbundle/issues/12#issuecomment-307912807

koenpunt commented 7 years ago

try is to change main.py at line 176 -- change the number 10 to a much larger number

I don't think that will make a difference, because even in a file where only a single marker is shown the ui is being blocked.

My python knowledge is minimal, but I do believe it's made for concurrency/multi threading, not sure if that's something we can make use of?

koenpunt commented 7 years ago

The new window is a HTML window, correct?

The new window is indeed a HTML window, but with of course plaintext being rendered.

And before changing, did it also open a HTML window, but without moving focus? Because behavior should be the same.

You mean before the change of output? Than no, before it was just a tooltip with the summary.