Conversation
kokobd
requested review from
OliverMadine,
berberman,
jneira and
pepeiborra
as code owners
|
I'm receiving a lot of logs like this if So disabling it for now |
|
I see. In haskell-language-server/exe/Main.hs Lines 72 to 82 in 347a718 Then it goes into infinite recursion... So we should not enable debug logs in |
|
Hmm, I thought I had convinced myself that wasn't a problem :| I'm not 100% sure what to do about this. It's nice to be able to log the responses from LSP, but we do indeed have a loop problem if we then forward them on like this. |
And some other tests depend on the logs, I had to fix them one by one. This makes the PR harder than I initially thought. Currently I'm working on hlint tests |
|
Finally, the CI is almost green. |
| instance Hashable Location | ||
| instance Hashable Range | ||
| instance Hashable Position | ||
| instance Hashable UInt |
There was a problem hiding this comment.
Instances have been added to lsp-types-1.5. This is not a question or complaint, just a note for the next reviewer
| backlog <- readTVarIO $ dirtyKeys shakeExtras | ||
| queue <- atomicallyNamed "actionQueue - peek" $ peekInProgress $ actionQueue shakeExtras | ||
|
|
||
| -- this log is required by tests |
|
|
||
| let lspCologAction :: MonadIO m2 => Colog.LogAction m2 (Colog.WithSeverity LspServerLog) | ||
| lspCologAction = toCologActionWithPrio $ cfilter | ||
| (\msg -> priority msg >= Info) |
There was a problem hiding this comment.
This is to filter out the bad debug logs from lsp right? I'd like to come up with a better solution there. Can you open a ticket in lsp and link to it here, so we remember why we're doing this? At the moment this means that the low-level message logs won't even go to the file logging, which is bad since they're useful.
An alternative solution could be to change the log action in HLS that sends logs to the client to never send debug logs. But I guess that's also surprising :|
| Warning -> MtWarning | ||
| Error -> MtError | ||
|
|
||
| toCologActionWithPrio :: (MonadIO m, HasCallStack) => Recorder (WithPriority msg) -> LogAction m (WithSeverity msg) |
There was a problem hiding this comment.
👍 Worth checking if there's anywhere else doing this already? I can't remember if there is...
There was a problem hiding this comment.
This is the first time co-log-core was introduced in HLS, so I think there isn't a existing function for this.
ghcide/test/exe/Main.hs
Outdated
| pure (rope, range, replacement) | ||
| monadicIO $ forAllM gen | ||
| $ \(rope, range, replacement) -> do | ||
| newRope <- liftIO $ applyChange (toCologActionWithPrio $ cmapWithPrio LogVfs recorder) rope (TextDocumentContentChangeEvent (Just range) Nothing replacement) |
There was a problem hiding this comment.
Why not pass mempty :: LogAction Identity here? then you don't need IO, and you'll just not get hte log messages which you don't care about anyway. Then I think you can keep the old code.
* upgrade lsp to 1.5 * fix stack.yaml * try fix tests * disable verbose logging in ghcide * fix more tests in ghcide * fix floskell test * disable debug log in func-test * disable debug log in lsp itself * Revert "disable debug log in func-test" This reverts commit 1fd6658. * remove unused import * fix hls test utils * upgrade lsp in nix * fix func-tests * Revert "fix func-tests" This reverts commit 2ecd76d. * fix waitForDiagnosticsFromSourceWithTimeout * use Null as dummy message in waitForDiagnosticsFromSourceWithTimeout * simplify a test case * add comment about lsp bad logs
3 participants
This is a prerequisite of #3046