Skip to content
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

fix: allow completers to trigger on newline characters #5519

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions ace.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,7 @@ export namespace Ace {
documentToScreenColumn(row: number, docColumn: number): number;
documentToScreenRow(docRow: number, docColumn: number): number;
getScreenLength(): number;
getPrecedingCharacter(): string;
toJSON(): Object;
destroy(): void;
}
Expand Down
24 changes: 16 additions & 8 deletions src/autocomplete/util.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
"use strict";

/**
* @typedef {import("../editor").Editor} Editor
*/

exports.parForEach = function(array, fn, callback) {
var completed = 0;
var arLength = array.length;
Expand Down Expand Up @@ -59,14 +63,18 @@
return prefix || this.retrievePrecedingIdentifier(line, pos.column);
};

exports.triggerAutocomplete = function (editor) {
var pos = editor.getCursorPosition();
var line = editor.session.getLine(pos.row);
var column = (pos.column === 0) ? 0 : pos.column - 1;
var previousChar = line[column];
return editor.completers.some((el) => {
if (el.triggerCharacters && Array.isArray(el.triggerCharacters)) {
return el.triggerCharacters.includes(previousChar);
/**
* @param {Editor} editor
* @param {string} [previousChar] if not provided, it falls back to the preceding character in the editor
* @returns {boolean} whether autocomplete should be triggered
*/
exports.triggerAutocomplete = function (editor, previousChar) {
var previousChar = previousChar == null
? editor.session.getPrecedingCharacter()

Check warning on line 73 in src/autocomplete/util.js

View check run for this annotation

Codecov / codecov/patch

src/autocomplete/util.js#L73

Added line #L73 was not covered by tests
: previousChar;
return editor.completers.some((completer) => {
if (completer.triggerCharacters && Array.isArray(completer.triggerCharacters)) {
return completer.triggerCharacters.includes(previousChar);
}
});
};
20 changes: 15 additions & 5 deletions src/autocomplete_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"use strict";

var sendKey = require("./test/user").type;
var {buildDom} = require("./lib/dom");

Check warning on line 8 in src/autocomplete_test.js

View workflow job for this annotation

GitHub Actions / build (16.x)

'buildDom' is assigned a value but never used
var ace = require("./ace");
var assert = require("./test/assertions");
var user = require("./test/user");
Expand Down Expand Up @@ -403,6 +403,7 @@
},
"test: trigger autocomplete for specific characters": function (done) {
var editor = initEditor("document");
var newLineCharacter = editor.session.doc.getNewLineCharacter();

editor.completers = [
{
Expand All @@ -418,18 +419,27 @@
];
callback(null, completions);
},
triggerCharacters: ["."]
triggerCharacters: [".", newLineCharacter]
}
];

editor.moveCursorTo(0, 8);
sendKey(".");
user.type(".");
var popup = editor.completer.popup;
check(function () {
assert.equal(popup.data.length, 2);
editor.onCommandKey(null, 0, 13);
user.type("Return"); // Accept suggestion
assert.equal(editor.getValue(), "document.all");
done();

user.type(Array(4).fill("Backspace")); // Delete '.all'
user.type("Return"); // Enter new line

check(function() {
assert.equal(popup.data.length, 2);
user.type("Return"); // Accept suggestion
assert.equal(editor.getValue(), `document${newLineCharacter}all`);
done();
});
});

function check(callback) {
Expand Down
14 changes: 14 additions & 0 deletions src/edit_session.js
Original file line number Diff line number Diff line change
Expand Up @@ -2366,6 +2366,20 @@
};
}

/**
* @returns {string} the last character preceding the cursor in the editor
*/
getPrecedingCharacter() {
var pos = this.selection.getCursor();

Check warning on line 2373 in src/edit_session.js

View check run for this annotation

Codecov / codecov/patch

src/edit_session.js#L2372-L2373

Added lines #L2372 - L2373 were not covered by tests

if (pos.column === 0) {
return pos.row === 0 ? "" : this.doc.getNewLineCharacter();

Check warning on line 2376 in src/edit_session.js

View check run for this annotation

Codecov / codecov/patch

src/edit_session.js#L2375-L2376

Added lines #L2375 - L2376 were not covered by tests
akoreman marked this conversation as resolved.
Show resolved Hide resolved
}

var currentLine = this.getLine(pos.row);
return currentLine[pos.column - 1];

Check warning on line 2380 in src/edit_session.js

View check run for this annotation

Codecov / codecov/patch

src/edit_session.js#L2379-L2380

Added lines #L2379 - L2380 were not covered by tests
}

destroy() {
if (!this.destroyed) {
this.bgTokenizer.setDocument(null);
Expand Down
3 changes: 2 additions & 1 deletion src/ext/language_tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ var showLiveAutocomplete = function(e) {
var editor = e.editor;
var prefix = util.getCompletionPrefix(editor);
// Only autocomplete if there's a prefix that can be matched or previous char is trigger character
var triggerAutocomplete = util.triggerAutocomplete(editor);
var previousChar = e.args;
var triggerAutocomplete = util.triggerAutocomplete(editor, previousChar);
if (prefix && prefix.length >= editor.$liveAutocompletionThreshold || triggerAutocomplete) {
var completer = Autocomplete.for(editor);
// Set a flag for auto shown
Expand Down