Skip to content

Commit e4d0e64

Browse files
rchiodorchiodo
authored
Fix some intellisense issues with notebooks (#8940)
* Fix $0 to work again Fix monaco editor to not have its own hover mouse leave * Add news entry * Review feedback * Better way to look for snippet strings. * Attempt to fix flakey test
1 parent 723a238 commit e4d0e64

File tree

5 files changed

+112
-137
lines changed

5 files changed

+112
-137
lines changed

‎news/2 Fixes/8843.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix scrolling inside of intellisense hover windows for notebooks.

‎src/client/datascience/interactive-common/intellisense/conversion.ts

Lines changed: 92 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -9,118 +9,93 @@ import * as vscodeLanguageClient from 'vscode-languageclient';
99

1010
// See the comment on convertCompletionItemKind below
1111
// Here's the monaco enum:
12-
// Method = 0,
13-
// Function = 1,
14-
// Constructor = 2,
15-
// Field = 3,
16-
// Variable = 4,
17-
// Class = 5,
18-
// Struct = 6,
19-
// Interface = 7,
20-
// Module = 8,
21-
// Property = 9,
22-
// Event = 10,
23-
// Operator = 11,
24-
// Unit = 12,
25-
// Value = 13,
26-
// Constant = 14,
27-
// Enum = 15,
28-
// EnumMember = 16,
29-
// Keyword = 17,
30-
// Text = 18,
31-
// Color = 19,
32-
// File = 20,
33-
// Reference = 21,
34-
// Customcolor = 22,
35-
// Folder = 23,
36-
// TypeParameter = 24,
37-
// Snippet = 25
12+
enum monacoCompletionItemKind {
13+
Method = 0,
14+
Function = 1,
15+
Constructor = 2,
16+
Field = 3,
17+
Variable = 4,
18+
Class = 5,
19+
Struct = 6,
20+
Interface = 7,
21+
Module = 8,
22+
Property = 9,
23+
Event = 10,
24+
Operator = 11,
25+
Unit = 12,
26+
Value = 13,
27+
Constant = 14,
28+
Enum = 15,
29+
EnumMember = 16,
30+
Keyword = 17,
31+
Text = 18,
32+
Color = 19,
33+
File = 20,
34+
Reference = 21,
35+
Customcolor = 22,
36+
Folder = 23,
37+
TypeParameter = 24,
38+
Snippet = 25
39+
}
3840
//
39-
// Here's the vscode enum
40-
// const Text: 1;
41-
// const Method: 2;
42-
// const Function: 3;
43-
// const Constructor: 4;
44-
// const Field: 5;
45-
// const Variable: 6;
46-
// const Class: 7;
47-
// const Interface: 8;
48-
// const Module: 9;
49-
// const Property: 10;
50-
// const Unit: 11;
51-
// const Value: 12;
52-
// const Enum: 13;
53-
// const Keyword: 14;
54-
// const Snippet: 15;
55-
// const Color: 16;
56-
// const File: 17;
57-
// const Reference: 18;
58-
// const Folder: 19;
59-
// const EnumMember: 20;
60-
// const Constant: 21;
61-
// const Struct: 22;
62-
// const Event: 23;
63-
// const Operator: 24;
64-
// const TypeParameter: 25;
6541

6642
// Left side is the vscode value.
6743
const mapCompletionItemKind: Map<number, number> = new Map<number, number>([
68-
[0, 9], // No value for zero in vscode
69-
[1, 18], // Text
70-
[2, 0], // Method
71-
[3, 1], // Function
72-
[4, 2], // Constructor
73-
[5, 3], // Field
74-
[6, 4], // Variable
75-
[7, 5], // Class
76-
[8, 7], // Interface
77-
[9, 8], // Module
78-
[10, 9], // Property
79-
[11, 12], // Unit
80-
[12, 13], // Value
81-
[13, 15], // Enum
82-
[14, 17], // Keyword
83-
[15, 25], // Snippet
84-
[16, 19], // Color
85-
[17, 20], // File
86-
[18, 21], // Reference
87-
[19, 23], // Folder
88-
[20, 16], // EnumMember
89-
[21, 14], // Constant
90-
[22, 6], // Struct
91-
[23, 10], // Event
92-
[24, 11], // Operator
93-
[25, 24] // TypeParameter
44+
[vscode.CompletionItemKind.Text, monacoCompletionItemKind.Text], // Text
45+
[vscode.CompletionItemKind.Method, monacoCompletionItemKind.Method], // Method
46+
[vscode.CompletionItemKind.Function, monacoCompletionItemKind.Function], // Function
47+
[vscode.CompletionItemKind.Constructor, monacoCompletionItemKind.Constructor], // Constructor
48+
[vscode.CompletionItemKind.Field, monacoCompletionItemKind.Field], // Field
49+
[vscode.CompletionItemKind.Variable, monacoCompletionItemKind.Variable], // Variable
50+
[vscode.CompletionItemKind.Class, monacoCompletionItemKind.Class], // Class
51+
[vscode.CompletionItemKind.Interface, monacoCompletionItemKind.Interface], // Interface
52+
[vscode.CompletionItemKind.Module, monacoCompletionItemKind.Module], // Module
53+
[vscode.CompletionItemKind.Property, monacoCompletionItemKind.Property], // Property
54+
[vscode.CompletionItemKind.Unit, monacoCompletionItemKind.Unit], // Unit
55+
[vscode.CompletionItemKind.Value, monacoCompletionItemKind.Value], // Value
56+
[vscode.CompletionItemKind.Enum, monacoCompletionItemKind.Enum], // Enum
57+
[vscode.CompletionItemKind.Keyword, monacoCompletionItemKind.Keyword], // Keyword
58+
[vscode.CompletionItemKind.Snippet, monacoCompletionItemKind.Snippet], // Snippet
59+
[vscode.CompletionItemKind.Color, monacoCompletionItemKind.Color], // Color
60+
[vscode.CompletionItemKind.File, monacoCompletionItemKind.File], // File
61+
[vscode.CompletionItemKind.Reference, monacoCompletionItemKind.Reference], // Reference
62+
[vscode.CompletionItemKind.Folder, monacoCompletionItemKind.Folder], // Folder
63+
[vscode.CompletionItemKind.EnumMember, monacoCompletionItemKind.EnumMember], // EnumMember
64+
[vscode.CompletionItemKind.Constant, monacoCompletionItemKind.Constant], // Constant
65+
[vscode.CompletionItemKind.Struct, monacoCompletionItemKind.Struct], // Struct
66+
[vscode.CompletionItemKind.Event, monacoCompletionItemKind.Event], // Event
67+
[vscode.CompletionItemKind.Operator, monacoCompletionItemKind.Operator], // Operator
68+
[vscode.CompletionItemKind.TypeParameter, monacoCompletionItemKind.TypeParameter] // TypeParameter
9469
]);
9570

9671
const mapJupyterKind: Map<string, number> = new Map<string, number>([
97-
['method', 0],
98-
['function', 1],
99-
['constructor', 2],
100-
['field', 3],
101-
['variable', 4],
102-
['class', 5],
103-
['struct', 6],
104-
['interface', 7],
105-
['module', 8],
106-
['property', 9],
107-
['event', 10],
108-
['operator', 11],
109-
['unit', 12],
110-
['value', 13],
111-
['constant', 14],
112-
['enum', 15],
113-
['enumMember', 16],
114-
['keyword', 17],
115-
['text', 18],
116-
['color', 19],
117-
['file', 20],
118-
['reference', 21],
119-
['customcolor', 22],
120-
['folder', 23],
121-
['typeParameter', 24],
122-
['snippet', 25],
123-
['<unknown>', 25]
72+
['method', monacoCompletionItemKind.Method],
73+
['function', monacoCompletionItemKind.Function],
74+
['constructor', monacoCompletionItemKind.Constructor],
75+
['field', monacoCompletionItemKind.Field],
76+
['variable', monacoCompletionItemKind.Variable],
77+
['class', monacoCompletionItemKind.Class],
78+
['struct', monacoCompletionItemKind.Struct],
79+
['interface', monacoCompletionItemKind.Interface],
80+
['module', monacoCompletionItemKind.Module],
81+
['property', monacoCompletionItemKind.Property],
82+
['event', monacoCompletionItemKind.Event],
83+
['operator', monacoCompletionItemKind.Operator],
84+
['unit', monacoCompletionItemKind.Unit],
85+
['value', monacoCompletionItemKind.Value],
86+
['constant', monacoCompletionItemKind.Constant],
87+
['enum', monacoCompletionItemKind.Enum],
88+
['enumMember', monacoCompletionItemKind.EnumMember],
89+
['keyword', monacoCompletionItemKind.Keyword],
90+
['text', monacoCompletionItemKind.Text],
91+
['color', monacoCompletionItemKind.Color],
92+
['file', monacoCompletionItemKind.File],
93+
['reference', monacoCompletionItemKind.Reference],
94+
['customcolor', monacoCompletionItemKind.Customcolor],
95+
['folder', monacoCompletionItemKind.Folder],
96+
['typeParameter', monacoCompletionItemKind.TypeParameter],
97+
['snippet', monacoCompletionItemKind.Snippet],
98+
['<unknown>', monacoCompletionItemKind.Snippet]
12499
]);
125100

126101
function convertToMonacoRange(range: vscodeLanguageClient.Range | undefined): monacoEditor.IRange | undefined {
@@ -141,13 +116,15 @@ function convertToMonacoRange(range: vscodeLanguageClient.Range | undefined): mo
141116
// import { EDITOR_DEFAULTS } from './common/config/editorOptions.js';
142117
// Instead just use a map
143118
function convertToMonacoCompletionItemKind(kind?: number): number {
144-
const value = kind ? mapCompletionItemKind.get(kind) : 9; // Property is 9
119+
const value = kind ? mapCompletionItemKind.get(kind) : monacoCompletionItemKind.Property; // Property is 9
145120
if (value) {
146121
return value;
147122
}
148-
return 9; // Property
123+
return monacoCompletionItemKind.Property;
149124
}
150125

126+
const SnippetEscape = 4;
127+
151128
function convertToMonacoCompletionItem(item: vscodeLanguageClient.CompletionItem, requiresKindConversion: boolean): monacoEditor.languages.CompletionItem {
152129
// They should be pretty much identical? Except for ranges.
153130
// tslint:disable-next-line: no-object-literal-type-assertion no-any
@@ -161,6 +138,14 @@ function convertToMonacoCompletionItem(item: vscodeLanguageClient.CompletionItem
161138
result.insertText = result.label;
162139
}
163140

141+
// tslint:disable-next-line: no-any
142+
const snippet = (result.insertText as any) as vscode.SnippetString;
143+
if (snippet.value) {
144+
result.insertTextRules = SnippetEscape;
145+
// Monaco can't handle the snippetText value, so rewrite it.
146+
result.insertText = snippet.value;
147+
}
148+
164149
// Make sure we don't have _documentPosition. It holds onto a huge tree of information
165150
// tslint:disable-next-line: no-any
166151
const resultAny = result as any;

‎src/datascience-ui/interactive-common/editor.tsx

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -188,25 +188,6 @@ export class Editor extends React.Component<IEditorProps, IEditorState> {
188188
private modelChanged = (e: monacoEditor.editor.IModelContentChangedEvent) => {
189189
if (this.state.model) {
190190
this.props.onChange(e.changes, this.state.model);
191-
const value = this.state.model.getValue();
192-
const secondHalf = value.substr(e.changes[0].rangeOffset);
193-
194-
// The second condition makes sure this block is only entered once after
195-
// adding the intellisense suggestion.
196-
if (secondHalf.includes('($0)') && e.changes[0].rangeOffset > 0) {
197-
// We leave the first half of value intact and only replace the first occurance
198-
// of '($0)' on the second half
199-
this.state.model.setValue(value.substr(0, e.changes[0].rangeOffset) + secondHalf.replace('($0)', '()'));
200-
201-
// The monaco editor is leaving the cursor at the end of the intellisense,
202-
// so we move it one position to the left to leave it inside the parenthesis
203-
setTimeout(() => {
204-
if (this.state.editor) {
205-
const pos = this.state.editor.getPosition();
206-
this.state.editor.setPosition(pos!.delta(0, -1));
207-
}
208-
}, 0);
209-
}
210191
}
211192
}
212193

‎src/datascience-ui/react-common/monacoEditor.tsx

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ const throttle = require('lodash/throttle') as typeof import('lodash/throttle');
1919
import './monacoEditor.css';
2020

2121
const LINE_HEIGHT = 18;
22+
23+
const HOVER_DISPOSABLE_EVENT_COUNT = 8;
24+
const HOVER_DISPOSABLE_LEAVE_INDEX = 5;
25+
2226
enum WidgetCSSSelector {
2327
/**
2428
* CSS Selector for the parameters widget displayed by Monaco.
@@ -704,17 +708,21 @@ export class MonacoEditor extends React.Component<IMonacoEditorProps, IMonacoEdi
704708
// mean the mouse has left the editor.
705709
// tslint:disable-next-line: no-any
706710
const hover = editor.getContribution('editor.contrib.hover') as any;
707-
if (hover._toUnhook && hover._toUnhook.length === 8 && hover.contentWidget) {
711+
if (hover._toUnhook && hover._toUnhook._toDispose && hover._toUnhook._toDispose && hover.contentWidget) {
708712
// This should mean our 5th element is the event handler for mouse leave. Remove it.
709-
const array = hover._toUnhook as IDisposable[];
710-
array[5].dispose();
711-
array.splice(5, 1);
712-
713-
// Instead listen to mouse leave for our hover widget
714-
const hoverWidget = this.widgetParent.getElementsByClassName('monaco-editor-hover')[0] as HTMLElement;
715-
if (hoverWidget) {
716-
hoverWidget.addEventListener('mouseenter', this.onHoverEnter);
717-
hoverWidget.addEventListener('mouseleave', this.onHoverLeave);
713+
const set = hover._toUnhook._toDispose as Set<IDisposable>;
714+
if (set.size === HOVER_DISPOSABLE_EVENT_COUNT) {
715+
// This is horribly flakey, Is set always guaranteed to put stuff in the same order?
716+
const array = Array.from(set);
717+
array[HOVER_DISPOSABLE_LEAVE_INDEX].dispose();
718+
set.delete(array[HOVER_DISPOSABLE_LEAVE_INDEX]);
719+
720+
// Instead listen to mouse leave for our hover widget
721+
const hoverWidget = this.widgetParent.getElementsByClassName('monaco-editor-hover')[0] as HTMLElement;
722+
if (hoverWidget) {
723+
hoverWidget.addEventListener('mouseenter', this.onHoverEnter);
724+
hoverWidget.addEventListener('mouseleave', this.onHoverLeave);
725+
}
718726
}
719727
}
720728
}

‎src/test/datascience/nativeEditor.functional.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1197,7 +1197,7 @@ for _ in range(50):
11971197
test('File saved with same format', async () => {
11981198
// Configure notebook to save automatically ever 1s.
11991199
when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('afterDelay');
1200-
when(ioc.mockedWorkspaceConfig.get<number>('autoSaveDelay', anything())).thenReturn(1_000);
1200+
when(ioc.mockedWorkspaceConfig.get<number>('autoSaveDelay', anything())).thenReturn(2_000);
12011201
ioc.forceSettingsChanged(ioc.getSettings().pythonPath);
12021202
const notebookFileContents = await fs.readFile(notebookFile.filePath, 'utf8');
12031203

0 commit comments

Comments
 (0)