Skip to content

Commit

Permalink
Provide full signature help with highlighted parameter (#416)
Browse files Browse the repository at this point in the history
* Basic tokenizer

* Fixed property names

* Tests, round I

* Tests, round II

* tokenizer test

* Remove temorary change

* Fix merge issue

* Merge conflict

* Merge conflict

* Completion test

* Fix last line

* Fix javascript math

* Make test await for results

* Add license headers

* Rename definitions to types

* License headers

* Fix typo in completion details (typo)

* Fix hover test

* Russian translations

* Update to better translation

* Fix typo

*  #70 How to get all parameter info when filling in a function param list

* Fix #70 How to get all parameter info when filling in a function param list

* Clean up

* Clean imports

* CR feedback

* Trim whitespace for test stability

* More tests

* Better handle no-parameters documentation

* Better handle ellipsis and Python3

* Remove test change
  • Loading branch information
Mikhail Arkhipov authored Dec 15, 2017
1 parent 992b654 commit 372d9b7
Show file tree
Hide file tree
Showing 8 changed files with 174 additions and 19 deletions.
5 changes: 0 additions & 5 deletions pythonFiles/completion.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import traceback
import platform

WORD_RE = re.compile(r'\w')
jediPreview = False

class RedirectStdout(object):
Expand Down Expand Up @@ -111,8 +110,6 @@ def _get_call_signatures(self, script):
continue
if param.name == 'self' and pos == 0:
continue
if WORD_RE.match(param.name) is None:
continue
try:
name, value = param.description.split('=')
except ValueError:
Expand Down Expand Up @@ -155,8 +152,6 @@ def _get_call_signatures_with_args(self, script):
continue
if param.name == 'self' and pos == 0:
continue
if WORD_RE.match(param.name) is None:
continue
try:
name, value = param.description.split('=')
except ValueError:
Expand Down
4 changes: 2 additions & 2 deletions src/client/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,8 @@ export function getSubDirectories(rootDir: string): Promise<string[]> {
subDirs.push(fullPath);
}
}
catch (ex) {
}
// tslint:disable-next-line:no-empty
catch (ex) {}
});
resolve(subDirs);
});
Expand Down
44 changes: 32 additions & 12 deletions src/client/providers/signatureProvider.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

import { EOL } from 'os';
import * as vscode from 'vscode';
import { CancellationToken, Position, SignatureHelp, TextDocument } from 'vscode';
import { JediFactory } from '../languageServices/jediProxyFactory';
Expand Down Expand Up @@ -53,21 +54,40 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider {

data.definitions.forEach(def => {
signature.activeParameter = def.paramindex;
// Don't display the documentation, as vs code doesn't format the docmentation.
// Don't display the documentation, as vs code doesn't format the documentation.
// i.e. line feeds are not respected, long content is stripped.

// Some functions do not come with parameter docs
let label: string;
let documentation: string;
const validParamInfo = def.params && def.params.length > 0 && def.docstring.startsWith(`${def.name}(`);

if (validParamInfo) {
const docLines = def.docstring.splitLines();
label = docLines.shift().trim();
documentation = docLines.join(EOL).trim();
} else {
label = def.description;
documentation = def.docstring;
}

const sig = <vscode.SignatureInformation>{
label: def.description,
label,
documentation,
parameters: []
};
sig.parameters = def.params.map(arg => {
if (arg.docstring.length === 0) {
arg.docstring = extractParamDocString(arg.name, def.docstring);
}
return <vscode.ParameterInformation>{
documentation: arg.docstring.length > 0 ? arg.docstring : arg.description,
label: arg.description.length > 0 ? arg.description : arg.name
};
});

if (validParamInfo) {
sig.parameters = def.params.map(arg => {
if (arg.docstring.length === 0) {
arg.docstring = extractParamDocString(arg.name, def.docstring);
}
return <vscode.ParameterInformation>{
documentation: arg.docstring.length > 0 ? arg.docstring : arg.description,
label: arg.name.trim()
};
});
}
signature.signatures.push(sig);
});
return signature;
Expand All @@ -85,7 +105,7 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider {
source: document.getText()
};
return this.jediFactory.getJediProxyHandler<proxy.IArgumentsResult>(document.uri).sendCommand(cmd, token).then(data => {
return PythonSignatureProvider.parseData(data);
return data ? PythonSignatureProvider.parseData(data) : new SignatureHelp();
});
}
}
2 changes: 2 additions & 0 deletions src/test/pythonFiles/signature/basicSig.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
range(c, 1,

6 changes: 6 additions & 0 deletions src/test/pythonFiles/signature/classCtor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class Person:
def __init__(self, name, age = 23):
self.name = name
self.age = age

p1 = Person('Bob', )
1 change: 1 addition & 0 deletions src/test/pythonFiles/signature/ellipsis.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
print(a, b, c)
1 change: 1 addition & 0 deletions src/test/pythonFiles/signature/noSigPy3.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pow()
130 changes: 130 additions & 0 deletions src/test/signature/signature.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
'use strict';

import * as assert from 'assert';
import * as path from 'path';
import * as vscode from 'vscode';
import { PythonSettings } from '../../client/common/configSettings';
import { execPythonFile } from '../../client/common/utils';
import { rootWorkspaceUri } from '../common';
import { closeActiveWindows, initialize, initializeTest } from '../initialize';

const autoCompPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'signature');

class SignatureHelpResult {
constructor(
public line: number,
public index: number,
public signaturesCount: number,
public activeParameter: number,
public parameterName: string | null) { }
}

// tslint:disable-next-line:max-func-body-length
suite('Signatures', () => {
let isPython3: Promise<boolean>;
suiteSetup(async () => {
await initialize();
const version = await execPythonFile(rootWorkspaceUri, PythonSettings.getInstance(rootWorkspaceUri).pythonPath, ['--version'], __dirname, true);
isPython3 = Promise.resolve(version.indexOf('3.') >= 0);
});
setup(initializeTest);
suiteTeardown(closeActiveWindows);
teardown(closeActiveWindows);

test('For ctor', async () => {
const expected = [
new SignatureHelpResult(5, 11, 0, 0, null),
new SignatureHelpResult(5, 12, 1, 0, 'name'),
new SignatureHelpResult(5, 13, 1, 0, 'name'),
new SignatureHelpResult(5, 14, 1, 0, 'name'),
new SignatureHelpResult(5, 15, 1, 0, 'name'),
new SignatureHelpResult(5, 16, 1, 0, 'name'),
new SignatureHelpResult(5, 17, 1, 0, 'name'),
new SignatureHelpResult(5, 18, 1, 1, 'age'),
new SignatureHelpResult(5, 19, 1, 1, 'age'),
new SignatureHelpResult(5, 20, 0, 0, null)
];

const document = await openDocument(path.join(autoCompPath, 'classCtor.py'));
for (let i = 0; i < expected.length; i += 1) {
await checkSignature(expected[i], document!.uri, i);
}
});

test('For intrinsic', async () => {
const expected = [
new SignatureHelpResult(0, 0, 0, 0, null),
new SignatureHelpResult(0, 1, 0, 0, null),
new SignatureHelpResult(0, 2, 0, 0, null),
new SignatureHelpResult(0, 3, 0, 0, null),
new SignatureHelpResult(0, 4, 0, 0, null),
new SignatureHelpResult(0, 5, 0, 0, null),
new SignatureHelpResult(0, 6, 1, 0, 'start'),
new SignatureHelpResult(0, 7, 1, 0, 'start'),
new SignatureHelpResult(0, 8, 1, 1, 'stop'),
new SignatureHelpResult(0, 9, 1, 1, 'stop'),
new SignatureHelpResult(0, 10, 1, 1, 'stop'),
new SignatureHelpResult(0, 11, 1, 2, 'step'),
new SignatureHelpResult(1, 0, 1, 2, 'step')
];

const document = await openDocument(path.join(autoCompPath, 'basicSig.py'));
for (let i = 0; i < expected.length; i += 1) {
await checkSignature(expected[i], document!.uri, i);
}
});

test('For ellipsis', async () => {
if (!await isPython3) {
return;
}
const expected = [
new SignatureHelpResult(0, 5, 0, 0, null),
new SignatureHelpResult(0, 6, 1, 0, 'value'),
new SignatureHelpResult(0, 7, 1, 0, 'value'),
new SignatureHelpResult(0, 8, 1, 1, '...'),
new SignatureHelpResult(0, 9, 1, 1, '...'),
new SignatureHelpResult(0, 10, 1, 1, '...'),
new SignatureHelpResult(0, 11, 1, 2, 'sep'),
new SignatureHelpResult(0, 12, 1, 2, 'sep')
];

const document = await openDocument(path.join(autoCompPath, 'ellipsis.py'));
for (let i = 0; i < expected.length; i += 1) {
await checkSignature(expected[i], document!.uri, i);
}
});

test('For pow', async () => {
let expected: SignatureHelpResult;
if (await isPython3) {
expected = new SignatureHelpResult(0, 4, 1, 0, null);
} else {
expected = new SignatureHelpResult(0, 4, 1, 0, 'x');
}

const document = await openDocument(path.join(autoCompPath, 'noSigPy3.py'));
await checkSignature(expected, document!.uri, 0);
});
});

async function openDocument(documentPath: string): Promise<vscode.TextDocument | undefined> {
const document = await vscode.workspace.openTextDocument(documentPath);
await vscode.window.showTextDocument(document!);
return document;
}

async function checkSignature(expected: SignatureHelpResult, uri: vscode.Uri, caseIndex: number) {
const position = new vscode.Position(expected.line, expected.index);
const actual = await vscode.commands.executeCommand<vscode.SignatureHelp>('vscode.executeSignatureHelpProvider', uri, position);
assert.equal(actual!.signatures.length, expected.signaturesCount, `Signature count does not match, case ${caseIndex}`);
if (expected.signaturesCount > 0) {
assert.equal(actual!.activeParameter, expected.activeParameter, `Parameter index does not match, case ${caseIndex}`);
if (expected.parameterName) {
const parameter = actual!.signatures[0].parameters[expected.activeParameter];
assert.equal(parameter.label, expected.parameterName, `Parameter name is incorrect, case ${caseIndex}`);
}
}
}

0 comments on commit 372d9b7

Please sign in to comment.