Refactor delayedProgressReporting lsp progress api #4206
Conversation
soulomoon
changed the title
| LSP.sendNotification SMethod_Progress $ | ||
| LSP.ProgressParams | ||
| { _token = token | ||
| , _value = case optProgressStyle of |
There was a problem hiding this comment.
We've lost this progress-style configurability, but honestly, configurability is bad, so better to get rid of it IMO!
There was a problem hiding this comment.
We should get rid of the option as well, I guess.
| inProgress = updateStateForFile inProgressState | ||
| return ProgressReporting{..} | ||
| where | ||
| lspShakeProgress InProgressState{..} = do | ||
| lspShakeProgressNew InProgressState{..} = do | ||
| -- first sleep a bit, so we only show progress messages if it's going to take |
There was a problem hiding this comment.
Ah, I would actually really like to do this in lsp as well, see haskell/lsp#549
|
good suggestion. Swtiching to |
| updateState start (Event KickStarted) (Running a) = cancel a >> Running <$> start | ||
| updateState _ (Event KickCompleted) (Running a) = cancel a $> NotStarted | ||
| updateState start (Event KickStarted) NotStarted = pure $ Running start | ||
| updateState start (Event KickStarted) (Running a) = signalBarrier a () $> Running start |
There was a problem hiding this comment.
Hmmm. Does this work? We signal the barrier and then go back into the running state, but won't the barrier have cancelled the whole progress reporting??
There was a problem hiding this comment.
we cancel the previous job and leave the new job cancellation here.
| cancelProgress <- newBarrier | ||
| LSP.runLspT lspEnv $ withProgress "Processing" Nothing Cancellable $ \update -> | ||
| race (liftIO $ waitBarrier cancelProgress) (loop update 0) | ||
| return cancelProgress |
There was a problem hiding this comment.
This can't work: withProgress runs until the inner operation terminates! So we won't ever get here!
There was a problem hiding this comment.
(Sorry, I don't have a suggestion for how to do it right, I need to read it carefully again...)
There was a problem hiding this comment.
oh, right, I forget to async it. I'll add it and see how would this go
|
Closing, now Michael picks this up in his PR #4218 |
It seems we have not been using lsp progress helper api any where.
Refactor
delayedProgressReportingto use lsp progress api.It is an easy target, we can see see how it works out here.
cc @michaelpj