Skip to content

Provide full signature help with highlighted parameter #416

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 37 commits into from
Dec 15, 2017
Merged

Provide full signature help with highlighted parameter #416

merged 37 commits into from
Dec 15, 2017

Conversation

MikhailArkhipov
Copy link

@MikhailArkhipov MikhailArkhipov commented Dec 14, 2017

Fix #70

Ignore Russian translation from pending PR

image

image

@@ -55,8 +56,13 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider {
signature.activeParameter = def.paramindex;
// Don't display the documentation, as vs code doesn't format the docmentation.
// i.e. line feeds are not respected, long content is stripped.
const docLines = def.docstring.splitLines();
const label = docLines[0].trim();
const documentation = docLines.length > 1 ? docLines.filter((line, index) => index > 0).join(EOL) : '';

Choose a reason for hiding this comment

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

You could improve the code as follows:

const label = docLines.shift();
const documentation = docLines.join(EOL);

const sig = <vscode.SignatureInformation>{
label: def.description,
label: label,
documentation: documentation,

Choose a reason for hiding this comment

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

instead of documentation: documentation, please use documentation,
That's the preferred style for object literals.

@@ -65,7 +71,7 @@ export class PythonSignatureProvider implements vscode.SignatureHelpProvider {
}
return <vscode.ParameterInformation>{
documentation: arg.docstring.length > 0 ? arg.docstring : arg.description,
label: arg.description.length > 0 ? arg.description : arg.name
label: arg.name

Choose a reason for hiding this comment

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

Lol, I hang my head in shame for this simple bug. 😞 . Nothing to do here.

@DonJayamanne
Copy link

DonJayamanne commented Dec 14, 2017

@MikhailArkhipov looks good. will approve after the release.
Btw, the travis tests are failing.

@MikhailArkhipov
Copy link
Author

MikhailArkhipov commented Dec 14, 2017

On Python 3.6 VSC gets different signature description which sometimes does not have signature in it. Looks like Jedi/Py3 issue. Ex (without new code)

Stock
image

Funny that pow argument comes from Jedi with both name and description as 'with two arguments'. This is probably because there is no actual signature in docs and Jedi confuses sentence Function pow ... (with two arguments) ... with a function signature...

@MikhailArkhipov
Copy link
Author

@DonJayamanne could you please look one more time. I removed requirement for param name to be a word in completions.py so we support ... in signature (ex print) and added separate tests for Py3 and Py2 since behavior is different in some functions (notably, pow and some others)

@@ -12,7 +12,8 @@ const options: MochaSetupOptions & { retries: number } = {
ui: 'tdd',
useColors: true,
timeout: 25000,
retries: 3
retries: 3,
grep: 'Signatures'

Choose a reason for hiding this comment

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

Please remove this.

Choose a reason for hiding this comment

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

@MikhailArkhipov please remove the grep once done it should be good. I'll approve this now. Just wanted to ensure you got this fixed before merging.

@MikhailArkhipov MikhailArkhipov merged commit 372d9b7 into microsoft:master Dec 15, 2017
DonJayamanne added a commit that referenced this pull request Jan 9, 2018
* upstream/master:
  Make use of new execution layer instead of spawning processes (#425)
  Use localhost instead of 0.0.0.0 for all local socket servers (#417)
  Provide full signature help with highlighted parameter (#416)
  Change `os.arch()` to `npm` `arch` to fix wrong arch detection. (#419)
  Document our Russian support (#424)
  Bump the version number to the next release as an alpha (#422)
  Russian translation (#411)
  Speed up virtual environment detection in workspace (#405)
@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to get all parameter info when filling in a function param list
2 participants