Format on save

[tomv564]

For issue #8

Adds support for sync request (blocking the editor for up to 1 second)

Default off, enable by setting lsp_format_on_save to true in preferences.

[coveralls]

Coverage remained the same at 36.015% when pulling c190a69 on format-on-save into 2072a12 on master.

[rwols]

I have something to say about this, but I need a little more time, so please don't merge this yet.

[rchl]

It might be related to this PR or might not but I think this will not work for eslint server. I believe (after a short look at its code) that the way eslint server formats code on saving, is that it expects to get textDocument/willSaveWaitUntil notification and from there it handles the formatting.

Some references:

• https://github.com/microsoft/vscode-languageserver-node/blob/master/server/src/main.ts#L282-L288
• https://github.com/microsoft/vscode-eslint/blob/52a3bba443b92fdf33ada0758eeeb2b93aa387d8/server/src/eslintServer.ts#L679-L698

[tomv564]

@rchl good to know, eslint doesn't have a formattingProvider so willSaveWaitUntil is the only way to make it work. I think the implementation would be very similar to this one, if you're keen on trying it out!

[rchl]

@tomv564 I'm quite intimidated by this code base and didn't have time to properly learn it. So we'll see if I get the courage. :)

[rwols]

Tried this locally and it works when there haven't been recent changes to the document. When the user types a bunch of characters and then hits CTRL+S the textDocument/didChange notification is sent out too late. Interesting to see the use of view.settings() directly. The while-loop is unncessarily busy on CPUs and we can do better with a simple condition variable.

• We should use a condition variable instead of a busy while loop,
• We should send textDocument/didChange just before doing the formatting.

[tomv564]

Good reminder from @rwols that this is the first time we introduce a regular sublime setting (at the view level) instead of an LSP setting. I didn't think much about it beyond the necessity of turning this feature on granularly (per project/syntax) - is it the best way to solve this?

[tomv564]

@rchl I added the willSaveWaitUntil logic to this PR as it was so similar. If you checkout branch workspace_configuration_request then you will get these changes and a minimal workspace/configuration implementation for the eslint server.

[rchl]

@tomv564 It does actually work (after I've adjusted my LSP-eslint to expose setting from a correct place). Very nice, thank you for your work. :)

As for the workspace/configuration changes, I guess that will be discussed separately.

[rwols]

I didn't think much about it beyond the necessity of turning this feature on granularly (per project/syntax) - is it the best way to solve this?

There should be a description in the user docs on how to enable this at least, right?