Skip to content
This repository was archived by the owner on Oct 16, 2020. It is now read-only.

Snippet support for autocompletion #323

Merged
merged 9 commits into from
Aug 3, 2017

Conversation

tomv564
Copy link
Contributor

@tomv564 tomv564 commented Jul 26, 2017

If the client sends snippetSupport: true in the textDocument/completion/completionItem capability, this PR adds support for responding with these fields set in CompletionItem:

insertTextFormat: InsertTextFormat.Snippet,
insertText: 'bar(${1:num})',

Both VS Code and Sublime Text support these snippets.
For clients not supporting snippets, CompletionItem will now contain

insertTextFormat: InsertTextFormat.PlainText
insertText: 'bar' // same as label

Copy link
Contributor

@felixfbecker felixfbecker left a comment

Choose a reason for hiding this comment

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

Thanks, awesome!

@@ -173,6 +174,8 @@ export class TypeScriptService {
}
};

private completionWithSnippets: boolean = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add docblock

Copy link
Contributor

Choose a reason for hiding this comment

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

The name is a bit vague. Please add a supports prefix or something like that

@@ -200,6 +203,10 @@ export class TypeScriptService {
if (params.rootUri || params.rootPath) {
this.root = params.rootPath || uri2path(params.rootUri!);
this.rootUri = params.rootUri || path2uri(params.rootPath!);

if (params.capabilities.textDocument && params.capabilities.textDocument.completion && params.capabilities.textDocument.completion.completionItem) {
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for the if, just set the flag to the result of the boolean expression

@@ -998,10 +1005,19 @@ export class TypeScriptService {
if (completions == null) {
return [];
}
function createSnippet(entry: ts.CompletionEntryDetails): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is only used in one place, so no benefit to put in an inline function, but makes the code not chronological. Better to do it inline and document

@@ -31,7 +36,7 @@ export interface TestContext {
* @param createService A factory that creates the TypeScript service. Allows to test subclasses of TypeScriptService
* @param files A Map from URI to file content of files that should be available in the workspace
*/
export const initializeTypeScriptService = (createService: TypeScriptServiceFactory, rootUri: string, files: Map<string, string>) => async function (this: TestContext & IBeforeAndAfterContext): Promise<void> {
export const initializeTypeScriptService = (createService: TypeScriptServiceFactory, rootUri: string, files: Map<string, string>, clientCapabilities?: ClientCapabilities) => async function (this: TestContext & IBeforeAndAfterContext): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Put the default value into the parameter

@@ -16,6 +16,11 @@ import { IBeforeAndAfterContext, ISuiteCallbackContext, ITestCallbackContext } f
chai.use(chaiAsPromised);
const assert = chai.assert;

const defaultCapabilities: ClientCapabilities = {
Copy link
Contributor

Choose a reason for hiding this comment

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

DEFAULT_CAPABILITIES

const parameters = entry.displayParts
.filter(p => p.kind === 'parameterName')
.map(p => {
index++;
Copy link
Contributor

Choose a reason for hiding this comment

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

map passes the index as a second parameter

@codecov
Copy link

codecov bot commented Aug 3, 2017

Codecov Report

Merging #323 into master will increase coverage by 0.24%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #323      +/-   ##
==========================================
+ Coverage   59.64%   59.88%   +0.24%     
==========================================
  Files          14       14              
  Lines        2126     2139      +13     
  Branches      347      350       +3     
==========================================
+ Hits         1268     1281      +13     
  Misses        708      708              
  Partials      150      150
Impacted Files Coverage Δ
src/typescript-service.ts 72.22% <100%> (+0.7%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54fb99e...a9cb785. Read the comment docs.

tomv564 and others added 3 commits August 3, 2017 16:53
Rename to supportsCompletionWithSnippets + doctype
Fix up assignment
Pull createSnippet into function
Use index param from map
rename constant, use as default value
@felixfbecker felixfbecker merged commit 8591294 into sourcegraph:master Aug 3, 2017
@tomv564
Copy link
Contributor Author

tomv564 commented Aug 3, 2017

Great, thanks for the reviews!

@tomv564 tomv564 deleted the support-snippets branch September 9, 2017 09:37
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.

2 participants