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

vscode-ext: fix flickering view rendering after server restart #27

Merged
merged 2 commits into from
Jun 4, 2024

Conversation

umutdural
Copy link
Collaborator

This PR fixes the flickering bug which occurs when the server is restarted multiple times.

  • Cause: The start method of the client calls the createClient method, which registered the onDidSaveTextDocument callback. Hence every restart registers a copy of the same callback. Therefore if the server is restarted $n$ times, the same onDidSaveTextDocument callback is being called $n$ times when the document is saved. this makes the server verify the same document $n$ times consecutively which ultimately causes the flickering of the view elements.

  • Fix: The onDidSaveTextDocument callback register is moved to the CaesarClients constructor (where also command callbacks are registered) instead of the createClient method

Copy link
Collaborator

@Philipp15b Philipp15b left a comment

Choose a reason for hiding this comment

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

Very nice catch, I'd never have found this!! A small change is requested.

@@ -113,6 +113,17 @@ export class CaesarClient {
this.needsRestart = true;
}
}));

// listen to onDidSaveTextDocument events
const autoVerify: string = CaesarConfig.get(ConfigurationConstants.automaticVerification);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will only check the automatic verification setting once when the extension is loaded, as opposed to every time the server is reloaded (which happens when the config changes). I have two options: a) just save the Disposable from onDidSaveTextDocument and dispose of the callback when the server is stopped, or b) do the check inside the callback (but then check for languageId first, only then do the config check).

@Philipp15b Philipp15b merged commit f617f01 into moves-rwth:main Jun 4, 2024
4 checks passed
@Philipp15b
Copy link
Collaborator

Thanks!

Philipp15b added a commit that referenced this pull request Jun 5, 2024
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