make quick fix be a command and a score

This commit is contained in:
Johannes Rieken 2015-12-02 13:02:18 +01:00
parent 0f8b0d7225
commit 797f9bf0ab
15 changed files with 91 additions and 98 deletions

View file

@ -47,7 +47,12 @@ export default class OmnisharpCodeActionProvider extends AbstractProvider implem
return {
title: ca.Name,
command: this._commandId,
arguments: [ca.Identifier, document, range]
arguments: [<V2.RunCodeActionRequest>{
Filename: document.fileName,
Selection: OmnisharpCodeActionProvider._asRange(range),
Identifier: ca.Identifier,
WantsTextChanges: true
}]
};
});
}, (error) => {
@ -55,14 +60,7 @@ export default class OmnisharpCodeActionProvider extends AbstractProvider implem
});
}
private _runCodeAction(id: string, document: TextDocument, range: Range): Promise<any> {
let req: V2.RunCodeActionRequest = {
Filename: document.fileName,
Selection: OmnisharpCodeActionProvider._asRange(range),
Identifier: id,
WantsTextChanges: true
};
private _runCodeAction(req: V2.RunCodeActionRequest): Promise<any> {
return this._server.makeRequest<V2.RunCodeActionResponse>(V2.RunCodeAction, req).then(response => {

View file

@ -471,10 +471,8 @@ export interface ISuggestSupport {
* Interface used to quick fix typing errors while accesing member fields.
*/
export interface IQuickFix {
label: string;
id: any;
command: ICommand;
score: number;
documentation?: string;
}
export interface IQuickFixResult {
@ -484,7 +482,8 @@ export interface IQuickFixResult {
export interface IQuickFixSupport {
getQuickFixes(resource: URI, range: IMarker | EditorCommon.IRange): TPromise<IQuickFix[]>;
runQuickFixAction(resource: URI, range: EditorCommon.IRange, id: any):TPromise<IQuickFixResult>;
//TODO@joh this should be removed in the furture such that we can trust the command and it's args
runQuickFixAction(resource: URI, range: EditorCommon.IRange, quickFix: IQuickFix):TPromise<IQuickFixResult>;
}
export interface IParameter {

View file

@ -276,9 +276,9 @@ class MarkerNavigationWidget extends ZoneWidget.ZoneWidget {
}
container.span({
class: 'quickfixentry',
text: fix.label
text: fix.command.title
}).on(DOM.EventType.CLICK,() => {
mode.quickFixSupport.runQuickFixAction(this.editor.getModel().getAssociatedResource(), marker, fix.id).then(result => {
mode.quickFixSupport.runQuickFixAction(this.editor.getModel().getAssociatedResource(), marker, fix).then(result => {
return bulkEdit(this._eventService, this._editorService, this.editor, result.edits);
});
return true;

View file

@ -77,7 +77,7 @@ export class QuickFixController implements EditorCommon.IEditorContribution {
return;
}
fix.support.runQuickFixAction(this.editor.getModel().getAssociatedResource(), range, fix.id).then(result => {
fix.support.runQuickFixAction(this.editor.getModel().getAssociatedResource(), range, { command: fix.command, score: fix.score }).then(result => {
if (result) {
if (result.message) {
this.messageService.show(Severity.Info, result.message);

View file

@ -24,8 +24,10 @@ import {IQuickFix2} from '../common/quickFix';
var $ = dom.emmet;
function isQuickFix(quickfix: any) : boolean {
return quickfix && quickfix.id && quickfix.label;
function isQuickFix(quickfix: any): quickfix is IQuickFix2 {
return quickfix
&& typeof (<IQuickFix2>quickfix).command === 'object'
&& typeof (<IQuickFix2> quickfix).command.title === 'string';
}
// To be used as a tree element when we want to show a message
@ -119,9 +121,9 @@ class Controller extends TreeDefaults.DefaultController {
function getHeight(tree:Tree.ITree, element:any): number {
var fix = <IQuickFix2>element;
if (!(element instanceof Message) && !!fix.documentation && tree.isFocused(fix)) {
return 35;
}
// if (!(element instanceof Message) && !!fix.documentation && tree.isFocused(fix)) {
// return 35;
// }
return 19;
}
@ -159,8 +161,8 @@ class Renderer implements Tree.IRenderer {
}
var quickFix = <IQuickFix2> element;
templateData.main.textContent = quickFix.label;
templateData.documentationLabel.textContent = quickFix.documentation || '';
templateData.main.textContent = quickFix.command.title;
templateData.documentationLabel.textContent = /*quickFix.documentation ||*/ '';
}
public disposeTemplate(tree: Tree.ITree, templateId: string, templateData: any): void {

View file

@ -19,6 +19,7 @@ export const QuickFixRegistry = new LanguageFeatureRegistry<IQuickFixSupport>('q
export interface IQuickFix2 extends IQuickFix {
support: IQuickFixSupport;
id: string;
}
export function getQuickFixes(model: IModel, range: IRange): TPromise<IQuickFix2[]> {
@ -29,12 +30,12 @@ export function getQuickFixes(model: IModel, range: IRange): TPromise<IQuickFix2
if (!Array.isArray(result)) {
return
}
let c = 0;
for (let fix of result) {
quickFixes.push({
id: fix.id,
label: fix.label,
documentation: fix.documentation,
command: fix.command,
score: fix.score,
id: `quickfix_#${c++}`,
support
});
}

View file

@ -386,9 +386,12 @@ export class CSSWorker extends AbstractModeWorker {
var score = strings.difference(propertyName, p);
if (score >= propertyName.length / 2 /*score_lim*/) {
result.push({
label: nls.localize('css.quickfix.rename', "Rename to '{0}'", p),
id: JSON.stringify({ type: 'rename', name: p }),
score: score
command: {
id: 'css.renameProptery',
title: nls.localize('css.quickfix.rename', "Rename to '{0}'", p),
arguments: [{ type: 'rename', name: p }]
},
score
});
}
}
@ -426,12 +429,12 @@ export class CSSWorker extends AbstractModeWorker {
});
}
public runQuickFixAction(resource:URI, range:EditorCommon.IRange, id: any):winjs.TPromise<Modes.IQuickFixResult>{
var command = JSON.parse(id);
switch (command.type) {
public runQuickFixAction(resource: URI, range: EditorCommon.IRange, quickFix: Modes.IQuickFix): winjs.TPromise<Modes.IQuickFixResult>{
let [{type, name}] = quickFix.command.arguments;
switch (type) {
case 'rename': {
return winjs.TPromise.as({
edits: [{ resource, range, newText: command.name }]
edits: [{ resource, range, newText: name }]
});
}
}

View file

@ -130,7 +130,7 @@ suite('Validation - CSS', () => {
};
var assertQuickFix= function(fixes: Modes.IQuickFix[], model: mm.MirrorModel, expectedContent:string[]) {
var labels = fixes.map(f => f.label);
var labels = fixes.map(f => f.command.title);
for (var index = 0; index < expectedContent.length; index++) {
assert.ok(labels.indexOf(expectedContent[index]) !== -1, 'Quick fix not found: ' + expectedContent[index]);

View file

@ -15,7 +15,7 @@ import nls = require('vs/nls');
import arrays = require('vs/base/common/arrays');
import {IMarker} from 'vs/platform/markers/common/markers';
export function evaluate(languageService: ts.LanguageService, resource: URI, range: EditorCommon.IRange, id: any): Modes.IQuickFixResult {
export function evaluate(languageService: ts.LanguageService, resource: URI, range: EditorCommon.IRange, quickFix: Modes.IQuickFix): Modes.IQuickFixResult {
var filename = resource.toString(),
sourceFile = languageService.getSourceFile(filename),
@ -26,7 +26,7 @@ export function evaluate(languageService: ts.LanguageService, resource: URI, ran
return null;
}
var command = JSON.parse(id);
var [command] = quickFix.command.arguments;
switch (command.type) {
case 'rename': {
var start = sourceFile.getLineAndCharacterOfPosition(token.getStart());
@ -129,9 +129,12 @@ function computeRenameProposals(languageService:ts.LanguageService, resource:URI
}
fixes.push({
label: nls.localize('typescript.quickfix.rename', "Rename to '{0}'", entry.name),
id: JSON.stringify({ type: 'rename', name: entry.name }),
score: score
command: {
id: 'ts.renameTo',
title: nls.localize('typescript.quickfix.rename', "Rename to '{0}'", entry.name),
arguments: [{ type: 'rename', name: entry.name }]
},
score
});
}
});
@ -204,19 +207,25 @@ function computeAddTypeDefinitionProposals(languageService: ts.LanguageService,
if (typingsMap.hasOwnProperty(currentWord)) {
var mapping = typingsMap[currentWord];
var dtsRefs: string[] = Array.isArray(mapping) ? <string[]> mapping : [ <string> mapping ];
dtsRefs.forEach((dtsRef) => {
dtsRefs.forEach((dtsRef, idx) => {
result.push({
label: nls.localize('typescript.quickfix.typeDefinitions', "Download type definition {0}", dtsRef.split('/')[1]),
id: JSON.stringify({ type: 'typedefinitions', name: dtsRef }),
score: 1
command: {
id: 'ts.downloadDts',
title: nls.localize('typescript.quickfix.typeDefinitions', "Download type definition {0}", dtsRef.split('/')[1]),
arguments: [{ type: 'typedefinitions', name: dtsRef }]
},
score: idx
});
});
}
if (strings.endsWith(resource.path, '.js')) {
result.push({
label: nls.localize('typescript.quickfix.addAsGlobal', "Mark '{0}' as global", currentWord),
id: JSON.stringify({ type: 'addglobal', name: currentWord }),
command: {
id: 'ts.addAsGlobal',
title: nls.localize('typescript.quickfix.addAsGlobal', "Mark '{0}' as global", currentWord),
arguments: [{ type: 'addglobal', name: currentWord }]
},
score: 1
});
}

View file

@ -34,8 +34,8 @@ export class QuickFixMainActions {
this._contextService = contextService;
}
public evaluate(resource: URI, range: EditorCommon.IRange, id: any) : winjs.TPromise<Modes.IQuickFixResult> {
var command = JSON.parse(id);
public evaluate(resource: URI, range: EditorCommon.IRange, quickFix: Modes.IQuickFix) : winjs.TPromise<Modes.IQuickFixResult> {
var [command] = quickFix.command.arguments;
switch (command.type) {
case 'typedefinitions': {
return this.evaluateAddTypeDefinitionProposal(command.name, resource);

View file

@ -35,35 +35,35 @@ suite('TS - quick fix', () => {
assertQuickFix('class C { private hello = 0; private world = this.hell0; }', 'a.ts', (elements) => {
assert.equal(elements.length, 1);
assert.equal(elements[0].label, "Rename to 'hello'");
assert.equal(elements[0].command.title, "Rename to 'hello'");
});
assertQuickFix('_.foo();', 'a.ts', (elements) => {
assert.equal(elements.length, 2);
assert.equal(elements[0].label, "Download type definition underscore.d.ts");
assert.equal(elements[1].label, "Download type definition lodash.d.ts");
assert.equal(elements[0].command.title, "Download type definition underscore.d.ts");
assert.equal(elements[1].command.title, "Download type definition lodash.d.ts");
});
assertQuickFix('describe("x");', 'a.js', (elements) => {
assert.equal(elements.length, 3);
assert.equal(elements[0].label, "Download type definition mocha.d.ts");
assert.equal(elements[1].label, "Download type definition jasmine.d.ts");
assert.equal(elements[2].label, "Mark 'describe' as global");
assert.equal(elements[0].command.title, "Download type definition mocha.d.ts");
assert.equal(elements[1].command.title, "Download type definition jasmine.d.ts");
assert.equal(elements[2].command.title, "Mark 'describe' as global");
});
assertQuickFix('angular.foo = 1;', 'a.ts', (elements) => {
assert.equal(elements.length, 1);
assert.equal(elements[0].label, "Download type definition angular.d.ts");
assert.equal(elements[0].command.title, "Download type definition angular.d.ts");
});
assertQuickFix('var x = __dirname;', 'a.ts', (elements) => {
assert.equal(elements.length, 1);
assert.equal(elements[0].label, "Download type definition node.d.ts");
assert.equal(elements[0].command.title, "Download type definition node.d.ts");
});
assertQuickFix('ko.observable(null);', 'a.ts', (elements) => {
assert.equal(elements.length, 1);
assert.equal(elements[0].label, "Download type definition knockout.d.ts");
assert.equal(elements[0].command.title, "Download type definition knockout.d.ts");
});
for (var id in quickfix.typingsMap) {
@ -76,12 +76,12 @@ suite('TS - quick fix', () => {
assertQuickFix('foo.observable(null);', 'a.js', (elements) => {
assert.equal(elements.length, 1);
assert.equal(elements[0].label, "Mark 'foo' as global");
assert.equal(elements[0].command.title, "Mark 'foo' as global");
});
assertQuickFix('toString();', 'a.js', (elements) => {
assert.equal(elements.length, 1);
assert.equal(elements[0].label, "Mark 'toString' as global");
assert.equal(elements[0].command.title, "Mark 'toString' as global");
});
});

View file

@ -195,10 +195,10 @@ export class ExtHostLanguageFeatureCommands {
range: typeConverters.fromRange(range)
};
return this._commands.executeCommand<IQuickFix2[]>('_executeCodeActionProvider', args).then(value => {
if (Array.isArray(value)) {
// TODO@joh this isn't proper!
return value.map(quickFix => ({ title: quickFix.label }));
if (!Array.isArray(value)) {
return;
}
return value.map(quickFix => typeConverters.Command.to(quickFix.command));
});
}

View file

@ -284,7 +284,6 @@ class QuickFixAdapter implements modes.IQuickFixSupport {
private _documents: PluginHostModelService;
private _commands: PluginHostCommands;
private _provider: vscode.CodeActionProvider;
private _cache: { [key: string]: vscode.Command[] } = Object.create(null);
constructor(documents: PluginHostModelService, commands: PluginHostCommands, provider: vscode.CodeActionProvider) {
this._documents = documents;
@ -294,10 +293,6 @@ class QuickFixAdapter implements modes.IQuickFixSupport {
getQuickFixes(resource: URI, range: IRange, marker?: IMarker[]): TPromise<modes.IQuickFix[]> {
// return this._executeCommand(resource, range, markers);
const key = resource.toString();
delete this._cache[key];
const doc = this._documents.getDocument(resource);
const ran = TypeConverters.toRange(range);
const diagnostics = marker.map(marker => {
@ -311,32 +306,18 @@ class QuickFixAdapter implements modes.IQuickFixSupport {
if (!Array.isArray(commands)) {
return;
}
this._cache[key] = commands;
return commands.map((command, i) => {
return <modes.IQuickFix> {
id: String(i),
label: command.title,
score: 1
command: TypeConverters.Command.from(command),
score: i
};
});
});
}
runQuickFixAction(resource: URI, range: IRange, id: string): any {
let commands = this._cache[resource.toString()];
if (!commands) {
return TPromise.wrapError('no command for ' + resource.toString());
}
let command = commands[Number(id)];
if (!command) {
return TPromise.wrapError('no command for ' + resource.toString());
}
return this._commands.executeCommand(command.command, ...command.arguments);
runQuickFixAction(resource: URI, range: IRange, quickFix: modes.IQuickFix): any {
let {command} = quickFix;
return this._commands.executeCommand(command.id, ...command.arguments);
}
}
@ -755,8 +736,8 @@ export class ExtHostLanguageFeatures {
return this._withAdapter(handle, QuickFixAdapter, adapter => adapter.getQuickFixes(resource, range, marker));
}
$runQuickFixAction(handle: number, resource: URI, range: IRange, id: string): any {
return this._withAdapter(handle, QuickFixAdapter, adapter => adapter.runQuickFixAction(resource, range, id));
$runQuickFixAction(handle: number, resource: URI, range: IRange, quickFix: modes.IQuickFix): any {
return this._withAdapter(handle, QuickFixAdapter, adapter => adapter.runQuickFixAction(resource, range, quickFix));
}
// --- formatting
@ -960,8 +941,8 @@ export class MainThreadLanguageFeatures {
});
return this._proxy.$getQuickFixes(handle, resource, range, markers);
},
runQuickFixAction: (resource: URI, range: IRange, id: string) => {
return this._proxy.$runQuickFixAction(handle, resource, range, id);
runQuickFixAction: (resource: URI, range: IRange, quickFix: modes.IQuickFix) => {
return this._proxy.$runQuickFixAction(handle, resource, range, quickFix);
}
});
return undefined;

View file

@ -270,8 +270,8 @@ suite('ExtHostLanguageFeatureCommands', function() {
assert.equal(value.length, 1);
let [first] = value;
assert.equal(first.title, 'Title');
// assert.equal(first.command, 'testing');
// assert.deepEqual(first.arguments, [1, 2, true]);
assert.equal(first.command, 'testing');
assert.deepEqual(first.arguments, [1, 2, true]);
done();
});
});

View file

@ -615,8 +615,8 @@ suite('ExtHostLanguageFeatures', function() {
disposables.push(extHost.registerCodeActionProvider(defaultSelector, <vscode.CodeActionProvider>{
provideCodeActions(): any {
return [
<vscode.Command>{ command: 'test', title: 'Testing1' },
<vscode.Command>{ command: 'test', title: 'Testing2' }
<vscode.Command>{ command: 'test1', title: 'Testing1' },
<vscode.Command>{ command: 'test2', title: 'Testing2' }
];
}
}));
@ -626,10 +626,10 @@ suite('ExtHostLanguageFeatures', function() {
assert.equal(value.length, 2);
let [first, second] = value;
assert.equal(first.label, 'Testing1');
assert.equal(first.id, String(0));
assert.equal(second.label, 'Testing2');
assert.equal(second.id, String(1));
assert.equal(first.command.title, 'Testing1');
assert.equal(first.command.id, 'test1');
assert.equal(second.command.title, 'Testing2');
assert.equal(second.command.id, 'test2');
done();
});
});
@ -653,7 +653,7 @@ suite('ExtHostLanguageFeatures', function() {
assert.equal(value.length, 1);
let [entry] = value;
entry.support.runQuickFixAction(model.getAssociatedResource(), model.getFullModelRange(), entry.id).then(value => {
entry.support.runQuickFixAction(model.getAssociatedResource(), model.getFullModelRange(), entry).then(value => {
assert.equal(value, undefined);
assert.equal(actualArgs.length, 4);