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.

Sign up for GitHub

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

Jump to bottom

fix: invoke only completers who have matching trigger characters #5274

Closed
wants to merge 2 commits into from
Closed

fix: invoke only completers who have matching trigger characters #5274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c1a905
fix: invoke only completers who have matching trigger characters
InspiredGuy Aug 2, 2023
0b6c47c
fix: trigger all completers when trigger char is a word character
InspiredGuy Aug 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/autocomplete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,8 +666,14 @@ class CompletionProvider {

var matches = [];
this.completers = editor.completers;
var total = editor.completers.length;
editor.completers.forEach(function(completer, i) {
var matchingCompleters = util.getCompletersMatchingTriggerCharacter(editor);
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens here if there is a completer with the trigger character a, would only the completer with the trigger character get triggered or would all completers still get triggered (as they should given that the user is typing a normal letter)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good question. the autocomplete is triggered when there is a trigger character matching at least one completer or there is a text prefix with non-zero length. string a would fall into prefix even if it's a trigger character, but we don't check for it here. I will update implementation to use all completers if there is a prefix.

// if there are no matches or there is a prefix, and the prefix is longer than the threshold,
// then use all completers
if (matchingCompleters.length === 0 || (prefix && prefix.length >= editor.$liveAutocompletionThreshold)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If there is no completer registered for trigger char {, and I press { does this mean that all registered completers would now get triggered by this non prefix char?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, autocomplete won't be triggered at all if there is no prefix (word characters A-Za-z) or trigger char does not match any of the completers, see https://github.com/InspiredGuy/ace/blob/invoke-only-matching-completers-for-trigger-chars/src/ext/language_tools.js#L160

Copy link
Contributor

Choose a reason for hiding this comment

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

ah right, ty

matchingCompleters = editor.completers;
}
var total = matchingCompleters.length;
matchingCompleters.forEach(function(completer, i) {
completer.getCompletions(editor, session, pos, prefix, function(err, results) {
if (!err && results)
matches = matches.concat(results);
Expand Down
4 changes: 2 additions & 2 deletions src/autocomplete/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ exports.getCompletionPrefix = function (editor) {
return prefix || this.retrievePrecedingIdentifier(line, pos.column);
};

exports.triggerAutocomplete = function (editor) {
exports.getCompletersMatchingTriggerCharacter = 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) => {
return editor.completers.filter((el) => {
if (el.triggerCharacters && Array.isArray(el.triggerCharacters)) {
return el.triggerCharacters.includes(previousChar);
}
Expand Down
155 changes: 155 additions & 0 deletions src/autocomplete_test.js
View file
Open in desktop
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 @@ -330,6 +330,161 @@
});
}
},
"test: gets completions only from matching completers for trigger characters": function (done) {
var editor = initEditor("document");

var dotCompleterCalls = 0;
var twoBracketsCompleterCalls = 0;
var oneBracketCompleterCalls = 0;

editor.completers = [
{
getCompletions: function (editor, session, pos, prefix, callback) {
dotCompleterCalls++;
var completions = [
{
caption: "append",
value: "append"
}, {
caption: "all",
value: "all"
}
];
callback(null, completions);
},
triggerCharacters: ["."]
},
{
getCompletions: function (editor, session, pos, prefix, callback) {
oneBracketCompleterCalls++;
var completions = [
{
caption: "banana",
value: "banana"
}, {
caption: "apple",
value: "apple"
}, {
caption: "orange",
value: "orange"
}
];
callback(null, completions);
},
triggerCharacters: ["{"]
},
{
getCompletions: function (editor, session, pos, prefix, callback) {
twoBracketsCompleterCalls++;
var completions = [
{
caption: "fruit",
value: "fruit"
}
];
callback(null, completions);
},
triggerCharacters: ["{", "["]
}
];

editor.moveCursorTo(0, 8);
sendKey(".");
var popup = editor.completer.popup;
check(function () {
assert.equal(popup.data.length, 2);
editor.onCommandKey(null, 0, 13);
assert.equal(editor.getValue(), "document.all");
assert.equal(dotCompleterCalls, 1);
assert.equal(twoBracketsCompleterCalls, 0);
assert.equal(oneBracketCompleterCalls, 0);

editor.moveCursorTo(0, 12);
sendKey("{");
check(function () {
assert.equal(popup.data.length, 4);
editor.onCommandKey(null, 0, 13);
assert.equal(editor.getValue(), "document.all{apple");
assert.equal(dotCompleterCalls, 1);
assert.equal(twoBracketsCompleterCalls, 1);
assert.equal(oneBracketCompleterCalls, 1);

editor.moveCursorTo(0, 18);
sendKey("[");
check(function () {
assert.equal(popup.data.length, 1);
editor.onCommandKey(null, 0, 13);
assert.equal(editor.getValue(), "document.all{apple[fruit");
assert.equal(dotCompleterCalls, 1);
assert.equal(twoBracketsCompleterCalls, 2);
assert.equal(oneBracketCompleterCalls, 1);

done();
});
});
});

function check(callback) {
popup = editor.completer.popup;
popup.renderer.on("afterRender", function wait() {
popup.renderer.off("afterRender", wait);
callback();
});
}
},
"test: trigger all completers if there is a textual(A-Za-z) trigger character": function (done) {
var editor = initEditor("");
editor.setOptions({
liveAutocompletionThreshold: 0
});

editor.completers = [
{
getCompletions: function (editor, session, pos, prefix, callback) {
var completions = [
{
caption: "agent",
value: "agent"
}, {
caption: "alarm",
value: "alarm"
}
];
callback(null, completions);
},
triggerCharacters: ["a"]
},
{
getCompletions: function (editor, session, pos, prefix, callback) {
var completions = [
{
caption: "abbath",
value: "abbath"
}
];
callback(null, completions);
}
}
];

editor.moveCursorTo(0, 0);
user.type("a");
var popup = editor.completer.popup;
check(function () {
assert.equal(popup.data.length, 3);
editor.onCommandKey(null, 0, 13);
assert.equal(editor.getValue(), "abbath");
done();
});

function check(callback) {
popup = editor.completer.popup;
popup.renderer.on("afterRender", function wait() {
popup.renderer.off("afterRender", wait);
callback();
});
}
},
"test: empty message if no suggestions available": function(done) {
var editor = initEditor("");
var emptyMessageText = "No suggestions.";
Expand Down
2 changes: 1 addition & 1 deletion src/ext/language_tools.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ 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 triggerAutocomplete = util.getCompletersMatchingTriggerCharacter(editor).length > 0;
if ((prefix || triggerAutocomplete) && prefix.length >= editor.$liveAutocompletionThreshold) {
var completer = Autocomplete.for(editor);
// Set a flag for auto shown
Expand Down
Loading