Skip to content
This repository has been archived by the owner on Jul 27, 2024. It is now read-only.

Errors related to missing or unused files do not go away after correcting them #136

Closed
macpham opened this issue Jan 28, 2021 · 5 comments · Fixed by #142
Closed

Errors related to missing or unused files do not go away after correcting them #136

macpham opened this issue Jan 28, 2021 · 5 comments · Fixed by #142
Assignees
Labels
bug Something isn't working

Comments

@macpham
Copy link

macpham commented Jan 28, 2021

I was missing a file, so then I add it or I deleted an unused file, it is still showing up in the problems list though in the output tab for theme-check it shows the count going down for the errors as I delete the unused files:

Found 57 templates, and 32 offenses
Handler does not respond to on_text_document_did_close
Checking /Users/mac/src/github.com/Shopify/blah
Found 56 templates, and 31 offenses
Handler does not respond to on_text_document_did_close
Checking /Users/mac/src/github.com/Shopify/blah
Found 55 templates, and 30 offenses
Handler does not respond to on_text_document_did_close
Checking /Users/mac/src/github.com/Shopify/blah
Found 54 templates, and 29 offenses
Handler does not respond to on_text_document_did_close
Checking /Users/mac/src/github.com/Shopify/blah
Found 54 templates, and 29 offenses
...

image

Slack context from @charlespwd :

CP Clermont:bulb: 3 days ago
Similar bug as the other one. Would need to send empty diagnostics.

CP Clermont:bulb: 3 days ago
We should open a GitHub issue for that

CP Clermont:bulb: 3 days ago
(in theme-check, not theme-check-vscode)

CP Clermont:bulb: 3 days ago
That's an LSP bug

@macpham macpham added the bug Something isn't working label Jan 28, 2021
@charlespwd
Copy link
Contributor

charlespwd commented Jan 28, 2021

More context on what's happening:

When a file changes it is the server's responsibility to re-compute diagnostics and push them to the client. If the computed set is empty it has to push the empty array to clear former diagnostics. Newly pushed diagnostics always replace previously pushed diagnostics. There is no merging that happens on the client side.

https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_publishDiagnostics

Note: in the Language Server Protocol context, "diagnostics" == "problems" in VS Code.

Since we're not clearing the error, I suspect we're not sending a publishDiagnostics with an empty array for those newly added or deleted files.

Either that, or we're not listening in on the event that the file was added/deleted and we're not recomputing the diagnostics.

@macournoyer
Copy link
Contributor

I think the issue is that we're not sending empty diagnostics for files that have been removed. We only send it for files still present in the repo: https://github.com/Shopify/theme-check/blob/master/lib/theme_check/language_server/handler.rb#L63.

@macpham
Copy link
Author

macpham commented Jan 28, 2021

That makes sense to explain one direction of errors, when you have an unused file and you remove it. What about what you're missing a file, and then you add it? A new file gets added that technically doesn't have any errors within itself, but solves the error that it was missing.

@macournoyer
Copy link
Contributor

What about what you're missing a file, and then you add it? A new file gets added that technically doesn't have any errors within itself, but solves the error that it was missing.

I was not able to reproduce this. It works for me:

  1. Add {% render 'new' %} to theme.liquid
  2. You can an error about missing snippets/new.liquid
  3. Add snippets/new.liquid, save.
  4. Error is gone, as expected.

@macpham
Copy link
Author

macpham commented Feb 3, 2021

Woohoo! Thank you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants