Skip to content

Commit f9ddacd

Browse files
committed
feat: match variables by name when copy paste
Ref #4768 Here improved copy paste experience between expressions. All expressions while editing have are no longer encoded with ids. For example `system.search.name` is the same. Though invalid js characters are encoded with code point like this `Collection Item.title` becomes `Collection$32$Item.title` when copy into textual editor. And this less obscure name can be copied between different lists with the same variable name.
1 parent bc618d4 commit f9ddacd

File tree

5 files changed

+259
-124
lines changed

5 files changed

+259
-124
lines changed

apps/builder/app/builder/features/settings-panel/variable-popover.tsx

+4-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,10 @@ const NameField = ({
9797
const variablesByName = useStore($variablesByName);
9898
const validateName = useCallback(
9999
(value: string) => {
100-
if (variablesByName.get(value) !== variableId) {
100+
if (
101+
variablesByName.has(value) &&
102+
variablesByName.get(value) !== variableId
103+
) {
101104
return "Name is already used by another variable on this instance";
102105
}
103106
if (value.trim().length === 0) {

apps/builder/app/builder/shared/expression-editor.tsx

+101-123
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import { useEffect, useMemo, type ReactNode, type RefObject } from "react";
22
import { matchSorter } from "match-sorter";
33
import type { SyntaxNode } from "@lezer/common";
4-
import { EditorState, Facet } from "@codemirror/state";
4+
import { Facet, RangeSetBuilder } from "@codemirror/state";
55
import {
66
type DecorationSet,
77
type ViewUpdate,
8-
MatchDecorator,
98
Decoration,
109
WidgetType,
1110
ViewPlugin,
@@ -28,12 +27,7 @@ import {
2827
} from "@codemirror/autocomplete";
2928
import { javascript } from "@codemirror/lang-javascript";
3029
import { textVariants, css, rawTheme } from "@webstudio-is/design-system";
31-
import {
32-
decodeDataSourceVariable,
33-
lintExpression,
34-
transpileExpression,
35-
} from "@webstudio-is/sdk";
36-
import { mapGroupBy } from "~/shared/shim";
30+
import { decodeDataVariableId, lintExpression } from "@webstudio-is/sdk";
3731
import {
3832
EditorContent,
3933
EditorDialog,
@@ -42,6 +36,12 @@ import {
4236
foldGutterExtension,
4337
type EditorApi,
4438
} from "./code-editor-base";
39+
import {
40+
decodeDataVariableName,
41+
encodeDataVariableName,
42+
restoreExpressionVariables,
43+
unsetExpressionVariables,
44+
} from "~/shared/data-variables";
4545

4646
export type { EditorApi };
4747

@@ -178,7 +178,7 @@ const completionPath = (
178178
// object (for example `globalThis`). Will enter properties
179179
// of the object when completing properties on a directly-named path.
180180
const scopeCompletionSource: CompletionSource = (context) => {
181-
const [{ scope, aliases }] = context.state.facet(VariablesData);
181+
const [{ scope }] = context.state.facet(VariablesData);
182182
const path = completionPath(context);
183183
if (path === undefined) {
184184
return null;
@@ -195,7 +195,7 @@ const scopeCompletionSource: CompletionSource = (context) => {
195195
if (typeof target === "object" && target !== null) {
196196
options = Object.entries(target).map(([name, value]) => ({
197197
label: name,
198-
displayLabel: aliases.get(name),
198+
displayLabel: decodeDataVariableName(name),
199199
detail: formatValuePreview(value),
200200
apply: (view, completion, from, to) => {
201201
// complete valid js identifier or top level variable without quotes
@@ -266,50 +266,48 @@ class VariableWidget extends WidgetType {
266266
}
267267
}
268268

269-
const variableMatcher = new MatchDecorator({
270-
regexp: /(\$ws\$dataSource\$\w+)/g,
271-
272-
decorate: (add, from, _to, match, view) => {
273-
const name = match[1];
274-
const [{ aliases }] = view.state.facet(VariablesData);
275-
276-
// The regexp may match variables not in scope, but the key problem we're solving is this:
277-
// We have an alias $ws$dataSource$321 -> SomeVar, which we display as '[SomeVar]' ([] means decoration in the editor).
278-
// If the user types a symbol (e.g., 'a') immediately after '[SomeVar]',
279-
// the raw text becomes $ws$dataSource$321a, but we want to display '[SomeVar]a'.
280-
const dataSourceId = [...aliases.keys()].find((key) => name.includes(key));
281-
282-
if (dataSourceId === undefined) {
283-
return;
284-
}
285-
286-
const endPos = from + dataSourceId.length;
287-
288-
add(
289-
from,
290-
endPos,
291-
Decoration.replace({
292-
widget: new VariableWidget(aliases.get(dataSourceId)!),
293-
})
294-
);
295-
},
296-
});
269+
const getVariableDecorations = (view: EditorView) => {
270+
const builder = new RangeSetBuilder<Decoration>();
271+
syntaxTree(view.state).iterate({
272+
from: 0,
273+
to: view.state.doc.length,
274+
enter: (node) => {
275+
if (node.name == "VariableName") {
276+
const [{ scope }] = view.state.facet(VariablesData);
277+
const identifier = view.state.doc.sliceString(node.from, node.to);
278+
const variableName = decodeDataVariableName(identifier);
279+
if (identifier in scope) {
280+
builder.add(
281+
node.from,
282+
node.to,
283+
Decoration.replace({
284+
widget: new VariableWidget(variableName!),
285+
})
286+
);
287+
}
288+
}
289+
},
290+
});
291+
return builder.finish();
292+
};
297293

298-
const variables = ViewPlugin.fromClass(
294+
const variablesPlugin = ViewPlugin.fromClass(
299295
class {
300-
variables: DecorationSet;
296+
decorations: DecorationSet;
301297
constructor(view: EditorView) {
302-
this.variables = variableMatcher.createDeco(view);
298+
this.decorations = getVariableDecorations(view);
303299
}
304300
update(update: ViewUpdate) {
305-
this.variables = variableMatcher.updateDeco(update, this.variables);
301+
if (update.docChanged) {
302+
this.decorations = getVariableDecorations(update.view);
303+
}
306304
}
307305
},
308306
{
309-
decorations: (instance) => instance.variables,
307+
decorations: (instance) => instance.decorations,
310308
provide: (plugin) =>
311309
EditorView.atomicRanges.of((view) => {
312-
return view.plugin(plugin)?.variables || Decoration.none;
310+
return view.plugin(plugin)?.decorations || Decoration.none;
313311
}),
314312
}
315313
);
@@ -323,76 +321,6 @@ const wrapperStyle = css({
323321
"--ws-code-editor-max-height": "320px",
324322
});
325323

326-
/**
327-
* Replaces variables with their IDs, e.g., someVar -> $ws$dataSource$321
328-
*/
329-
const replaceWithWsVariables = EditorState.transactionFilter.of((tr) => {
330-
if (!tr.docChanged) {
331-
return tr;
332-
}
333-
334-
const state = tr.startState;
335-
const [{ aliases }] = state.facet(VariablesData);
336-
337-
const aliasesByName = mapGroupBy(Array.from(aliases), ([_id, name]) => name);
338-
339-
// The idea of cursor preservation is simple:
340-
// There are 2 cases we are handling:
341-
// 1. A variable is replaced while typing its name. In this case, we preserve the cursor position from the end of the text.
342-
// 2. A variable is replaced when an operation makes the expression valid. For example, ('' b) -> ('' + b).
343-
// In this case, we preserve the cursor position from the start of the text.
344-
// This does not cover cases like (a b) -> (a + b). We are not handling it because I haven't found a way to enter such a case into real input.
345-
// We can improve it if issues arise.
346-
347-
const cursorPos = tr.selection?.main.head ?? 0;
348-
const cursorPosFromEnd = tr.newDoc.length - cursorPos;
349-
350-
const content = tr.newDoc.toString();
351-
const originalContent = tr.startState.doc.toString();
352-
353-
let updatedContent = content;
354-
355-
try {
356-
updatedContent = transpileExpression({
357-
expression: content,
358-
replaceVariable: (identifier) => {
359-
if (decodeDataSourceVariable(identifier) && aliases.has(identifier)) {
360-
return;
361-
}
362-
// prevent matching variables by unambiguous name
363-
const matchedAliases = aliasesByName.get(identifier);
364-
if (matchedAliases && matchedAliases.length === 1) {
365-
const [id, _name] = matchedAliases[0];
366-
367-
return id;
368-
}
369-
},
370-
});
371-
} catch {
372-
// empty block
373-
}
374-
375-
if (updatedContent !== content) {
376-
return [
377-
{
378-
changes: {
379-
from: 0,
380-
to: originalContent.length,
381-
insert: updatedContent,
382-
},
383-
selection: {
384-
anchor:
385-
updatedContent.slice(0, cursorPos) === content.slice(0, cursorPos)
386-
? cursorPos
387-
: updatedContent.length - cursorPosFromEnd,
388-
},
389-
},
390-
];
391-
}
392-
393-
return tr;
394-
});
395-
396324
const linterTooltipTheme = EditorView.theme({
397325
".cm-tooltip:has(.cm-tooltip-lint)": {
398326
backgroundColor: "transparent",
@@ -416,10 +344,10 @@ const linterTooltipTheme = EditorView.theme({
416344
});
417345

418346
const expressionLinter = linter((view) => {
419-
const [{ aliases }] = view.state.facet(VariablesData);
347+
const [{ scope }] = view.state.facet(VariablesData);
420348
return lintExpression({
421349
expression: view.state.doc.toString(),
422-
availableVariables: new Set(aliases.keys()),
350+
availableVariables: new Set(Object.keys(scope)),
423351
});
424352
});
425353

@@ -450,26 +378,64 @@ export const ExpressionEditor = ({
450378
onChange: (value: string) => void;
451379
onChangeComplete: (value: string) => void;
452380
}) => {
381+
const { nameById, idByName } = useMemo(() => {
382+
const nameById = new Map();
383+
const idByName = new Map();
384+
for (const [identifier, name] of aliases) {
385+
const id = decodeDataVariableId(identifier);
386+
if (id) {
387+
nameById.set(id, name);
388+
idByName.set(name, id);
389+
}
390+
}
391+
return { nameById, idByName };
392+
}, [aliases]);
393+
const expressionWithUnsetVariables = useMemo(() => {
394+
return unsetExpressionVariables({
395+
expression: value,
396+
unsetNameById: nameById,
397+
});
398+
}, [value, nameById]);
399+
const scopeWithUnsetVariables = useMemo(() => {
400+
const newScope: typeof scope = {};
401+
for (const [identifier, value] of Object.entries(scope)) {
402+
const name = aliases.get(identifier);
403+
if (name) {
404+
newScope[encodeDataVariableName(name)] = value;
405+
}
406+
}
407+
return newScope;
408+
}, [scope, aliases]);
409+
const aliasesWithUnsetVariables = useMemo(() => {
410+
const newAliases: typeof aliases = new Map();
411+
for (const [_identifier, name] of aliases) {
412+
newAliases.set(encodeDataVariableName(name), name);
413+
}
414+
return newAliases;
415+
}, []);
416+
453417
const extensions = useMemo(
454418
() => [
455419
bracketMatching(),
456420
closeBrackets(),
457421
javascript({}),
458-
VariablesData.of({ scope, aliases }),
459-
replaceWithWsVariables,
422+
VariablesData.of({
423+
scope: scopeWithUnsetVariables,
424+
aliases: aliasesWithUnsetVariables,
425+
}),
460426
// render autocomplete in body
461427
// to prevent popover scroll overflow
462428
tooltips({ parent: document.body }),
463429
autocompletion({
464430
override: [scopeCompletionSource],
465431
icons: false,
466432
}),
467-
variables,
433+
variablesPlugin,
468434
keymap.of([...closeBracketsKeymap, ...completionKeymap]),
469435
expressionLinter,
470436
linterTooltipTheme,
471437
],
472-
[scope, aliases]
438+
[scopeWithUnsetVariables, aliasesWithUnsetVariables]
473439
);
474440

475441
// prevent clicking on autocomplete options propagating to body
@@ -497,9 +463,21 @@ export const ExpressionEditor = ({
497463
invalid={color === "error"}
498464
readOnly={readOnly}
499465
autoFocus={autoFocus}
500-
value={value}
501-
onChange={onChange}
502-
onChangeComplete={onChangeComplete}
466+
value={expressionWithUnsetVariables}
467+
onChange={(newValue) => {
468+
const expressionWithRestoredVariables = restoreExpressionVariables({
469+
expression: newValue,
470+
maskedIdByName: idByName,
471+
});
472+
onChange(expressionWithRestoredVariables);
473+
}}
474+
onChangeComplete={(newValue) => {
475+
const expressionWithRestoredVariables = restoreExpressionVariables({
476+
expression: newValue,
477+
maskedIdByName: idByName,
478+
});
479+
onChangeComplete(expressionWithRestoredVariables);
480+
}}
503481
/>
504482
);
505483

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import { expect, test } from "vitest";
2+
import {
3+
decodeDataVariableName,
4+
encodeDataVariableName,
5+
restoreExpressionVariables,
6+
unsetExpressionVariables,
7+
} from "./data-variables";
8+
9+
test("encode data variable name when necessary", () => {
10+
expect(encodeDataVariableName("formState")).toEqual("formState");
11+
expect(encodeDataVariableName("Collection Item")).toEqual(
12+
"Collection$32$Item"
13+
);
14+
expect(encodeDataVariableName("$my$Variable")).toEqual("$36$my$36$Variable");
15+
});
16+
17+
test("dencode data variable name", () => {
18+
expect(decodeDataVariableName(encodeDataVariableName("formState"))).toEqual(
19+
"formState"
20+
);
21+
expect(
22+
decodeDataVariableName(encodeDataVariableName("Collection Item"))
23+
).toEqual("Collection Item");
24+
});
25+
26+
test("dencode data variable name with dollar sign", () => {
27+
expect(
28+
decodeDataVariableName(encodeDataVariableName("$my$Variable"))
29+
).toEqual("$my$Variable");
30+
expect(decodeDataVariableName("$my$Variable")).toEqual("$my$Variable");
31+
});
32+
33+
test("unset expression variables", () => {
34+
expect(
35+
unsetExpressionVariables({
36+
expression: `$ws$dataSource$myId + arbitaryVariable`,
37+
unsetNameById: new Map([["myId", "My Variable"]]),
38+
})
39+
).toEqual("My$32$Variable + arbitaryVariable");
40+
});
41+
42+
test("ignore not existing variables in expressions", () => {
43+
expect(
44+
unsetExpressionVariables({
45+
expression: `$ws$dataSource$myId + arbitaryVariable`,
46+
unsetNameById: new Map(),
47+
})
48+
).toEqual("$ws$dataSource$myId + arbitaryVariable");
49+
});
50+
51+
test("restore expression variables", () => {
52+
expect(
53+
restoreExpressionVariables({
54+
expression: `My$32$Variable + missingVariable`,
55+
maskedIdByName: new Map([["My Variable", "myId"]]),
56+
})
57+
).toEqual("$ws$dataSource$myId + missingVariable");
58+
});

0 commit comments

Comments
 (0)