Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #72. Reset service when tsconfig changes #807

Merged
merged 1 commit into from
Dec 5, 2023
Merged

Fix #72. Reset service when tsconfig changes #807

merged 1 commit into from
Dec 5, 2023

Conversation

STRd6
Copy link
Contributor

@STRd6 STRd6 commented Nov 15, 2023

Still need to actually test but the approach seems correct.

Copy link
Collaborator

@edemaine edemaine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this out and didn't see any reloading behavior, when editing or when saving tsconfig.json. I didn't even see an onDidChangeContent message. Perhaps the plugin isn't running on the tsconfig.json file, so doesn't see the change. (Currently only registered for *.civet and *.ts?)

@STRd6
Copy link
Contributor Author

STRd6 commented Nov 15, 2023

Good point, probably need to register .json for the LSP for this method to work.

@STRd6 STRd6 marked this pull request as ready for review November 18, 2023 02:29
@edemaine
Copy link
Collaborator

edemaine commented Nov 23, 2023

Sorry, I missed that this was ready to review for a while.

I just tested and I think the behavior is still not quite right. Here's what I see:

  • As I edit tsconfig.json, Civet plugin furiously tries to update diagnostics for .civet files.
  • But it's still loading tsconfig.json from disk (I think), so it's getting the old version.
  • When I save tsconfig.json, it doesn't trigger Civet to update diagnostics, so things are still out-of-date.
  • Only after making an (unsaved) change again to tsconfig.json does Civet actually update, finally with the new tsconfig.json.

I don't think we really need to update when tsconfig.json is edited, only when it's saved. (As it's edited, it probably has errors.) I wonder if, instead of including jsonc files in the plugin, we should use something like createFileSystemWatcher. It could watch for Civet config files too.

@STRd6
Copy link
Contributor Author

STRd6 commented Dec 2, 2023

Thanks for the tip! createFileSystemWatcher seems like a much better approach. I've tested it quickly and it looks like it is working.

@STRd6 STRd6 requested a review from edemaine December 3, 2023 18:37
@STRd6 STRd6 merged commit f818c1e into main Dec 5, 2023
2 checks passed
@STRd6 STRd6 deleted the 72 branch December 5, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants