First steps in autocomplete by rchiodo · Pull Request #5601 · microsoft/vscode-python

Rich Chiodo (rchiodo) · 2019-05-10T22:26:36Z

For #5342

Rewire everything to use monaco and provide suggestions and hover support. Just use the base language server for now (jedi or dotnetcore)

…te_investigate

Ian Huff (IanMatthewHuff) · 2019-05-10T23:06:56Z

    "@types/react-codemirror": "^1.0.2",

Can be removed #Resolved

Refers to: package.json:2227 in a4f81e6. [](commit_id = a4f81e6, deletion_comment = False)

Rich Chiodo (rchiodo) · 2019-05-10T23:07:07Z


 @injectable()
 export class LanguageServer implements ILanguageServer {
-    private readonly startupCompleted: Deferred<void>;


@DonJayamme if you could take a look at this part, that'd be great. I think making this all a singleton makes sense. But not entirely sure.

I think the LanguageServer was effectively a singleton before (as it's only created by the LanguageServerManager)

In reply to: 283065695 [](ancestors = 283065695)

Rich Chiodo (rchiodo) · 2019-05-10T23:12:34Z

    "@types/react-codemirror": "^1.0.2",

Will do.

In reply to: 491453735 [](ancestors = 491453735)

Refers to: package.json:2227 in a4f81e6. [](commit_id = a4f81e6, deletion_comment = False)

Ian Huff (IanMatthewHuff) · 2019-05-10T23:22:02Z

+        }
+
+        // Force the colors to the defaults
+        return null;


More of a question here. Why null for functions like this? As far as I'd seen the general though was always using undefined versus null unless specifically required by an external API. Want to make sure that we are both coding with the right concept of null versus undefined. #WontFix

VSCode and Typescript compiler have a general undefined over null rule that they mostly stick to.

In reply to: 283067624 [](ancestors = 283067624)

I think it's because some of the apis I was using were returning null colors.

In reply to: 283067624 [](ancestors = 283067624)

Yeah not sure vscode returns undefined. I think they actually return null for a lot of stuff. At least the completion stuff did. I think 'null' means no value returned, as opposed to not sure if the function was called or not.

In reply to: 283067802 [](ancestors = 283067802,283067624)

Yeah this says the same thing:
https://basarat.gitbooks.io/typescript/docs/javascript/null-undefined.html

Null and Undefined
Free youtube video on the subject
JavaScript (and by extension TypeScript) has two bottom types : null and undefined. They are intended to mean different things:
Something hasn't been initialized : undefined.
Something is currently unavailable: null.

In reply to: 283067961 [](ancestors = 283067961,283067802,283067624)

If the APIs return it that's enough of a reason for me no need to change. Sounds like (in the second link) some VSCode folks want only undefined and some want to use it like the link you posted.
https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines#null-and-undefined
microsoft/vscode#70020

In reply to: 283068106 [](ancestors = 283068106,283067961,283067802,283067624)

Here's another example:

node_modules\vscode-languageserver-protocol\lib\protocol.d.ts

returns '| null' on just about everything. There's no '| undefined' anywhere.

In reply to: 283068106 [](ancestors = 283068106,283067961,283067802,283067624)

Oh it's DOM apis that return null. Looks like that's the root of where it comes from.

In reply to: 283068385 [](ancestors = 283068385,283068106,283067961,283067802,283067624)

Ian Huff (IanMatthewHuff) · 2019-05-10T23:33:03Z

+    // https://github.com/Microsoft/vscode/blob/master/src/vs/editor/standalone/common/themes.ts#L13
+    private generateMonacoThemeObject(args: IApplyThemeArgs) : monacoEditor.editor.IStandaloneThemeData {
+        const result: monacoEditor.editor.IStandaloneThemeData = {
+            base: args.isDark ? 'vs-dark' : 'vs',


Isn't it vs-light for the light theme? #Resolved

Not in monaco. See this as an example:
https://microsoft.github.io/monaco-editor/playground.html#customizing-the-appearence-exposed-colors

In reply to: 283069004 [](ancestors = 283069004)

Or this:
https://microsoft.github.io/monaco-editor/api/modules/monaco.editor.html#builtintheme

In reply to: 283069188 [](ancestors = 283069188,283069004)

Ian Huff (IanMatthewHuff) · 2019-05-10T23:42:13Z

+        if (args.baseColors) {
+            const keys = Object.keys(args.baseColors);
+            keys.forEach(k => {
+                const color = args.baseColors && args.baseColors[k] ? args.baseColors[k] : '#000000';


args.baseColors [](start = 30, length = 15)

Already checked at if #WontFix

Linter doesn't think so ;(

In reply to: 283070043 [](ancestors = 283070043)

Ian Huff (IanMatthewHuff) · 2019-05-10T23:52:48Z

+
+        // This theme may include others. If so we need to combine the two together
+        const include = theme ? theme.include : undefined;
+        if (include && include !== null) {


null should already be falsy. #Resolved

Will fix.

In reply to: 283071173 [](ancestors = 283071173)

Ian Huff (IanMatthewHuff) · 2019-05-10T23:58:13Z

+        // Should be somewhere under currentPath/resources/app/extensions inside of a json file
+        let extensionsPath = path.join(currentPath, 'resources', 'app', 'extensions');
+        if (!(await fs.pathExists(extensionsPath))) {
+            // Might be on mac or linux. try a different path


Don't we have an IsWindows that we use other places? #WontFix

Yes, we do. But it still needs to check the first path I think.

In reply to: 283071695 [](ancestors = 283071695)

I guess I'd feel more comfortable checking both.

In reply to: 283071819 [](ancestors = 283071819,283071695)

Fine by me.

In reply to: 283072200 [](ancestors = 283072200,283071819,283071695)

Ian Huff (IanMatthewHuff) · 2019-05-14T17:43:53Z

+                incomplete: list.isIncomplete
+            };
+        } else {
+            const array = result as vscodeLanguageClient.CompletionItem[];


If the result is a vscode.CompletionItem[] or vscode.CompletionList don't they both get cast as the array here? Or do they have the same property isIncomplete for the completion item and the completion list as the vscodeLanguageClient items? If so a comment would be nice to specify that. #Resolved

Yes vscode.CompletionList and vcodeLanguageClient.CompletionList both have the isIncomplete property. This else should only come through here if the result is one of the two array types.

I'll add a comment to that effect.

In reply to: 283921359 [](ancestors = 283921359)

Actually this is a good catch since one of those types isIncomplete is an optional parameter. It should be checking for 'items' instead.

In reply to: 283922093 [](ancestors = 283922093,283921359)

Rich Chiodo (rchiodo) · 2019-05-14T17:46:21Z

"information_for_contributors": [

This is the history for this file in vscode:
https://github.com/microsoft/vscode/commits/master/extensions/python/syntaxes/MagicPython.tmLanguage.json #Resolved

Refers to: resources/MagicPython.tmLanguage.json:2 in cfefcf0. [](commit_id = cfefcf0, deletion_comment = False)

Rich Chiodo (rchiodo) · 2019-05-14T17:54:00Z

"information_for_contributors": [

Last actual change was in aug 2018

In reply to: 492339165 [](ancestors = 492339165)

Refers to: resources/MagicPython.tmLanguage.json:2 in cfefcf0. [](commit_id = cfefcf0, deletion_comment = False)

Ian Huff (IanMatthewHuff) · 2019-05-14T18:12:38Z

+    private submitContent = () => {
+        let content = this.getContents();
+        if (content) {
+            // Remove empty lines off the end


Does it make sense to trim leading new lines as well? #WontFix

Maybe? I thought after was less obvious it was there so the user might not have intended it. On the front they did. But not sure why they'd want them. They just make the output cell bigger. Maybe somebody might do that on purpose though? Although I guess you could still achieve the same thing with a comment and bunch of space.

In reply to: 283932962 [](ancestors = 283932962)

Ian Huff (IanMatthewHuff) · 2019-05-14T18:21:24Z

+        {
+            comments: {
+                lineComment: '#',
+                blockComment: ['\'\'\'', '\'\'\'']


doc comments are ''' or """ #Resolved

These actually didn't seem to be used. I'll change it and see what happens. I think the tokenizer takes precedence.

In reply to: 283936732 [](ancestors = 283936732)

Ian Huff (IanMatthewHuff)

Rich Chiodo (rchiodo) added 30 commits April 18, 2019 10:08

Testing codemirror's autocomplete

4f44f84

Remember monacoEditor pieces

20f721d

New idea for using the language client

88ca7e1

Farther along the completion provider idea.

192db0d

New idea mapped together

00cb505

Merge remote-tracking branch 'origin/master' into rchiodo/auto_comple…

a98a817

…te_investigate

Closer. no more errors

f12e3f8

Merge remote-tracking branch 'origin/master' into rchiodo/auto_comple…

d49a60a

…te_investigate

Working completion

16df587

Sorting

fa93010

Reverse sort kind

0f738ae

Merge remote-tracking branch 'origin/master' into rchiodo/auto_comple…

00d0f3d

…te_investigate

Getting monaco to show context menu and completion

d8bacb0

Fix margin width

dc6cbde

Add comment about language service

e092f96

Start of completion provider

16a2913

Update to use monaco types instead

e23e7cc

New IHistoryListener idea

f2f6d9b

Fix document position

939dd5e

Theme support

54b9f6b

Colorization mostly working

53516a0

Merge remote-tracking branch 'origin/master' into rchiodo/auto_comple…

612e632

…te_investigate

Move editor to standalone panel at the bottom

c48c6fc

Fix startup of the language server

b31a54e

Merge from master

7003077

Input history working again

303d756

Fix problem with convertToOffset

df3fd6a

Write own react monaco editor to handle update problems

d75f48c

Add unit test for the completion provider

4a49ab6

Fix editing for intellisense

0d008f7

Rich Chiodo (rchiodo) self-assigned this May 10, 2019

Add news entry

a4f81e6

Rich Chiodo (rchiodo) commented May 10, 2019

View reviewed changes

Remove react-codemirror

79de88c

Ian Huff (IanMatthewHuff) reviewed May 10, 2019

View reviewed changes

Fix hygiene

01abc78

Ian Huff (IanMatthewHuff) reviewed May 10, 2019

View reviewed changes

Add variable to check remote

396dd9e

Ian Huff (IanMatthewHuff) reviewed May 10, 2019

View reviewed changes

Fix default rules to support comment/string beginnings

c1d28d1

Ian Huff (IanMatthewHuff) reviewed May 10, 2019

View reviewed changes

Rich Chiodo (rchiodo) added 5 commits May 10, 2019 16:59

Fix unnecessary isNull check

bd9c49b

Make sure to cleanup after waiting for the suggestion to show up

cfefcf0

Fix out of memory error.

c808487

Separate out the babel steps as not required for single workspace

a95001b

Fix color test

d4e84d6

Ian Huff (IanMatthewHuff) reviewed May 14, 2019

View reviewed changes

Review feedback

dc840df

Ian Huff (IanMatthewHuff) reviewed May 14, 2019

View reviewed changes

Update incorrect block comment

f246e57

Ian Huff (IanMatthewHuff) approved these changes May 14, 2019

View reviewed changes

Rich Chiodo (rchiodo) merged commit 2312c3e into master May 14, 2019

Rich Chiodo (rchiodo) deleted the rchiodo/auto_complete_investigate branch May 14, 2019 20:44

lock Bot locked as resolved and limited conversation to collaborators Jul 30, 2019

Uh oh!

Conversation

Rich Chiodo (rchiodo) commented May 10, 2019

Uh oh!

Ian Huff (IanMatthewHuff) commented May 10, 2019 • edited by rchiodo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rich Chiodo (rchiodo) commented May 10, 2019

Uh oh!

Ian Huff (IanMatthewHuff) May 10, 2019 • edited by rchiodo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ian Huff (IanMatthewHuff) May 10, 2019 • edited by rchiodo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ian Huff (IanMatthewHuff) May 10, 2019 • edited by rchiodo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ian Huff (IanMatthewHuff) May 10, 2019 • edited by rchiodo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ian Huff (IanMatthewHuff) May 10, 2019 • edited by rchiodo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ian Huff (IanMatthewHuff) May 14, 2019 • edited by rchiodo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rich Chiodo (rchiodo) commented May 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rich Chiodo (rchiodo) commented May 14, 2019

Uh oh!

Ian Huff (IanMatthewHuff) May 14, 2019 • edited by rchiodo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ian Huff (IanMatthewHuff) commented May 10, 2019 •

edited by rchiodo

Loading

Ian Huff (IanMatthewHuff) May 10, 2019 •

edited by rchiodo

Loading

Ian Huff (IanMatthewHuff) May 10, 2019 •

edited by rchiodo

Loading

Ian Huff (IanMatthewHuff) May 10, 2019 •

edited by rchiodo

Loading

Ian Huff (IanMatthewHuff) May 10, 2019 •

edited by rchiodo

Loading

Ian Huff (IanMatthewHuff) May 10, 2019 •

edited by rchiodo

Loading

Ian Huff (IanMatthewHuff) May 14, 2019 •

edited by rchiodo

Loading

Rich Chiodo (rchiodo) commented May 14, 2019 •

edited

Loading

Ian Huff (IanMatthewHuff) May 14, 2019 •

edited by rchiodo

Loading

Ian Huff (IanMatthewHuff) May 14, 2019 •

edited by rchiodo

Loading