(core) Show example values in formula autocomplete

Summary:
This diff adds a preview of the value of certain autocomplete suggestions, especially of the form `$foo.bar` or `user.email`. The main initial motivation was to show the difference between `$Ref` and `$Ref.DisplayCol`, but the feature is more general.

The client now sends the row ID of the row being edited (along with the table and column IDs which were already sent) to the server to fetch autocomplete suggestions. The returned suggestions are now tuples `(suggestion, example_value)` where `example_value` is a string or null. The example value is simply obtained by evaluating (in a controlled way) the suggestion in the context of the given record and the current user. The string representation is similar to the standard `repr` but dates and datetimes are formatted, and the whole thing is truncated for efficiency.

The example values are shown in the autocomplete popup separated from the actual suggestion by a number of spaces calculated to:

1. Clearly separate the suggestion from the values
2. Left-align the example values in most cases
3. Avoid having so much space such that connecting suggestions and values becomes visually difficult.

The tokenization of the row is then tweaked to show the example in light grey to deemphasise it.

Main discussion where the above was decided: https://grist.slack.com/archives/CDHABLZJT/p1661795588100009

The diff also includes various other small improvements and fixes:

- The autocomplete popup is much wider to make room for long suggestions, particularly lookups, as pointed out in https://phab.getgrist.com/D3580#inline-41007. The wide popup is the reason a fancy solution was needed to position the example values. I didn't see a way to dynamically resize the popup based on suggestions, and it didn't seem like a good idea to try.
- The `grist` and `python` labels previously shown on the right are removed. They were not helpful (https://grist.slack.com/archives/CDHABLZJT/p1659697086155179) and would get in the way of the example values.
- Fixed a bug in our custom tokenization that caused function arguments to be weirdly truncated in the middle: https://grist.slack.com/archives/CDHABLZJT/p1661956353699169?thread_ts=1661953258.342739&cid=CDHABLZJT and https://grist.slack.com/archives/C069RUP71/p1659696778991339
- Hide suggestions involving helper columns like `$gristHelper_Display` or `Table.lookupRecords(gristHelper_Display=` (https://grist.slack.com/archives/CDHABLZJT/p1661953258342739). The former has been around for a while and seems to be a mistake. The fix is simply to use `is_visible_column` instead of `is_user_column`. Since the latter is not used anywhere else, and using it in the first place seems like a mistake more than anything else, I've also removed the function to prevent similar mistakes in the future.
- Don't suggest private columns as lookup arguments: https://grist.slack.com/archives/CDHABLZJT/p1662133416652499?thread_ts=1661795588.100009&cid=CDHABLZJT
- Only fetch fresh suggestions specifically after typing `lookupRecords(` or `lookupOne(` rather than just `(`, as this would needlessly hide function suggestions which could still be useful to see the arguments. However this only makes a difference when there are still multiple matching suggestions, otherwise Ace hides them anyway.

Test Plan: Extended and updated several Python and browser tests.

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D3611
This commit is contained in:
Alex Hall
2022-09-28 16:47:55 +02:00
parent 1864b7ba5d
commit 792565976a
13 changed files with 544 additions and 157 deletions

View File

@@ -8,6 +8,10 @@
cursor: pointer;
}
.ace_grist_example {
color: #8f8f8f;
}
.ace_editor.ace_autocomplete .ace_completion-highlight.ace_grist_link {
color: var(--grist-color-dark-green);
}
@@ -16,3 +20,8 @@
z-index: 7;
pointer-events: auto;
}
.ace_editor.ace_autocomplete {
width: 500px !important; /* the default in language_tools.js is 280px */
max-width: 80%; /* of the screen, for hypothetical mobile support */
}

View File

@@ -189,9 +189,11 @@ AceEditor.prototype._setup = function() {
this.editor = this.autoDisposeWith('destroy', ace.edit(this.editorDom));
if (this.gristDoc && this.column) {
const getSuggestions = (prefix) => {
const tableId = this.gristDoc.viewModel.activeSection().table().tableId();
const section = this.gristDoc.viewModel.activeSection();
const tableId = section.table().tableId();
const columnId = this.column.colId();
return this.gristDoc.docComm.autocomplete(prefix, tableId, columnId);
const rowId = section.activeRowId();
return this.gristDoc.docComm.autocomplete(prefix, tableId, columnId, rowId);
};
setupAceEditorCompletions(this.editor, {getSuggestions});
}

View File

@@ -1,14 +1,8 @@
import {ISuggestionWithValue} from 'app/common/ActiveDocAPI';
import * as ace from 'brace';
// Suggestion may be a string, or a tuple [funcname, argSpec, isGrist], where:
// - funcname (e.g. "DATEADD") will be auto-completed with "(", AND linked to Grist
// documentation.
// - argSpec (e.g. "(start_date, days=0, ...)") is to be shown as autocomplete caption.
// - isGrist determines whether to tag this suggestion as "grist" or "python".
export type ISuggestion = string | [string, string, boolean];
export interface ICompletionOptions {
getSuggestions(prefix: string): Promise<ISuggestion[]>;
getSuggestions(prefix: string): Promise<ISuggestionWithValue[]>;
}
const completionOptions = new WeakMap<ace.Editor, ICompletionOptions>();
@@ -27,18 +21,29 @@ export function setupAceEditorCompletions(editor: ace.Editor, options: ICompleti
completer.autoSelect = false;
(editor as any).completer = completer;
// Patch updateCompletions and insertMatch so that fresh completions are fetched when the user types '.' or '('
// Used in the patches below. Returns true if the client should fetch fresh completions from the server,
// as it may have new suggestions that aren't currently shown.
completer._gristShouldRefreshCompletions = function(this: any, start: any) {
// These two lines are based on updateCompletions() in the ace autocomplete source code.
const end = this.editor.getCursorPosition();
const prefix: string = this.editor.session.getTextRange({start, end}).toLowerCase();
return (
prefix.endsWith(".") || // to get fresh attributes of references
prefix.endsWith(".lookupone(") || // to get initial argument suggestions
prefix.endsWith(".lookuprecords(")
);
}.bind(completer);
// Patch updateCompletions and insertMatch so that fresh completions are fetched when appropriate.
const originalUpdate = completer.updateCompletions.bind(completer);
completer.updateCompletions = function(this: any, keepPopupPosition: boolean) {
// The next three lines are copied from updateCompletions() in the ace autocomplete source code.
// This next line is copied from updateCompletions() in the ace autocomplete source code.
if (keepPopupPosition && this.base && this.completions) {
const pos = this.editor.getCursorPosition();
const prefix = this.editor.session.getTextRange({start: this.base, end: pos});
// If the cursor is just after '.' or '(', prevent this same block from running
// When we need fresh completions, prevent this same block from running
// in the original updateCompletions() function. Otherwise it will just keep any remaining completions that match,
// or not show any completions at all.
// But the last character implies that the set of completions is likely to have changed.
if (prefix.endsWith(".") || prefix.endsWith("(")) {
if (this._gristShouldRefreshCompletions(this.base)) {
this.completions = null;
}
}
@@ -50,12 +55,7 @@ export function setupAceEditorCompletions(editor: ace.Editor, options: ICompleti
completer.insertMatch = function(this: any) {
const base = this.base; // this.base may become null after the next line, save it now.
const result = originalInsertMatch.apply(...arguments);
// Like in the above patch, get the current text in the editor to be completed.
const pos = this.editor.getCursorPosition();
const prefix = this.editor.session.getTextRange({start: base, end: pos});
// This patch is specifically for when a previous completion is inserted by pressing Enter/Tab,
// and such completions may end in '(', which can lead to more completions, e.g. for `.lookupRecords(`.
if (prefix.endsWith("(")) {
if (this._gristShouldRefreshCompletions(base)) {
this.showPopup(this.editor);
}
return result;
@@ -100,19 +100,73 @@ function initCustomCompleter() {
const suggestions = await options.getSuggestions(prefix);
// ACE autocompletions are very poorly documented. This is somewhat helpful:
// https://prog.world/implementing-code-completion-in-ace-editor/
callback(null, suggestions.map(suggestion => {
const completions: AceSuggestion[] = suggestions.map(suggestionWithValue => {
const [suggestion, example] = suggestionWithValue;
if (Array.isArray(suggestion)) {
const [funcname, argSpec, isGrist] = suggestion;
const meta = isGrist ? 'grist' : 'python';
return {value: funcname + '(', caption: funcname + argSpec, score: 1, meta, funcname};
const [funcname, argSpec] = suggestion;
return {
value: funcname + '(',
caption: funcname + argSpec,
score: 1,
example,
funcname,
};
} else {
return {value: suggestion, score: 1, meta: "python"};
return {
value: suggestion,
caption: suggestion,
score: 1,
example,
funcname: '',
};
}
}));
});
// For suggestions with example values, calculate the 'shared padding', i.e.
// the minimum width in characters that all suggestions should fill
// (before adding 'base padding') so that the examples are aligned.
const captionLengths = completions.filter(c => c.example).map(c => c.caption.length);
const sharedPadding = Math.min(
Math.min(...captionLengths) + MAX_RELATIVE_SHARED_PADDING,
Math.max(...captionLengths),
MAX_ABSOLUTE_SHARED_PADDING,
);
// Add the padding spaces and example values to the captions.
for (const c of completions) {
if (!c.example) { continue; }
const numSpaces = Math.max(0, sharedPadding - c.caption.length) + BASE_PADDING;
c.caption = c.caption + ' '.repeat(numSpaces) + c.example;
}
callback(null, completions);
},
});
}
// Regardless of other suggestions, always add this many spaces between the caption and the example.
const BASE_PADDING = 8;
// In addition to the base padding, there's shared padding, which is the minimum number of spaces
// that all suggestions should fill so that the examples are aligned.
// However, one extremely long suggestion shouldn't result in huge padding for all suggestions.
// To mitigate this, there are two limits on the shared padding.
// The first limit is relative to the shortest caption in the suggestions.
// So if all the suggestions are similarly long, there will still be some shared padding.
const MAX_RELATIVE_SHARED_PADDING = 15;
// The second limit is absolute, so that even if all suggestions are long, we don't run out of popup space.
const MAX_ABSOLUTE_SHARED_PADDING = 40;
// Suggestion objects that are passed to ace.
interface AceSuggestion {
value: string; // the actual value inserted by the autocomplete
caption: string; // the value displayed in the popup
score: number;
// Custom attributes used only by us
example: string | null; // example value of the suggestion to show on the right
funcname: string; // name of a function to link to in documentation
}
/**
* When autocompleting a known function (with funcname received from the server call), turn the
* function name into a link to Grist documentation.
@@ -159,8 +213,8 @@ interface TokenInfo extends ace.TokenInfo {
type: string;
}
function retokenizeAceCompleterRow(rowData: any, tokens: TokenInfo[]): TokenInfo[] {
if (!rowData.funcname) {
function retokenizeAceCompleterRow(rowData: AceSuggestion, tokens: TokenInfo[]): TokenInfo[] {
if (!(rowData.funcname || rowData.example)) {
// Not a special completion, pass through the result of ACE's original tokenizing.
return tokens;
}
@@ -186,23 +240,50 @@ function retokenizeAceCompleterRow(rowData: any, tokens: TokenInfo[]): TokenInfo
rowData.funcname.slice(linkStart, linkEnd).toLowerCase();
newTokens.push({value: href, type: 'grist_link_hidden'});
// Find where the example value (if any) starts, so that it can be shown in grey.
let exampleStart: number | undefined;
if (rowData.example) {
if (!rowData.caption.endsWith(rowData.example)) {
// Just being cautious, this shouldn't happen.
console.warn(`Example "${rowData.example}" does not match caption "${rowData.caption}"`);
} else {
exampleStart = rowData.caption.length - rowData.example.length;
}
}
// Go through tokens, splitting them if needed, and modifying those that form the link part.
let position = 0;
for (const t of tokens) {
// lStart/lEnd are indices of the link within the token, possibly negative.
const lStart = linkStart - position, lEnd = linkEnd - position;
if (lStart > 0) {
const beforeLink = t.value.slice(0, lStart);
newTokens.push({value: beforeLink, type: t.type});
}
if (lEnd > 0) {
const inLink = t.value.slice(Math.max(0, lStart), lEnd);
const newType = t.type + (t.type ? '.' : '') + 'grist_link';
newTokens.push({value: inLink, type: newType});
}
if (lEnd < t.value.length) {
const afterLink = t.value.slice(lEnd);
newTokens.push({value: afterLink, type: t.type});
if (exampleStart && position + t.value.length > exampleStart) {
// Ensure that all text after `exampleStart` has the type 'grist_example'.
// Don't combine that type with the existing type, because ace highlights weirdly sometimes
// and it's best to just override that.
const end = exampleStart - position;
if (end > 0) {
newTokens.push({value: t.value.slice(0, end), type: t.type});
newTokens.push({value: t.value.slice(end), type: 'grist_example'});
} else {
newTokens.push({value: t.value, type: 'grist_example'});
}
} else {
// Handle links to documentation.
// lStart/lEnd are indices of the link within the token, possibly negative.
const lStart = linkStart - position, lEnd = linkEnd - position;
if (lStart > 0) {
const beforeLink = t.value.slice(0, lStart);
newTokens.push({value: beforeLink, type: t.type});
}
if (lEnd > 0) {
const inLink = t.value.slice(Math.max(0, lStart), lEnd);
const newType = t.type + (t.type ? '.' : '') + 'grist_link';
newTokens.push({value: inLink, type: newType});
if (lEnd < t.value.length) {
const afterLink = t.value.slice(lEnd);
newTokens.push({value: afterLink, type: t.type});
}
} else {
newTokens.push(t);
}
}
position += t.value.length;
}