lsp icon indicating copy to clipboard operation
lsp copied to clipboard

Provide explicit diagnostic store flush function

Open PanAeon opened this issue 6 years ago • 3 comments

There's a logic in the Diagnostics.hs that will append diagnostics to the existing ones if the version is the same, and create new diagnostic store (discard old one) if versions are different. At least if I understood that correctly.

This causing me a problem because LSP doesn't return version on SaveDocument notification, only on update document. As a consequence, when some file contains error and I try to return empty diagnostics on save, old error message is preserved. What is a bit surprising, when different error is produced, the error message got updated.

I suggest to either provide explicit public function that will flush the store for a specific URI when no error message is available, or evern better, to change behaviour of updateDiagnostics, so when called with version Nothing then the store should be flushed regardless of wether previous version is Nothing.

I can help with this issue if you give me a go-ahead.

PanAeon avatar Feb 27 '19 11:02 PanAeon

Can you make a PR for this, so we can discuss actual code?

alanz avatar Feb 28 '19 19:02 alanz

@alanz I figured out my confusion. When I sent empty diagnostics back to the client I was essentially doing publishDiagnosticsFunc ... (partitionBySource []). So I was sending empty map for the same version of the source file (Nothing) because I didn't have any other versions. As result my error messages in VSCode persisted, although I tried to clear them up. Once I started sending something like Map.singleton (Just defaultDiagnosticSource) (Data.SortedList.toSortedList []) instead of an empty map my stuck diagnostic messages gone.

You can either close this ticket now, or if you think this is a good idea, I'd rather change the signature of partitionBySource as such:

partitionBySource :: [DiagnosticSource] -> [Diagnostic] -> DiagnosticsBySource
partitionBySource srcs diags = Map.union diags' defaults
  where
      emptyDiag src = Map.singleton (Just src) (SL.toSortedList [])
      defaults = emptyDiag <$> srcs
      diags' = Map.fromListWith mappend $ map (\d -> (J._source d, (SL.singleton d))) diags

so the person who uses it won't fall in the same trap as I did. What do you think?

PanAeon avatar Mar 01 '19 11:03 PanAeon

I just ran into this exact issue. For now I defined this function based on the above:

partitionBySource ::
  [LSP.Types.DiagnosticSource] ->
  [LSP.Types.Diagnostic] ->
  LSP.Diagnostics.DiagnosticsBySource
partitionBySource sources diagnostics = Map.unions (diagnostics' : defaults)
  where
  emptyDiagnostic ::
    LSP.Types.DiagnosticSource ->
    LSP.Diagnostics.DiagnosticsBySource
  emptyDiagnostic source =
    Map.singleton (Just source) (SortedList.toSortedList [])

  defaults :: [LSP.Diagnostics.DiagnosticsBySource]
  defaults = emptyDiagnostic <$> sources

  diagnostics' :: LSP.Diagnostics.DiagnosticsBySource
  diagnostics' = LSP.Diagnostics.partitionBySource diagnostics

paulyoung avatar Aug 27 '20 00:08 paulyoung