Skip to content

Fetch tooltip details on-demand for auto-completions #368

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

Merged
merged 33 commits into from
Dec 11, 2017
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
7675901
Basic tokenizer
Dec 1, 2017
eb42669
Fixed property names
Dec 1, 2017
2756974
Tests, round I
Dec 1, 2017
c2c1ced
Tests, round II
Dec 2, 2017
a108c96
merge master
Dec 3, 2017
14864a5
tokenizer test
Dec 4, 2017
0ed51d6
Remove temorary change
Dec 4, 2017
51b544c
Fix merge issue
Dec 4, 2017
3cd11e6
Merge conflict
Dec 4, 2017
82e0ad1
Merge conflict
Dec 4, 2017
9295c1a
Completion test
Dec 4, 2017
06eb1a5
Fix last line
Dec 4, 2017
e9db8e0
Fix javascript math
Dec 4, 2017
d12ca03
Merge master
Dec 5, 2017
d8ab041
Make test await for results
Dec 5, 2017
db75cd0
Add license headers
Dec 5, 2017
9ab2c47
Rename definitions to types
Dec 5, 2017
d9c95d0
Round I
Dec 5, 2017
d587485
License headers
Dec 5, 2017
1da5e0a
Merge branch 'master' of https://github.com/Microsoft/vscode-python
Dec 5, 2017
3ac66b6
Merge branch 'master' into 152
Dec 5, 2017
94a5a7e
Separate completion and doc fetching
Dec 6, 2017
df5af0e
Test fixes
Dec 7, 2017
af5c648
Merge master
Dec 7, 2017
fdb0e57
Undo temp change
Dec 7, 2017
c67ac49
Merge branch 'master' into 152
Dec 7, 2017
3f27b3b
Merge branch 'master' of https://github.com/Microsoft/vscode-python i…
Dec 7, 2017
7163a1e
Merge branch '152' of https://github.com/MikhailArkhipov/vscode-pytho…
Dec 7, 2017
2c8b179
CR feedback
Dec 8, 2017
3689667
Restore signature, make info colorized
Dec 9, 2017
e193805
Provide details string
Dec 11, 2017
ed48871
Merge branch 'master' of https://github.com/Microsoft/vscode-python i…
Dec 11, 2017
92655ec
Fix tests
Dec 11, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 23 additions & 23 deletions pythonFiles/completion.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,16 +208,16 @@ def _serialize_completions(self, script, identifier=None, prefix=''):
_completion['snippet'] = '%s=$1$0' % name
_completion['text'] = name
_completion['displayText'] = name
if self.show_doc_strings:
try:
_completion['description'] = signature.docstring()
_completion['raw_docstring'] = signature.docstring(raw=True)
except Exception:
_completion['description'] = ''
_completion['raw_docstring'] = ''
else:
_completion['description'] = self._generate_signature(
signature)
# if self.show_doc_strings:
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to keep this (and the other commented-out code) around?

Copy link
Author

Choose a reason for hiding this comment

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

Just in case we need it (or for reference purposes - i.e. how to do the doc fetch). But I am fine removing it, up to you folks.

Copy link
Member

Choose a reason for hiding this comment

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

I say strip it; we have a VCS for a reason. 😉

Copy link
Author

Choose a reason for hiding this comment

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

OK

# try:
# _completion['description'] = signature.docstring()
# _completion['raw_docstring'] = signature.docstring(raw=True)
# except Exception:
# _completion['description'] = ''
# _completion['raw_docstring'] = ''
# else:
# _completion['description'] = self._generate_signature(
# signature)
_completions.append(_completion)

try:
Expand All @@ -227,22 +227,22 @@ def _serialize_completions(self, script, identifier=None, prefix=''):
except :
completions = []
for completion in completions:
if self.show_doc_strings:
try:
description = completion.docstring()
except Exception:
description = ''
else:
description = self._generate_signature(completion)
# if self.show_doc_strings:
# try:
# description = completion.docstring()
# except Exception:
# description = ''
# else:
# description = self._generate_signature(completion)

try:
rawDocstring = completion.docstring(raw=True)
# rawDocstring = completion.docstring(raw=True)
_completion = {
'text': completion.name,
'type': self._get_definition_type(completion),
'raw_type': completion.type,
'description': description,
'raw_docstring': rawDocstring,
# 'description': description,
# 'raw_docstring': rawDocstring,
'rightLabel': self._additional_info(completion)
}
except Exception:
Expand All @@ -252,9 +252,9 @@ def _serialize_completions(self, script, identifier=None, prefix=''):
if c['text'] == _completion['text']:
c['type'] = _completion['type']
c['raw_type'] = _completion['raw_type']
if len(c['description']) == 0 and len(c['raw_docstring']) == 0:
c['description'] = _completion['description']
c['raw_docstring'] = _completion['description']
# if len(c['description']) == 0 and len(c['raw_docstring']) == 0:
# c['description'] = _completion['description']
# c['raw_docstring'] = _completion['description']


if any([c['text'].split('=')[0] == _completion['text']
Expand Down
2 changes: 1 addition & 1 deletion src/client/common/configSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export interface ITerminalSettings {
launchArgs: string[];
}

function isTestExecution(): boolean {
export function isTestExecution(): boolean {
// tslint:disable-next-line:interface-name no-string-literal
return process.env['VSC_PYTHON_CI_TEST'] === '1';
}
Expand Down
7 changes: 4 additions & 3 deletions src/client/common/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export function isNotInstalledError(error: Error): boolean {
return errorObj.code === 'ENOENT' || errorObj.code === 127 || isModuleNoInstalledError;
}

// tslint:disable-next-line:interface-name
export interface Deferred<T> {
readonly promise: Promise<T>;
readonly resolved: boolean;
Expand All @@ -36,12 +37,12 @@ class DeferredImpl<T> implements Deferred<T> {
this._reject = rej;
});
}
resolve(value?: T | PromiseLike<T>) {
public resolve(value?: T | PromiseLike<T>) {
this._resolve.apply(this.scope ? this.scope : this, arguments);
this._resolved = true;
}
// tslint:disable-next-line:no-any
reject(reason?: any) {
public reject(reason?: any) {
this._reject.apply(this.scope ? this.scope : this, arguments);
this._rejected = true;
}
Expand Down Expand Up @@ -71,7 +72,7 @@ export function createTemporaryFile(extension: string, temporaryDirectory?: stri
}

return new Promise<{ filePath: string, cleanupCallback: Function }>((resolve, reject) => {
tmp.file(options, function _tempFileCreated(err, tmpFile, fd, cleanupCallback) {
tmp.file(options, (err, tmpFile, fd, cleanupCallback) => {
if (err) {
return reject(err);
}
Expand Down
79 changes: 19 additions & 60 deletions src/client/providers/completionProvider.ts
Original file line number Diff line number Diff line change
@@ -1,76 +1,35 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

Choose a reason for hiding this comment

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

Please remove the license header, they should be added for new files.

Copy link
Author

Choose a reason for hiding this comment

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

Only new files?

Choose a reason for hiding this comment

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

Yes, apparently that's what CELA have informed Brett.

'use strict';

import * as vscode from 'vscode';
import { Position, ProviderResult, SnippetString, Uri } from 'vscode';
import { PythonSettings } from '../common/configSettings';
import { Tokenizer } from '../language/tokenizer';
import { TokenType } from '../language/types';
import { isTestExecution } from '../common/configSettings';
import { JediFactory } from '../languageServices/jediProxyFactory';
import { captureTelemetry } from '../telemetry';
import { COMPLETION } from '../telemetry/constants';
import { extractSignatureAndDocumentation } from './jediHelpers';
import * as proxy from './jediProxy';
import { CompletionSource } from './completionSource';

export class PythonCompletionItemProvider implements vscode.CompletionItemProvider {
private completionSource: CompletionSource;

public constructor(private jediFactory: JediFactory) { }
private static parseData(data: proxy.ICompletionResult, resource: Uri): vscode.CompletionItem[] {
if (data && data.items.length > 0) {
return data.items.map(item => {
const sigAndDocs = extractSignatureAndDocumentation(item);
const completionItem = new vscode.CompletionItem(item.text);
completionItem.kind = item.type;
completionItem.documentation = sigAndDocs[1].length === 0 ? item.description : sigAndDocs[1];
completionItem.detail = sigAndDocs[0].split(/\r?\n/).join('');
if (PythonSettings.getInstance(resource).autoComplete.addBrackets === true &&
(item.kind === vscode.SymbolKind.Function || item.kind === vscode.SymbolKind.Method)) {
completionItem.insertText = new SnippetString(item.text).appendText('(').appendTabstop().appendText(')');
}

// ensure the built in memebers are at the bottom
completionItem.sortText = (completionItem.label.startsWith('__') ? 'z' : (completionItem.label.startsWith('_') ? 'y' : '__')) + completionItem.label;
return completionItem;
});
}
return [];
constructor(jediFactory: JediFactory) {
this.completionSource = new CompletionSource(jediFactory);
}

@captureTelemetry(COMPLETION)
public provideCompletionItems(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken): ProviderResult<vscode.CompletionItem[]> {
if (position.character <= 0) {
return Promise.resolve([]);
}
const filename = document.fileName;
const lineText = document.lineAt(position.line).text;
if (lineText.match(/^\s*\/\//)) {
return Promise.resolve([]);
public async provideCompletionItems(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken):
Promise<vscode.CompletionItem[]> {
const items = await this.completionSource.getVsCodeCompletionItems(document, position, token);
if (isTestExecution()) {
for (let i = 0; i < Math.min(3, items.length); i += 1) {
items[i] = await this.resolveCompletionItem(items[i], token);
}
}
// Suppress completion inside string and comments
if (this.isPositionInsideStringOrComment(document, position)) {
return Promise.resolve([]);
}
const type = proxy.CommandType.Completions;
const columnIndex = position.character;

const source = document.getText();
const cmd: proxy.ICommand<proxy.ICommandResult> = {
command: type,
fileName: filename,
columnIndex: columnIndex,
lineIndex: position.line,
source: source
};

return this.jediFactory.getJediProxyHandler<proxy.ICompletionResult>(document.uri).sendCommand(cmd, token).then(data => {
return PythonCompletionItemProvider.parseData(data, document.uri);
});
return items;
}

private isPositionInsideStringOrComment(document: vscode.TextDocument, position: vscode.Position): boolean {
const tokenizeTo = position.translate(1, 0);
const text = document.getText(new vscode.Range(new Position(0, 0), tokenizeTo));
const t = new Tokenizer();
const tokens = t.Tokenize(text);
const index = tokens.getItemContaining(document.offsetAt(position));
return index >= 0 && (tokens[index].TokenType === TokenType.String || tokens[index].TokenType === TokenType.Comment);
public async resolveCompletionItem(item: vscode.CompletionItem, token: vscode.CancellationToken): Promise<vscode.CompletionItem> {
item.documentation = await this.completionSource.getDocumentation(item, token);

Choose a reason for hiding this comment

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

We should probably check if item.documentation is empty or not. If not empty, then try to get the documentation. Else we get the documentation again.

Choose a reason for hiding this comment

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

Previous code used to initialize both the documentation and detail properties. I believe one is the signature of the method (display on the right of the method) and the other is the full documentation (displayed when you click on the i information icon).

I just tested the impact of the change, we loose the signature.
Guess we have two issues that need to be addressed:

  1. Create unit tests to ensure documentation and details properties are validated (something I failed to add). Will be useful for future usage as well, if we use a language server or the like.
  2. Ensure the above resolveCompletionItem populates both documentation and detail

Copy link
Author

Choose a reason for hiding this comment

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

I believe VSC is not calling resolve for items with documentation, but I'' double check.

Choose a reason for hiding this comment

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

Oh yes, that's true.

Copy link
Author

Choose a reason for hiding this comment

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

OK, I got the signature, good catch. But - choices, choices. Details is a plain string which is displayed on top of the documentation. But in a tooltip it is markdown/colorized. So, do we want TS way or display info with colors? I kinda like colored version...

image

Choose a reason for hiding this comment

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

I like the last item (colored version).

return item;
}
}
126 changes: 126 additions & 0 deletions src/client/providers/completionSource.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
'use strict';

import * as vscode from 'vscode';
import { PythonSettings } from '../common/configSettings';
import { Tokenizer } from '../language/tokenizer';
import { TokenType } from '../language/types';
import { JediFactory } from '../languageServices/jediProxyFactory';
import { HoverSource } from './hoverSource';
import * as proxy from './jediProxy';

class DocumentPosition {
constructor(public document: vscode.TextDocument, public position: vscode.Position) { }

public static fromObject(item: object): DocumentPosition {
// tslint:disable-next-line:no-any
return (item as any).documentPosition as DocumentPosition;

Choose a reason for hiding this comment

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

Can we change the name of the property to _documentPosition or _pyDocumentPosition, so as to ensure this will not conflict with any future API vscode adds to the TextDocument object.
Cuz using any we'll never get any compilation errors (hence the dangers of using any).

Copy link
Author

Choose a reason for hiding this comment

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

OK

}

public attachTo(item: object): void {
// tslint:disable-next-line:no-any
(item as any).documentPosition = this;
}
}

export class CompletionSource {
private jediFactory: JediFactory;
private hoverSource: HoverSource;

constructor(jediFactory: JediFactory) {
this.jediFactory = jediFactory;
this.hoverSource = new HoverSource(jediFactory);
}

public async getVsCodeCompletionItems(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken)
: Promise<vscode.CompletionItem[]> {
const result = await this.getCompletionResult(document, position, token);
if (result === undefined) {
return Promise.resolve([]);
}
return this.toVsCodeCompletions(new DocumentPosition(document, position), result, document.uri);
}

public async getDocumentation(completionItem: vscode.CompletionItem, token: vscode.CancellationToken): Promise<string> {
const documentPosition = DocumentPosition.fromObject(completionItem);
if (documentPosition === undefined) {
Promise.resolve<string>('');

Choose a reason for hiding this comment

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

Do you mean to return something? This is a no-op statement.
I think you intend to return something,
Since you're in an async method, you can return the raw value (just as you would in C# 7)

return '';

}

// Supply hover source with simulated document text where item in question was 'already typed'.
const document = documentPosition.document;
const position = documentPosition.position;
const itemText = completionItem.insertText ? completionItem.insertText : completionItem.label;
const wordRange = document.getWordRangeAtPosition(position);

const leadingRange = wordRange !== undefined
? new vscode.Range(new vscode.Position(0, 0), wordRange.start)
: new vscode.Range(new vscode.Position(0, 0), position);

const itemString = `${itemText}`;
const sourceText = `${document.getText(leadingRange)}${itemString}`;
const range = new vscode.Range(leadingRange.end, leadingRange.end.translate(0, itemString.length));

const hoverStrings = await this.hoverSource.getHoverStringsFromText(document.uri, document.fileName, range, sourceText, token);
if (!hoverStrings || hoverStrings.length === 0) {
return '';
}
return hoverStrings.join('\n');
}

private async getCompletionResult(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken)
: Promise<proxy.ICompletionResult | undefined> {
if (position.character <= 0) {
return undefined;
}
const filename = document.fileName;
const lineText = document.lineAt(position.line).text;
if (lineText.match(/^\s*\/\//)) {
return undefined;
}
// Suppress completion inside string and comments

Choose a reason for hiding this comment

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

Please end sentences with periods (.)

if (this.isPositionInsideStringOrComment(document, position)) {
return undefined;
}
const type = proxy.CommandType.Completions;
const columnIndex = position.character;

const source = document.getText();
const cmd: proxy.ICommand<proxy.ICommandResult> = {
command: type,
fileName: filename,
columnIndex: columnIndex,
lineIndex: position.line,
source: source
};

return await this.jediFactory.getJediProxyHandler<proxy.ICompletionResult>(document.uri).sendCommand(cmd, token);
}

private toVsCodeCompletions(documentPosition: DocumentPosition, data: proxy.ICompletionResult, resource: vscode.Uri): vscode.CompletionItem[] {
return data && data.items.length > 0 ? data.items.map(item => this.toVsCodeCompletion(documentPosition, item, resource)) : [];
}

private toVsCodeCompletion(documentPosition: DocumentPosition, item: proxy.IAutoCompleteItem, resource: vscode.Uri): vscode.CompletionItem {
const completionItem = new vscode.CompletionItem(item.text);
completionItem.kind = item.type;
if (PythonSettings.getInstance(resource).autoComplete.addBrackets === true &&
(item.kind === vscode.SymbolKind.Function || item.kind === vscode.SymbolKind.Method)) {
completionItem.insertText = new vscode.SnippetString(item.text).appendText('(').appendTabstop().appendText(')');
}
// ensure the built in members are at the bottom

Choose a reason for hiding this comment

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

Please end sentence with period (.)

completionItem.sortText = (completionItem.label.startsWith('__') ? 'z' : (completionItem.label.startsWith('_') ? 'y' : '__')) + completionItem.label;
documentPosition.attachTo(completionItem);
return completionItem;
}

private isPositionInsideStringOrComment(document: vscode.TextDocument, position: vscode.Position): boolean {
const tokenizeTo = position.translate(1, 0);
const text = document.getText(new vscode.Range(new vscode.Position(0, 0), tokenizeTo));
const t = new Tokenizer();
const tokens = t.Tokenize(text);
const index = tokens.getItemContaining(document.offsetAt(position));
return index >= 0 && (tokens[index].TokenType === TokenType.String || tokens[index].TokenType === TokenType.Comment);
}
}
Loading