GhostCells: Fix removal of ghost rendering and prevent stale inserts

[jlewi]

This PR fixes two issues with ghost cells.

Preventing stale inserts

The server can return a completion after the editor has changed invalidating the completion. For example, prior to this PR we observe the following

• A user is editing cell N
• So server is generating suggestions for cell N+1
• User switches the focus to cell N-2
• Completion for N+1 arrives
• Cell N+1 gets inserted

This is not what should happen. As soon as the focus switches away from cell N any completions based on cell N should be invalidated and ignored.

To deal with that we introduce a context id. This is just a ULID that gets changed whenever something changes warranting a new stream of completions. The context id changes whenever the user changes the focus in the notebook. The context id gets set on the completions sent to the foyle and is returned in the completion responses.
The frontend can then check whether the context id in the response matches the current context; if it doesn't then the completion is ignored.

Ghost Decorations weren't being removed

The ghost decorations weren't being removed when a cell was being accepted. The problem is that to remove the textdecorations we need to pass a reference to the decoration to be removed to the setDecoration function. I however was using the function getGhostDecoration to create a new instance of the decoration so it wasn't getting removed.

[jlewi]

Need to merge #1536 first and then rebase.

[sourishkrout]

Need to merge #1536 first and then rebase.

@jlewi merged!

[jlewi]

@sourishkrout This is ready for review. The caveat is the unittest is failing because of a problem with mocking out the createTextEditorDecorationType function.
Do you see an easy way to fix the mock?

An alternative solution is I can refactor the code to define it as an instance variable on the GhostCell Generator class.

[sourishkrout]

👍 will take a look.

Do you see an easy way to fix the mock?

I'll figure out a fix. I'm sure there's a way without requiring a refactor.

[jlewi]

Thank you!

[sourishkrout]

@sourishkrout This is ready for review. The caveat is the unittest is failing because of a problem with mocking out the createTextEditorDecorationType function.
Do you see an easy way to fix the mock?

This should fix the mock, @jlewi. I haven't reviewed the rest of the PR yet. Will do that tmrw.

diff --git a/__mocks__/vscode.ts b/__mocks__/vscode.ts
index ce828ce9..32cd0025 100644
--- a/__mocks__/vscode.ts
+++ b/__mocks__/vscode.ts
@@ -93,6 +93,7 @@ export const window = {
   showInformationMessage: vi.fn(),
   showErrorMessage: vi.fn(),
   createTerminal: vi.fn().mockReturnValue(terminal),
+  createTextEditorDecorationType: vi.fn(),
   showNotebookDocument: vi.fn(),
   showTextDocument: vi.fn(),
   onDidChangeActiveNotebookEditor: vi.fn().mockReturnValue({ dispose: vi.fn() }),

[sonarcloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
75.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[jlewi]

That did it thank you very much.

[sourishkrout]

✅ LGTM and works locally.