Skip to content

Commit

Permalink
Fixwd #455. Replaced Binds with Input's in Evaluate to prevent …
Browse files Browse the repository at this point in the history
…invalid bind metadata in evaluations.
  • Loading branch information
amyjko committed Jun 30, 2024
1 parent b01ee00 commit 30b34e6
Show file tree
Hide file tree
Showing 36 changed files with 398 additions and 163 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Dates are in `YYYY-MM-DD` format and versions are in [semantic versioning](http:
- [#504](https://github.com/wordplaydev/wordplay/issues/504). Account for non-fixed-width characters in caret positioning.
- [#488](https://github.com/wordplaydev/wordplay/issues/488). Added animations off indicator on stage.
- [#500](https://github.com/wordplaydev/wordplay/issues/500). Improved explanation when there's a space between an evaluation's name and inputs.
- [#455](https://github.com/wordplaydev/wordplay/issues/455). Replaced `Bind`s with `Input`'s in `Evaluate` to prevent invalid bind metadata in evaluations.

### Maintenance

Expand Down
7 changes: 5 additions & 2 deletions src/basis/Basis.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import Project from '../models/Project';
import Example from '../nodes/Example';
import { Basis } from './Basis';
import DefaultLocale, { DefaultLocales } from '../locale/DefaultLocale';
import Templates from '@concepts/Templates';

const basis = Basis.getLocalizedBasis(DefaultLocales);

Expand Down Expand Up @@ -43,7 +44,9 @@ function checkBasisNodes(node: Node) {
!(conflict instanceof UnusedBind) &&
!context
.getRoot(node)
?.getAncestors(conflict.getConflictingNodes().primary.node)
?.getAncestors(
conflict.getConflictingNodes(Templates).primary.node,
)
.some((n) => n instanceof Example),
);

Expand All @@ -52,7 +55,7 @@ function checkBasisNodes(node: Node) {
conflicts
.map((c) =>
c
.getConflictingNodes()
.getConflictingNodes(Templates)
.primary.explanation(DefaultLocales, context)
.toText(),
)
Expand Down
3 changes: 2 additions & 1 deletion src/components/annotations/Annotations.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import Context from '@nodes/Context';
import CommandButton from '@components/widgets/CommandButton.svelte';
import Expander from '@components/widgets/Expander.svelte';
import Templates from '@concepts/Templates';
/** The project for which annotations should be shown */
export let project: Project;
Expand Down Expand Up @@ -138,7 +139,7 @@
// Conflict all of the active conflicts to a list of annotations.
annotations = conflicts
.map((conflict: Conflict) => {
const nodes = conflict.getConflictingNodes();
const nodes = conflict.getConflictingNodes(Templates);
const primary = nodes.primary;
const secondary = nodes.secondary;
// Based on the primary and secondary nodes given, decide what to show.
Expand Down
12 changes: 12 additions & 0 deletions src/components/editor/InputView.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<svelte:options immutable={true} />

<script lang="ts">
import type Input from '@nodes/Input';
import NodeView from './NodeView.svelte';
export let node: Input;
</script>

<NodeView node={node.name} /><NodeView node={node.bind} /><NodeView
node={node.value}
/>
13 changes: 7 additions & 6 deletions src/components/editor/Menu.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import Token from '../../nodes/Token';
import Bind from '../../nodes/Bind';
import Evaluate from '../../nodes/Evaluate';
import Input from '@nodes/Input';
export let menu: Menu;
/* What to run when hiding the menu */
Expand Down Expand Up @@ -48,13 +49,13 @@
let evaluateBind: Bind | undefined;
$: if (
selectedRevision instanceof Revision &&
newNode instanceof Bind &&
newNode instanceof Input &&
newParent instanceof Evaluate
) {
const fun = newParent.getFunction(selectedRevision.context);
evaluateBind = fun?.inputs.find(
(input) =>
newNode instanceof Bind && input.hasName(newNode.getNames()[0])
newNode instanceof Input && input.hasName(newNode.getName()),
);
}
$: selectedConcept =
Expand Down Expand Up @@ -108,11 +109,11 @@
.some(
(node) =>
node instanceof Token &&
node.getText().startsWith(event.key)
node.getText().startsWith(event.key),
)
: $locales
.get((l) => l.term[revision.purpose])
.startsWith(event.key)
.startsWith(event.key),
);
if (match)
menu = menu.inSubmenu()
Expand Down Expand Up @@ -194,8 +195,8 @@
`/${$locales.get((l) =>
entry instanceof RevisionSet
? l.term[entry.purpose]
: ''
)}…/`
: '',
)}…/`,
)}
/>
{/if}
Expand Down
3 changes: 3 additions & 0 deletions src/components/editor/util/nodeToView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import FormattedTranslationView from '../FormattedTranslationView.svelte';
import IsLocaleView from '../IsLocaleView.svelte';
import SpreadView from '../SpreadView.svelte';
import MatchView from '../MatchView.svelte';
import InputView from '../InputView.svelte';

import type Node from '@nodes/Node';
import Program from '@nodes/Program';
Expand Down Expand Up @@ -172,6 +173,7 @@ import Spread from '@nodes/Spread';
import NoneOrView from '../OtherwiseView.svelte';
import Otherwise from '@nodes/Otherwise';
import Match from '@nodes/Match';
import Input from '@nodes/Input';

const nodeToView = new Map<Function, ComponentType<SvelteComponent>>();

Expand Down Expand Up @@ -217,6 +219,7 @@ nodeToView.set(TextType, TextTypeView);
nodeToView.set(FunctionDefinition, FunctionDefinitionView);
nodeToView.set(FunctionType, FunctionTypeView);
nodeToView.set(Evaluate, EvaluateView);
nodeToView.set(Input, InputView);

nodeToView.set(ExpressionPlaceholder, ExpressionPlaceholderView);
nodeToView.set(BinaryEvaluate, BinaryEvaluateView);
Expand Down
3 changes: 2 additions & 1 deletion src/components/palette/editOutput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ import { toExpression } from '../../parser/parseExpression';
import { getPlaceExpression } from '../../output/getOrCreatePlace';
import type Spread from '../../nodes/Spread';
import type Locales from '../../locale/Locales';
import Input from '@nodes/Input';

export function getNumber(given: Expression): number | undefined {
const measurement =
given instanceof NumberLiteral
? given
: given instanceof Bind && given.value instanceof NumberLiteral
: given instanceof Input && given.value instanceof NumberLiteral
? given.value
: given instanceof UnaryEvaluate &&
given.isNegation() &&
Expand Down
3 changes: 2 additions & 1 deletion src/components/project/SourceTileToggle.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import Toggle from '../widgets/Toggle.svelte';
import { locales } from '../../db/Database';
import Emoji from '@components/app/Emoji.svelte';
import Templates from '@concepts/Templates';
export let source: Source;
export let expanded: boolean;
Expand All @@ -22,7 +23,7 @@
secondaryCount = 0;
if ($conflicts) {
for (const conflict of $conflicts) {
const nodes = conflict.getConflictingNodes();
const nodes = conflict.getConflictingNodes(Templates);
if (source.has(nodes.primary.node)) {
if (!conflict.isMinor()) primaryCount++;
else secondaryCount++;
Expand Down
3 changes: 3 additions & 0 deletions src/concepts/Templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,14 @@ import Borrow from '../nodes/Borrow';
import Otherwise from '@nodes/Otherwise';
import Match from '@nodes/Match';
import Spread from '@nodes/Spread';
import Input from '@nodes/Input';

/** These are ordered by appearance in the docs. */
const Templates: Node[] = [
// Evaluation
Evaluate.make(ExpressionPlaceholder.make(), []),
Input.make('_', ExpressionPlaceholder.make()),

FunctionDefinition.make(
undefined,
Names.make(['_']),
Expand Down
2 changes: 1 addition & 1 deletion src/conflicts/Conflict.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default abstract class Conflict {
* and "secondary" ones, which are involved. We use this distiction in the editor to decide what to highlight,
* but also how to position the various parties involved in the visual portrayal of the conflict.
*/
abstract getConflictingNodes(): {
abstract getConflictingNodes(concepts: Node[]): {
primary: ConflictingNode;
secondary?: ConflictingNode;
resolutions?: Resolution[];
Expand Down
44 changes: 0 additions & 44 deletions src/conflicts/MisplacedInput.ts

This file was deleted.

18 changes: 9 additions & 9 deletions src/conflicts/UnexpectedInput.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type Evaluate from '@nodes/Evaluate';
import Conflict from './Conflict';
import type Expression from '@nodes/Expression';
import type Bind from '@nodes/Bind';
import type BinaryEvaluate from '@nodes/BinaryEvaluate';
import type StructureDefinition from '@nodes/StructureDefinition';
import type FunctionDefinition from '@nodes/FunctionDefinition';
Expand All @@ -14,12 +13,12 @@ import type Locales from '../locale/Locales';
export default class UnexpectedInputs extends Conflict {
readonly func: FunctionDefinition | StructureDefinition | StreamDefinition;
readonly evaluate: Evaluate | BinaryEvaluate;
readonly input: Expression | Bind;
readonly input: Expression;

constructor(
func: FunctionDefinition | StructureDefinition | StreamDefinition,
evaluate: Evaluate | BinaryEvaluate,
input: Expression | Bind
input: Expression,
) {
super(false);
this.func = func;
Expand All @@ -30,28 +29,29 @@ export default class UnexpectedInputs extends Conflict {
getConflictingNodes() {
return {
primary: {
node: this.evaluate,
node: this.input,
explanation: (locales: Locales, context: Context) =>
concretize(
locales,
locales.get(
(l) =>
l.node.Evaluate.conflict.UnexpectedInput.primary
l.node.Evaluate.conflict.UnexpectedInput
.primary,
),
new NodeRef(this.input, locales, context)
new NodeRef(this.input, locales, context),
),
},
secondary: {
node: this.input,
node: this.func.names,
explanation: (locales: Locales, context: Context) =>
concretize(
locales,
locales.get(
(l) =>
l.node.Evaluate.conflict.UnexpectedInput
.secondary
.secondary,
),
new NodeRef(this.input, locales, context)
new NodeRef(this.input, locales, context),
),
},
};
Expand Down
16 changes: 10 additions & 6 deletions src/conflicts/UnknownInput.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
import type Evaluate from '@nodes/Evaluate';
import Conflict from './Conflict';
import type Bind from '@nodes/Bind';
import type StructureDefinition from '@nodes/StructureDefinition';
import type FunctionDefinition from '@nodes/FunctionDefinition';
import type StreamDefinition from '../nodes/StreamDefinition';
import concretize from '../locale/concretize';
import type BinaryEvaluate from '../nodes/BinaryEvaluate';
import type Locales from '../locale/Locales';
import type Input from '@nodes/Input';
import NodeRef from '@locale/NodeRef';
import Context from '@nodes/Context';

export default class UnknownInput extends Conflict {
readonly func: FunctionDefinition | StructureDefinition | StreamDefinition;
readonly evaluate: Evaluate | BinaryEvaluate;
readonly given: Bind;
readonly given: Input;

constructor(
func: FunctionDefinition | StructureDefinition | StreamDefinition,
evaluate: Evaluate | BinaryEvaluate,
given: Bind,
given: Input,
) {
super(false);

Expand All @@ -28,25 +30,27 @@ export default class UnknownInput extends Conflict {
getConflictingNodes() {
return {
primary: {
node: this.given.names,
explanation: (locales: Locales) =>
node: this.given.name,
explanation: (locales: Locales, context: Context) =>
concretize(
locales,
locales.get(
(l) =>
l.node.Evaluate.conflict.UnknownInput.primary,
),
new NodeRef(this.func, locales, context),
),
},
secondary: {
node: this.given.names,
node: this.func.names,
explanation: (locales: Locales) =>
concretize(
locales,
locales.get(
(l) =>
l.node.Evaluate.conflict.UnknownInput.secondary,
),
this.given.name.getText(),
),
},
};
Expand Down
10 changes: 5 additions & 5 deletions src/conflicts/UnparsableConflict.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { toTokens } from '@parser/toTokens';
import { Any, IsA } from '@nodes/Node';
import Token from '@nodes/Token';
import NodeRef from '@locale/NodeRef';
import type Node from '@nodes/Node';

export class UnparsableConflict extends Conflict {
readonly unparsable: UnparsableType | UnparsableExpression;
Expand All @@ -24,7 +25,7 @@ export class UnparsableConflict extends Conflict {
this.context = context;
}

getConflictingNodes() {
getConflictingNodes(nodes: Node[]) {
return {
primary: {
node: this.unparsable,
Expand All @@ -39,20 +40,19 @@ export class UnparsableConflict extends Conflict {
this.unparsable instanceof UnparsableExpression,
),
},
resolutions: this.getLikelyIntensions(),
resolutions: this.getLikelyIntentions(nodes),
};
}

getLikelyIntensions(): Resolution[] {
getLikelyIntentions(templates: Node[]): Resolution[] {
// Construct a set of tokens that weren't parseable so that we can find overlaps between this and possible templates.
const unparsableTokens = new Set(
this.unparsable.unparsables.map((t) => t.toWordplay()),
);

// Scan through templates of possible expressions in the language, scoring them by number of overlapping tokens.
return (
this.context
.getTemplates()
templates
// Only consider expressions
.filter(
(template): template is Expression =>
Expand Down
Loading

0 comments on commit 30b34e6

Please sign in to comment.