Only run diagnostics onSave

[lorenzo]

This introduces a behaviour change in that hie will only run diagnostics on a file after it has been open and saved, unless the user hasn't manually configured the client to run diagnostics on chages.

I have found that running diagnostics as I type creates too much noise in my editor as more often than not I'm in a broken state. Additionally, this it makes HIE appear that it runs faster.

• Added the diagnosticsOnChange client config value
• Refactored the reactor a bit to not have hardcoded defaults smattered everywhere, which in some cases were inconsistent.

[lukel97]

I’m confused as to how the tests are passing, since they aren’t sending any didSave notifications.

[lorenzo]

@bubba only 2 tests where calling noDiagnostics after executing a text change. The other tests were only asserting that diagnostics were returned after a file was opened.

[lukel97]

@lorenzo some of the tests do send applyedit requests though:

haskell-ide-engine/test/functional/CompletionSpec.hs

Lines 16 to 30 in 4c64789

	it "works" $ runSession hieCommand fullCaps "test/testdata/completion" $ do
	doc <- openDoc "Completion.hs" "haskell"
	_ <- skipManyTill loggingNotification (count 2 noDiagnostics)
	let te = TextEdit (Range (Position 5 7) (Position 5 24)) "put"
	_ <- applyEdit doc te
	compls <- getCompletions doc (Position 5 9)
	let item = head $ filter ((== "putStrLn") . (^. label)) compls
	liftIO $ do
	item ^. label `shouldBe` "putStrLn"
	item ^. kind `shouldBe` Just CiFunction
	item ^. detail `shouldBe` Just "String -> IO ()\nPrelude"
	item ^. insertTextFormat `shouldBe` Just Snippet
	item ^. insertText `shouldBe` Just "putStrLn ${1:String}"

[lukel97]

+1 on the addition of of the config option, but I'm not so sure on making onSave the default. I can imagine this would cause confusion for people upgrading when diagnostics are no longer being returned immediately. Are there any other language servers that only return on save by default? Most of the builtin VS code language servers (JS/python etc) seem to run diagnostics on change.
Hopefully with the upcoming changes the BIOS and the new .hie files, the latency with running and fetching diagnostics won't be so bad.

[lorenzo]

Are there any other language servers that only return on save by default

All of the language servers I used for work do it on save: Golang, Python, Elm, PHP... The one I struggle with the most is the Haskell one because of all the constant noise I get while I'm in an obviously broken state as I move stuff around. I'm confident most people must be having the same type of problems as I do.

The problem is not the latency, which is also annoying, but the constant showing of errors as I type.

Edit: I just realised that I changed the python's lang server behaviour for the exact same problem. Way too much noise as I typed.

[lorenzo]

some of the tests do send applyedit requests though:

Right, but the test is not doing any assertions on the diagnostics after changing the test. The only thing that I changed here is running diagnostics, the rest is kept unchanged.

[alanz]

I agree that the default should be to preserve the current behaviour, rather than only generating diagnostics on save.

[lukel97]

The problem is not the latency, which is also annoying, but the constant showing of errors as I type.

I do agree though that we should definitely support this workflow in some form. I can also imagine a wide variety of scenarios such as having really large modules which take a long time to typecheck where on save behaviour is probably preferred.

(Accidentally closed this, please ignore!)

[lorenzo]

Ok, seems there does not seem to be lots of support for making this the default, I'll make it a configuration option only and revert to the previous default.

[lorenzo]

@bubba @alanz I set the default to True to preserve the old behaviour. This can be merged whenever you like