Skip to content

Commit

Permalink
Use the same debounced restart for all watchers (#2818)
Browse files Browse the repository at this point in the history
### Motivation

I noticed another issue with our restart watchers: they don't share the same debounced function. This matters because currently if two separate watchers are triggered in sequence, then we restart twice.

This is of course not what we want. We want to restart a single time within the debounce time, no matter how many watchers have been triggered.

### Implementation

The core of the issue is that if we have multiple debounced functions, they each have their own timeout object with a different ID. That leads to this scenario:

1. Rebase watcher fires its debounced restart
2. After rebase is complete, the lockfile was modified so that watcher is fired too
3. Because the debounced restart function of the lockfile is not the same as the rebase watcher, it won't clear the timeout of the rebase invocation
4. This triggers two consecutive restarts for no reason

The fix is to ensure that we only have one single debounced restart function that is used by all of the watchers. This guarantees that, no matter which watcher is fired, it will always clear the previous restart timeout and avoid doing it multiple times.
  • Loading branch information
vinistock authored Nov 4, 2024
1 parent aa6fdaf commit fbe2350
Showing 1 changed file with 12 additions and 20 deletions.
32 changes: 12 additions & 20 deletions vscode/src/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ export class Workspace implements WorkspaceInterface {
#inhibitRestart = false;
#error = false;

private readonly debouncedRestart = debounce(async (reason: string) => {
this.outputChannel.info(`Restarting the Ruby LSP because ${reason}`);
await this.restart();
}, 5000);

constructor(
context: vscode.ExtensionContext,
workspaceFolder: vscode.WorkspaceFolder,
Expand Down Expand Up @@ -376,7 +381,7 @@ export class Workspace implements WorkspaceInterface {

// Handler for only triggering restart if the contents of the file have been modified. If the file was just touched,
// but the contents are the same, we don't want to restart
const debouncedRestartWithHashCheck = debounce(async (uri: vscode.Uri) => {
const debouncedRestartWithHashCheck = async (uri: vscode.Uri) => {
const fileContents = await vscode.workspace.fs.readFile(uri);
const fsPath = uri.fsPath;

Expand All @@ -385,21 +390,14 @@ export class Workspace implements WorkspaceInterface {
const currentSha = hash.digest("hex");

if (this.restartDocumentShas.get(fsPath) !== currentSha) {
this.outputChannel.info(
`Restarting the Ruby LSP because ${pattern} changed`,
);

this.restartDocumentShas.set(fsPath, currentSha);
await this.restart();
await this.debouncedRestart(`${pattern} changed`);
}
}, 5000);
};

const debouncedRestart = debounce(async () => {
this.outputChannel.info(
`Restarting the Ruby LSP because ${pattern} changed`,
);
await this.restart();
}, 5000);
const debouncedRestart = async () => {
await this.debouncedRestart(`${pattern} changed`);
};

context.subscriptions.push(
watcher,
Expand All @@ -417,16 +415,10 @@ export class Workspace implements WorkspaceInterface {
const start = () => {
this.#inhibitRestart = true;
};
const debouncedRestart = debounce(async () => {
this.outputChannel.info(
`Restarting the Ruby LSP because ${glob} changed`,
);
await this.restart();
}, 5000);

const stop = async () => {
this.#inhibitRestart = false;
await debouncedRestart();
await this.debouncedRestart(`${glob} changed`);
};

this.context.subscriptions.push(
Expand Down

0 comments on commit fbe2350

Please sign in to comment.