Skip to content

Support completions for JSDoc @param tag names#16299

Merged
3 commits merged into
masterfrom
jsdocParameterNameCompletion
Jun 7, 2017
Merged

Support completions for JSDoc @param tag names#16299
3 commits merged into
masterfrom
jsdocParameterNameCompletion

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jun 6, 2017

Fixes #15853

@ghost ghost force-pushed the jsdocParameterNameCompletion branch from cff0089 to 117926a Compare June 6, 2017 18:20
Comment thread src/compiler/parser.ts
}

function finishNode<T extends Node>(node: T, end?: number): T {
node.end = end === undefined ? scanner.getStartPos() : end;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we use this all the time. the new code will result in two statements. please run perf tests to ensure we are not messing up the engine optimizations.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Since this isn't necessary, reverted the change. Will do in a separate branch.

Comment thread src/compiler/parser.ts Outdated
// Include trailing whitespace inside the range of the `@param` tag.
// This way we can provide completions after `@param `.
let end = scanner.getStartPos();
while (ts.isWhiteSpaceSingleLine(scanner.getText().charCodeAt(end))) end++;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we do not do that for other nodes. why do we need this?

Copy link
Copy Markdown
Author

@ghost ghost Jun 6, 2017

Choose a reason for hiding this comment

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

I was using it so that at @param /*completions here*/ you would get completions by being inside the @param node. But we can do this a different way by changing getJsDocTagAtPosition to handle ranges differently.

Comment thread src/compiler/core.ts

export function mapDefined<T>(array: ReadonlyArray<T>, mapFn: (x: T, i: number) => T | undefined): ReadonlyArray<T> {
const result: T[] = [];
export function mapDefined<T, U>(array: ReadonlyArray<T>, mapFn: (x: T, i: number) => U | undefined): U[] {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not ReadonlyArray<U>?

Copy link
Copy Markdown
Author

@ghost ghost Jun 6, 2017

Choose a reason for hiding this comment

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

We always create a fresh array. If the caller wants a readonly array they can type it that way.
Can't just change this one to ReadonlyArray because services/types.ts currently uses mutable arrays. We should change that separately. #16312

@ghost ghost merged commit abb9681 into master Jun 7, 2017
@ghost ghost deleted the jsdocParameterNameCompletion branch June 7, 2017 19:28
@ghost ghost mentioned this pull request Jun 7, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
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.

3 participants