Skip to content

Commit

Permalink
Fix #40941 - Suggest trigger at language boundaries
Browse files Browse the repository at this point in the history 
Fixes #40941

**Bug**
Suggestions not auto triggered when you `x` in `const z = <div x>`. See #40941 (comment) for explaination of root cause

**Proposed fix**
In the case where suggestions are triggered at the boundary between two languages, I believe we should trigger. This change adds special logic to handle that case by making sure we check the word at the  previous character position instead of at the current character positon when at language boundaries
  • Loading branch information
@mjbvz
mjbvz committed Mar 16, 2018
1 parent c55926a commit 1714875
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 19 deletions.
14 changes: 13 additions & 1 deletion src/vs/editor/contrib/suggest/suggestModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,19 @@ export class LineContext {
}
const pos = editor.getPosition();
model.tokenizeIfCheap(pos.lineNumber);
const word = model.getWordAtPosition(pos);

// When we are at an embedded language boundary, check the word at the previous character
const cursorLang = model.getLanguageIdAtPosition(pos.lineNumber, pos.column);
const preLang = model.getLanguageIdAtPosition(pos.lineNumber, pos.column - 1);
let posToCheckWordAt = pos;
if (cursorLang !== preLang) {
const postLang = model.getLanguageIdAtPosition(pos.lineNumber, pos.column + 1);
if (postLang === cursorLang) {
posToCheckWordAt = new Position(pos.lineNumber, Math.max(pos.column - 1, 1));
}
}

const word = model.getWordAtPosition(posToCheckWordAt);
if (!word) {
return false;
}
Expand Down
98 changes: 80 additions & 18 deletions src/vs/editor/contrib/suggest/test/suggestModel.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
import * as assert from 'assert';
import { Event } from 'vs/base/common/event';
import URI from 'vs/base/common/uri';
import { IDisposable, dispose } from 'vs/base/common/lifecycle';
import { IDisposable, dispose, Disposable } from 'vs/base/common/lifecycle';
import { TextModel } from 'vs/editor/common/model/textModel';
import { Handler } from 'vs/editor/common/editorCommon';
import { ISuggestSupport, ISuggestResult, SuggestRegistry, SuggestTriggerKind } from 'vs/editor/common/modes';
import { ISuggestSupport, ISuggestResult, SuggestRegistry, SuggestTriggerKind, LanguageIdentifier, TokenizationRegistry, IState, MetadataConsts } from 'vs/editor/common/modes';
import { SuggestModel, LineContext } from 'vs/editor/contrib/suggest/suggestModel';
import { TestCodeEditor, MockScopeLocation } from 'vs/editor/test/browser/testCodeEditor';
import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
Expand All @@ -27,6 +27,10 @@ import { SuggestController } from 'vs/editor/contrib/suggest/suggestController';
import { IStorageService, NullStorageService } from 'vs/platform/storage/common/storage';
import { SnippetController2 } from 'vs/editor/contrib/snippet/snippetController2';
import { ISelectedSuggestion } from 'vs/editor/contrib/suggest/suggestWidget';
import { MockMode } from 'vs/editor/test/common/mocks/mockMode';
import { LanguageConfigurationRegistry } from 'vs/editor/common/modes/languageConfigurationRegistry';
import { TokenizationResult2 } from 'vs/editor/common/core/token';
import { NULL_STATE } from 'vs/editor/common/modes/nullMode';

function createMockEditor(model: TextModel): TestCodeEditor {
const contextKeyService = new MockContextKeyService();
Expand All @@ -43,33 +47,91 @@ function createMockEditor(model: TextModel): TestCodeEditor {
}

suite('SuggestModel - Context', function () {
const OUTER_LANGUAGE_ID = new LanguageIdentifier('outerMode', 3);
const INNER_LANGUAGE_ID = new LanguageIdentifier('innerMode', 4);

class OuterMode extends MockMode {
constructor() {
super(OUTER_LANGUAGE_ID);
this._register(LanguageConfigurationRegistry.register(this.getLanguageIdentifier(), {}));

this._register(TokenizationRegistry.register(this.getLanguageIdentifier().language, {
getInitialState: (): IState => NULL_STATE,
tokenize: undefined,
tokenize2: (line: string, state: IState): TokenizationResult2 => {
const tokensArr: number[] = [];
let prevLanguageId: LanguageIdentifier = undefined;
for (let i = 0; i < line.length; i++) {
const languageId = (line.charAt(i) === 'x' ? INNER_LANGUAGE_ID : OUTER_LANGUAGE_ID);
if (prevLanguageId !== languageId) {
tokensArr.push(i);
tokensArr.push((languageId.id << MetadataConsts.LANGUAGEID_OFFSET));
}
prevLanguageId = languageId;
}

const tokens = new Uint32Array(tokensArr.length);
for (let i = 0; i < tokens.length; i++) {
tokens[i] = tokensArr[i];
}
return new TokenizationResult2(tokens, state);
}
}));
}
}

let model: TextModel;
class InnerMode extends MockMode {
constructor() {
super(INNER_LANGUAGE_ID);
this._register(LanguageConfigurationRegistry.register(this.getLanguageIdentifier(), {}));
}
}

setup(function () {
model = TextModel.createFromString('Das Pferd frisst keinen Gurkensalat - Philipp Reis 1861.\nWer hat\'s erfunden?');
const assertAutoTrigger = (model: TextModel, offset: number, expected: boolean, message?: string): void => {
const pos = model.getPositionAt(offset);
const editor = createMockEditor(model);
editor.setPosition(pos);
assert.equal(LineContext.shouldAutoTrigger(editor), expected, message);
editor.dispose();
};

let disposables: Disposable[] = [];

setup(() => {
disposables = [];
});

teardown(function () {
model.dispose();
dispose(disposables);
disposables = [];
});

test('Context - shouldAutoTrigger', function () {
const model = TextModel.createFromString('Das Pferd frisst keinen Gurkensalat - Philipp Reis 1861.\nWer hat\'s erfunden?');
disposables.push(model);

function assertAutoTrigger(offset: number, expected: boolean): void {
const pos = model.getPositionAt(offset);
const editor = createMockEditor(model);
editor.setPosition(pos);
assert.equal(LineContext.shouldAutoTrigger(editor), expected);
editor.dispose();
}

assertAutoTrigger(3, true); // end of word, Das|
assertAutoTrigger(4, false); // no word Das |
assertAutoTrigger(1, false); // middle of word D|as
assertAutoTrigger(55, false); // number, 1861|
assertAutoTrigger(model, 3, true, 'end of word, Das|');
assertAutoTrigger(model, 4, false, 'no word Das |');
assertAutoTrigger(model, 1, false, 'middle of word D|as');
assertAutoTrigger(model, 55, false, 'number, 1861|');
});

test('shouldAutoTrigger at embedded language boundaries', () => {
const outerMode = new OuterMode();
const innerMode = new InnerMode();
disposables.push(outerMode, innerMode);

const model = TextModel.createFromString('a<xx>a<x>', undefined, outerMode.getLanguageIdentifier());
disposables.push(model);

assertAutoTrigger(model, 1, true, 'a|<x — should trigger at end of word');
assertAutoTrigger(model, 2, false, 'a<|x — should NOT trigger at start of word');
assertAutoTrigger(model, 3, false, 'a<x|x — should NOT trigger in middle of word');
assertAutoTrigger(model, 4, true, 'a<xx|> — should trigger at boundary between languages');
assertAutoTrigger(model, 5, false, 'a<xx>|a — should NOT trigger at start of word');
assertAutoTrigger(model, 6, true, 'a<xx>a|< — should trigger at end of word');
assertAutoTrigger(model, 8, true, 'a<xx>a<x|> — should trigger at end of word at boundary');
});
});

suite('SuggestModel - TriggerAndCancelOracle', function () {
Expand Down

0 comments on commit 1714875

Please sign in to comment.