Skip to content

Commit

Permalink
Fix autocomplete prefix for custom identifierRegexes
Browse files Browse the repository at this point in the history
Hi, Ace team! My team has been using a fork of Ace, and I'm hoping to
merge some of our changes upstream so that we can return to using your
official releases.

The engineer who originally accomplished this work has long since
departed my team, so forgive me for this somewhat rote transcription of
his working notes.

> This was sent upstream in #2352,
> but closed because fcebd0f
> seemed to address the same issue.  That fix unfortunately does not
> work as we need, and fails the behavior described in our test named
> `test leading @ not duplicated on autocomplete`
>
> The root cause: Autocomplete prefix can be wrong for completions with
> a custom identifierRegex because `Autocompleter.base` is computed
> relative to a prefix computed for the default identifierRegex.

Below, I reproduce the test my colleague referred to.

```
'test leading @ not duplicated on autocomplete': function (test) {
    editor.setValue('');
    editor.navigateFileStart();

    var text = 'view: users { derived_table: { sql: @{;; } }';
    exec('insertstring', 1, text);
    editor.moveCursorTo(0, 38);
    editor.getValue();

    attachedSymbolsToSession(text, editor.session, { const: 'value' });
    editor.execCommand('startAutocomplete');

    var completion = editor.completer.completions.filtered.filter(function(completion) { return completion.caption === '@{const}'; });
    test.ok(completion[0]);

    editor.completer.insertMatch(completion[0]);
    test.equal(editor.getValue(), 'view: users { derived_table: { sql: @{const};; } }');

    test.done();
},
```
  • Loading branch information
AprilArcus committed Sep 6, 2022
1 parent ff3dd69 commit fc7659b
Showing 1 changed file with 13 additions and 3 deletions.
16 changes: 13 additions & 3 deletions src/autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,15 @@ var Autocomplete = function() {
"PageDown": function(editor) { editor.completer.popup.gotoPageDown(); }
};

this.getPrefix = function(session, pos, prefixRegex) {
var line = session.getLine(pos.row);
var prefix = util.retrievePrecedingIdentifier(line, pos.column, prefixRegex);

this.base = session.doc.createAnchor(pos.row, pos.column - prefix.length);
this.base.$insertRight = true;
return prefix;
}

this.gatherCompletions = function(editor, callback) {
var session = editor.getSession();
var pos = editor.getCursorPosition();
Expand All @@ -206,13 +215,14 @@ var Autocomplete = function() {
if (!err && results)
matches = matches.concat(results);
// Fetch prefix again, because they may have changed by now
var prefix = this.getPrefix(session, pos, results[0] && results[0].identifierRegex);
callback(null, {
prefix: util.getCompletionPrefix(editor),
prefix: prefix,
matches: matches,
finished: (--total === 0)
});
});
});
}.bind(this));
}, this);
return true;
};

Expand Down

0 comments on commit fc7659b

Please sign in to comment.