Skip to content

Support services settings #22236

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
13 commits merged into from
Mar 20, 2018
17 changes: 8 additions & 9 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,6 @@ namespace FourSlash {

function memoWrap(ls: ts.LanguageService, target: TestState): ts.LanguageService {
const cacheableMembers: (keyof typeof ls)[] = [
"getCompletionsAtPosition",
"getCompletionEntryDetails",
"getCompletionEntrySymbol",
"getQuickInfoAtPosition",
Expand Down Expand Up @@ -1719,7 +1718,7 @@ Actual: ${stringify(fullActual)}`);
Harness.IO.log(stringify(sigHelp));
}

public printCompletionListMembers(options: ts.GetCompletionsAtPositionOptions | undefined) {
public printCompletionListMembers(options: ts.Options | undefined) {
const completions = this.getCompletionListAtCaret(options);
this.printMembersOrCompletions(completions);
}
Expand Down Expand Up @@ -1818,7 +1817,7 @@ Actual: ${stringify(fullActual)}`);
}
else if (prevChar === " " && /A-Za-z_/.test(ch)) {
/* Completions */
this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
this.languageService.getCompletionsAtPosition(this.activeFile.fileName, offset, ts.defaultOptions);
}

if (i % checkCadence === 0) {
Expand Down Expand Up @@ -2393,7 +2392,7 @@ Actual: ${stringify(fullActual)}`);
public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) {
this.goToMarker(markerName);

const actualCompletion = this.getCompletionListAtCaret({ includeExternalModuleExports: true, includeInsertTextCompletions: false }).entries.find(e =>
const actualCompletion = this.getCompletionListAtCaret({ ...ts.defaultOptions, includeExternalModuleExports: true }).entries.find(e =>
e.name === options.name && e.source === options.source);

if (!actualCompletion.hasAction) {
Expand Down Expand Up @@ -2445,7 +2444,7 @@ Actual: ${stringify(fullActual)}`);
const { fixId, newFileContent } = options;
const fixIds = ts.mapDefined(this.getCodeFixes(this.activeFile.fileName), a => a.fixId);
ts.Debug.assert(ts.contains(fixIds, fixId), "No available code fix has that group id.", () => `Expected '${fixId}'. Available action ids: ${fixIds}`);
const { changes, commands } = this.languageService.getCombinedCodeFix({ type: "file", fileName: this.activeFile.fileName }, fixId, this.formatCodeSettings);
const { changes, commands } = this.languageService.getCombinedCodeFix({ type: "file", fileName: this.activeFile.fileName }, fixId, this.formatCodeSettings, ts.defaultOptions);
assert.deepEqual(commands, options.commands);
assert(changes.every(c => c.fileName === this.activeFile.fileName), "TODO: support testing codefixes that touch multiple files");
this.applyChanges(changes);
Expand Down Expand Up @@ -2525,7 +2524,7 @@ Actual: ${stringify(fullActual)}`);
return;
}

return this.languageService.getCodeFixesAtPosition(fileName, diagnostic.start, diagnostic.start + diagnostic.length, [diagnostic.code], this.formatCodeSettings);
return this.languageService.getCodeFixesAtPosition(fileName, diagnostic.start, diagnostic.start + diagnostic.length, [diagnostic.code], this.formatCodeSettings, ts.defaultOptions);
});
}

Expand Down Expand Up @@ -4419,7 +4418,7 @@ namespace FourSlashInterface {
this.state.printCurrentSignatureHelp();
}

public printCompletionListMembers(options: ts.GetCompletionsAtPositionOptions | undefined) {
public printCompletionListMembers(options: ts.Options | undefined) {
this.state.printCompletionListMembers(options);
}

Expand Down Expand Up @@ -4616,11 +4615,11 @@ namespace FourSlashInterface {
}

export type ExpectedCompletionEntry = string | { name: string, insertText?: string, replacementSpan?: FourSlash.Range };
export interface CompletionsAtOptions extends ts.GetCompletionsAtPositionOptions {
export interface CompletionsAtOptions extends Partial<ts.Options> {
isNewIdentifierLocation?: boolean;
}

export interface VerifyCompletionListContainsOptions extends ts.GetCompletionsAtPositionOptions {
export interface VerifyCompletionListContainsOptions extends ts.Options {
sourceDisplay: string;
isRecommended?: true;
insertText?: string;
Expand Down
2 changes: 1 addition & 1 deletion src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ namespace Harness.LanguageService {
getEncodedSemanticClassifications(fileName: string, span: ts.TextSpan): ts.Classifications {
return unwrapJSONCallResult(this.shim.getEncodedSemanticClassifications(fileName, span.start, span.length));
}
getCompletionsAtPosition(fileName: string, position: number, options: ts.GetCompletionsAtPositionOptions | undefined): ts.CompletionInfo {
getCompletionsAtPosition(fileName: string, position: number, options: ts.Options | undefined): ts.CompletionInfo {
return unwrapJSONCallResult(this.shim.getCompletionsAtPosition(fileName, position, options));
}
getCompletionEntryDetails(fileName: string, position: number, entryName: string, options: ts.FormatCodeOptions | undefined, source: string | undefined): ts.CompletionEntryDetails {
Expand Down
2 changes: 1 addition & 1 deletion src/harness/unittests/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ namespace ts.server {

session.onMessage(JSON.stringify(configureRequest));

assert.equal(session.getProjectService().getFormatCodeOptions().indentStyle, IndentStyle.Block);
assert.equal(session.getProjectService().getFormatCodeOptions("" as NormalizedPath).indentStyle, IndentStyle.Block);

const setOptionsRequest: protocol.SetCompilerOptionsForInferredProjectsRequest = {
command: CommandNames.CompilerOptionsForInferredProjects,
Expand Down
16 changes: 8 additions & 8 deletions src/harness/unittests/tsserverProjectSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1321,13 +1321,13 @@ namespace ts.projectSystem {
service.checkNumberOfProjects({ externalProjects: 1 });
checkProjectActualFiles(service.externalProjects[0], [f1.path, f2.path, libFile.path]);

const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, defaultOptions);
// should contain completions for string
assert.isTrue(completions1.entries.some(e => e.name === "charAt"), "should contain 'charAt'");
assert.isFalse(completions1.entries.some(e => e.name === "toExponential"), "should not contain 'toExponential'");

service.closeClientFile(f2.path);
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 2, defaultOptions);
// should contain completions for string
assert.isFalse(completions2.entries.some(e => e.name === "charAt"), "should not contain 'charAt'");
assert.isTrue(completions2.entries.some(e => e.name === "toExponential"), "should contain 'toExponential'");
Expand All @@ -1353,11 +1353,11 @@ namespace ts.projectSystem {
service.checkNumberOfProjects({ externalProjects: 1 });
checkProjectActualFiles(service.externalProjects[0], [f1.path, f2.path, libFile.path]);

const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
const completions1 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, defaultOptions);
assert.isTrue(completions1.entries.some(e => e.name === "somelongname"), "should contain 'somelongname'");

service.closeClientFile(f2.path);
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
const completions2 = service.externalProjects[0].getLanguageService().getCompletionsAtPosition(f1.path, 0, defaultOptions);
assert.isFalse(completions2.entries.some(e => e.name === "somelongname"), "should not contain 'somelongname'");
const sf2 = service.externalProjects[0].getLanguageService().getProgram().getSourceFile(f2.path);
assert.equal(sf2.text, "");
Expand Down Expand Up @@ -1962,7 +1962,7 @@ namespace ts.projectSystem {

// Check identifiers defined in HTML content are available in .ts file
const project = configuredProjectAt(projectService, 0);
let completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 1, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
let completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 1, defaultOptions);
assert(completions && completions.entries[0].name === "hello", `expected entry hello to be in completion list`);

// Close HTML file
Expand All @@ -1976,7 +1976,7 @@ namespace ts.projectSystem {
checkProjectActualFiles(configuredProjectAt(projectService, 0), [file1.path, file2.path, config.path]);

// Check identifiers defined in HTML content are not available in .ts file
completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 5, { includeExternalModuleExports: false, includeInsertTextCompletions: false });
completions = project.getLanguageService().getCompletionsAtPosition(file1.path, 5, defaultOptions);
assert(completions && completions.entries[0].name !== "hello", `unexpected hello entry in completion list`);
});

Expand Down Expand Up @@ -2629,7 +2629,7 @@ namespace ts.projectSystem {
assert.equal(lastEvent.data.project, project, "project name");
assert.isFalse(lastEvent.data.languageServiceEnabled, "Language service state");

const options = projectService.getFormatCodeOptions();
const options = projectService.getFormatCodeOptions(f1.path as server.NormalizedPath);
const edits = project.getLanguageService().getFormattingEditsForDocument(f1.path, options);
assert.deepEqual(edits, [{ span: createTextSpan(/*start*/ 7, /*length*/ 3), newText: " " }]);
});
Expand Down Expand Up @@ -3525,7 +3525,7 @@ namespace ts.projectSystem {
const projectService = createProjectService(host);
projectService.openClientFile(f1.path);

const defaultSettings = projectService.getFormatCodeOptions();
const defaultSettings = projectService.getFormatCodeOptions(f1.path as server.NormalizedPath);

// set global settings
const newGlobalSettings1 = clone(defaultSettings);
Expand Down
5 changes: 3 additions & 2 deletions src/server/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,9 @@ namespace ts.server {
};
}

getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions | undefined): CompletionInfo {
const args: protocol.CompletionsRequestArgs = { ...this.createFileLocationRequestArgs(fileName, position), ...options };
getCompletionsAtPosition(fileName: string, position: number, _settings: Options | undefined): CompletionInfo {
// Not passing along 'settings' because server should already have those from the 'configure' command
const args: protocol.CompletionsRequestArgs = this.createFileLocationRequestArgs(fileName, position);

const request = this.processRequest<protocol.CompletionsRequest>(CommandNames.Completions, args);
const response = this.processResponse<protocol.CompletionsResponse>(request);
Expand Down
24 changes: 14 additions & 10 deletions src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ namespace ts.server {

export interface HostConfiguration {
formatCodeOptions: FormatCodeSettings;
options: Options;
hostInfo: string;
extraFileExtensions?: JsFileExtensionInfo[];
}
Expand Down Expand Up @@ -442,6 +443,7 @@ namespace ts.server {

this.hostConfiguration = {
formatCodeOptions: getDefaultFormatCodeSettings(this.host),
options: defaultOptions,
hostInfo: "Unknown host",
extraFileExtensions: []
};
Expand Down Expand Up @@ -703,15 +705,14 @@ namespace ts.server {
return project.dirty && project.updateGraph();
}

getFormatCodeOptions(file?: NormalizedPath) {
let formatCodeSettings: FormatCodeSettings;
if (file) {
const info = this.getScriptInfoForNormalizedPath(file);
if (info) {
formatCodeSettings = info.getFormatCodeSettings();
}
}
return formatCodeSettings || this.hostConfiguration.formatCodeOptions;
getFormatCodeOptions(file: NormalizedPath) {
const info = this.getScriptInfoForNormalizedPath(file);
return info && info.getFormatCodeSettings() || this.hostConfiguration.formatCodeOptions;
}

getOptions(file: NormalizedPath): Options {
const info = this.getScriptInfoForNormalizedPath(file);
return info && info.getOptions() || this.hostConfiguration.options;
}

private onSourceFileChanged(fileName: NormalizedPath, eventKind: FileWatcherEventKind) {
Expand Down Expand Up @@ -1829,7 +1830,7 @@ namespace ts.server {
if (args.file) {
const info = this.getScriptInfoForNormalizedPath(toNormalizedPath(args.file));
if (info) {
info.setFormatOptions(convertFormatOptions(args.formatOptions));
info.setOptions(convertFormatOptions(args.formatOptions), args.options);
this.logger.info(`Host configuration update for file ${args.file}`);
}
}
Expand All @@ -1842,6 +1843,9 @@ namespace ts.server {
mergeMapLikes(this.hostConfiguration.formatCodeOptions, convertFormatOptions(args.formatOptions));
this.logger.info("Format host information updated");
}
if (args.options) {
mergeMapLikes(this.hostConfiguration.options, args.options);
}
if (args.extraFileExtensions) {
this.hostConfiguration.extraFileExtensions = args.extraFileExtensions;
// We need to update the project structures again as it is possible that existing
Expand Down
26 changes: 20 additions & 6 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1232,6 +1232,8 @@ namespace ts.server.protocol {
*/
formatOptions?: FormatCodeSettings;

options?: Options;

/**
* The host's additional supported .js file extensions
*/
Expand Down Expand Up @@ -1707,15 +1709,13 @@ namespace ts.server.protocol {
*/
prefix?: string;
/**
* If enabled, TypeScript will search through all external modules' exports and add them to the completions list.
* This affects lone identifier completions but not completions on the right hand side of `obj.`.
* @deprecated Use Options
*/
includeExternalModuleExports: boolean;
includeExternalModuleExports?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure why should these be lumped with the other... i mean we can change to a global config system, but maybe on a separate change, and then we need to discuss that as well with the VSCode folks and with @amcasey

Copy link
Contributor

Choose a reason for hiding this comment

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

Spoke to @amcasey and seems like it will work for him. so i am fine with he change. we probably need to give them more descriptive names.

Copy link
Author

Choose a reason for hiding this comment

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

Wouldn't changing the name be a breaking change though? Although we could deprecate the existing name and add an equivalent one.

Copy link
Member

Choose a reason for hiding this comment

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

I think @mhegazy meant that we should give them more descriptive names in their new location. In their old location, they have to stay as-is for back-compat.

Copy link
Author

Choose a reason for hiding this comment

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

@amcasey Do you have some good suggestions for the new names?

Copy link
Member

Choose a reason for hiding this comment

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

I had envisioned that they would be in a nested object called (e.g.) "completions". However, if that's not idiomatic, incorporating "completions" into the name should be sufficient - e.g. "includeCompletionsForExternalModuleExports".

/**
* If enabled, the completion list will include completions with invalid identifier names.
* For those entries, The `insertText` and `replacementSpan` properties will be set to change from `.x` property access to `["x"]`.
* @deprecated Use Options
*/
includeInsertTextCompletions: boolean;
includeInsertTextCompletions?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we have two separate names for the same concept (i.e. different thing in protocol vs services)

Copy link
Author

Choose a reason for hiding this comment

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

This is a deprecated property of CompletionsRequestArgs to support the old name, the new name is later on in this file.

}

/**
Expand Down Expand Up @@ -2588,6 +2588,20 @@ namespace ts.server.protocol {
insertSpaceBeforeTypeAnnotation?: boolean;
}

export interface Options {
Copy link
Member

Choose a reason for hiding this comment

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

UserPreferences? Thoughts @RyanCavanaugh? (keep #22236 (comment) in mind)

quote: "double" | "single";
Copy link
Contributor

Choose a reason for hiding this comment

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

these should be all be optional.

/**
* If enabled, TypeScript will search through all external modules' exports and add them to the completions list.
* This affects lone identifier completions but not completions on the right hand side of `obj.`.
*/
includeExternalModuleExports: boolean;
/**
* If enabled, the completion list will include completions with invalid identifier names.
* For those entries, The `insertText` and `replacementSpan` properties will be set to change from `.x` property access to `["x"]`.
*/
includeInsertTextCompletions: boolean;
}

export interface CompilerOptions {
allowJs?: boolean;
allowSyntheticDefaultImports?: boolean;
Expand Down
23 changes: 15 additions & 8 deletions src/server/scriptInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ namespace ts.server {
* All projects that include this file
*/
readonly containingProjects: Project[] = [];
private formatCodeSettings: FormatCodeSettings;
private formatSettings: FormatCodeSettings | undefined;
private options: Options | undefined;

/* @internal */
fileWatcher: FileWatcher;
Expand Down Expand Up @@ -293,9 +294,8 @@ namespace ts.server {
return this.realpath && this.realpath !== this.path ? this.realpath : undefined;
}

getFormatCodeSettings() {
return this.formatCodeSettings;
}
getFormatCodeSettings(): FormatCodeSettings { return this.formatSettings; }
getOptions(): Options { return this.options; }

attachToProject(project: Project): boolean {
const isNew = !this.isAttached(project);
Expand Down Expand Up @@ -388,12 +388,19 @@ namespace ts.server {
}
}

setFormatOptions(formatSettings: FormatCodeSettings): void {
setOptions(formatSettings: FormatCodeSettings, options: Options): void {
if (formatSettings) {
if (!this.formatCodeSettings) {
this.formatCodeSettings = getDefaultFormatCodeSettings(this.host);
if (!this.formatSettings) {
this.formatSettings = getDefaultFormatCodeSettings(this.host);
}
mergeMapLikes(this.formatSettings, formatSettings);
}

if (options) {
if (!this.options) {
this.options = clone(defaultOptions);
}
mergeMapLikes(this.formatCodeSettings, formatSettings);
mergeMapLikes(this.options, options);
}
}

Expand Down
Loading