Display argument placeholders for autoscoped constructors (#9737)

Fixes #9635 

<img width="925" alt="Screenshot 2024-04-18 at 2 43 09 PM" src="https://github.com/enso-org/enso/assets/6566674/fbdce484-ac0b-4e30-8577-1c9dc419dffe">
This commit is contained in:
Ilya Bogdanov 2024-04-24 15:40:42 +04:00 committed by GitHub
parent 70985932b1
commit 717f6bb330
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 126 additions and 6 deletions

View File

@ -5,7 +5,7 @@ import { mockCollapsedFunctionInfo } from './expressionUpdates'
import { CONTROL_KEY } from './keyboard' import { CONTROL_KEY } from './keyboard'
import * as locate from './locate' import * as locate from './locate'
const MAIN_FILE_NODES = 11 const MAIN_FILE_NODES = 12
const COLLAPSE_SHORTCUT = `${CONTROL_KEY}+G` const COLLAPSE_SHORTCUT = `${CONTROL_KEY}+G`
@ -120,6 +120,7 @@ async function expectInsideMain(page: Page) {
await expect(locate.graphNodeByBinding(page, 'data')).toExist() await expect(locate.graphNodeByBinding(page, 'data')).toExist()
await expect(locate.graphNodeByBinding(page, 'aggregated')).toExist() await expect(locate.graphNodeByBinding(page, 'aggregated')).toExist()
await expect(locate.graphNodeByBinding(page, 'filtered')).toExist() await expect(locate.graphNodeByBinding(page, 'filtered')).toExist()
await expect(locate.graphNodeByBinding(page, 'autoscoped')).toExist()
} }
async function expectInsideFunc1(page: Page) { async function expectInsideFunc1(page: Page) {

View File

@ -101,7 +101,7 @@ test('Graph Editor pans to Component Browser', async ({ page }) => {
await expect(locate.graphNodeByBinding(page, 'five')).toBeInViewport() await expect(locate.graphNodeByBinding(page, 'five')).toBeInViewport()
const outputPort = await locate.outputPortCoordinates(locate.graphNodeByBinding(page, 'final')) const outputPort = await locate.outputPortCoordinates(locate.graphNodeByBinding(page, 'final'))
await page.mouse.click(outputPort.x, outputPort.y) await page.mouse.click(outputPort.x, outputPort.y)
await page.mouse.click(100, 1550) await page.mouse.click(100, 1700)
await expect(locate.graphNodeByBinding(page, 'five')).not.toBeInViewport() await expect(locate.graphNodeByBinding(page, 'five')).not.toBeInViewport()
await expectAndCancelBrowser(page, 'final.') await expectAndCancelBrowser(page, 'final.')
}) })

View File

@ -12,7 +12,7 @@ test('Existence of edges between nodes', async ({ page }) => {
await expect(await edgesFromNodeWithBinding(page, 'aggregated')).toHaveCount(0) await expect(await edgesFromNodeWithBinding(page, 'aggregated')).toHaveCount(0)
await expect(await edgesFromNodeWithBinding(page, 'filtered')).toHaveCount(0) await expect(await edgesFromNodeWithBinding(page, 'filtered')).toHaveCount(0)
await expect(await edgesFromNodeWithBinding(page, 'data')).toHaveCount(3 * EDGE_PARTS) await expect(await edgesFromNodeWithBinding(page, 'data')).toHaveCount(4 * EDGE_PARTS)
await expect(await edgesFromNodeWithBinding(page, 'list')).toHaveCount(0) await expect(await edgesFromNodeWithBinding(page, 'list')).toHaveCount(0)
await expect(await edgesFromNodeWithBinding(page, 'final')).toHaveCount(0) await expect(await edgesFromNodeWithBinding(page, 'final')).toHaveCount(0)
await expect(await edgesFromNodeWithBinding(page, 'selected')).toHaveCount(0) await expect(await edgesFromNodeWithBinding(page, 'selected')).toHaveCount(0)

View File

@ -440,3 +440,39 @@ test('Managing aggregates in `aggregate` node', async ({ page }) => {
// '"', // '"',
// ]) // ])
}) })
// Test that autoscoped constructors provide argument placeholders.
// This test can be removed when `aggregate` inserts autoscoped constructors by default,
// so this behavior will be tested in regular `aggregate` tests.
test('Autoscoped constructors', async ({ page }) => {
await actions.goToGraph(page)
await mockMethodCallInfo(page, 'autoscoped', {
methodPointer: {
module: 'Standard.Table.Table',
definedOnType: 'Standard.Table.Table.Table',
name: 'aggregate',
},
notAppliedArguments: [2, 3],
})
await mockMethodCallInfo(
page,
{ binding: 'autoscoped', expr: '..Group_By' },
{
methodPointer: {
module: 'Standard.Table.Aggregate_Column',
definedOnType: 'Standard.Table.Aggregate_Column.Aggregate_Column',
name: 'Group_By',
},
notAppliedArguments: [0, 1],
},
)
const node = locate.graphNodeByBinding(page, 'autoscoped')
const topLevelArgs = node.locator('.WidgetTopLevelArgument')
// Wait for hidden arguments to appear after selecting the node.
await node.click()
await expect(topLevelArgs).toHaveCount(3)
const groupBy = node.locator('.item').nth(0)
await expect(groupBy).toBeVisible()
await expect(groupBy.locator('.WidgetArgumentName')).toContainText(['column', 'new_name'])
})

View File

@ -17,6 +17,7 @@ const conf = [
'templates', 'templates',
'.histoire', '.histoire',
'playwright-report', 'playwright-report',
'test-results',
], ],
}, },
...compat.extends('plugin:vue/vue3-recommended'), ...compat.extends('plugin:vue/vue3-recommended'),

View File

@ -70,6 +70,7 @@ main =
data = Data.read data = Data.read
filtered = data.filter filtered = data.filter
aggregated = data.aggregate aggregated = data.aggregate
autoscoped = data.aggregate [..Group_By]
selected = data.select_columns selected = data.select_columns
` `

View File

@ -48,7 +48,7 @@ export default defineConfig({
use: { use: {
headless: !DEBUG, headless: !DEBUG,
trace: 'retain-on-failure', trace: 'retain-on-failure',
viewport: { width: 1920, height: 1600 }, viewport: { width: 1920, height: 1750 },
...(DEBUG ? ...(DEBUG ?
{} {}
: { : {

View File

@ -39,6 +39,7 @@ import {
App, App,
Assignment, Assignment,
Ast, Ast,
AutoscopedIdentifier,
BodyBlock, BodyBlock,
Documented, Documented,
Function, Function,
@ -203,6 +204,12 @@ class Abstractor {
} }
break break
} }
case RawAst.Tree.Type.AutoscopedIdentifier: {
const opr = this.abstractToken(tree.opr)
const ident = this.abstractToken(tree.ident)
node = AutoscopedIdentifier.concrete(this.module, opr, ident)
break
}
case RawAst.Tree.Type.OprApp: { case RawAst.Tree.Type.OprApp: {
const lhs = tree.lhs ? this.abstractTree(tree.lhs) : undefined const lhs = tree.lhs ? this.abstractTree(tree.lhs) : undefined
const opr = const opr =

View File

@ -77,6 +77,7 @@ export interface IdentifierToken extends Token {
declare const qualifiedNameBrand: unique symbol declare const qualifiedNameBrand: unique symbol
declare const identifierBrand: unique symbol declare const identifierBrand: unique symbol
declare const typeOrConsIdentifierBrand: unique symbol
declare const operatorBrand: unique symbol declare const operatorBrand: unique symbol
/** A string representing a valid qualified name of our language. /** A string representing a valid qualified name of our language.
@ -89,6 +90,9 @@ export type QualifiedName = string & { [qualifiedNameBrand]: never }
/** A string representing a lexical identifier. */ /** A string representing a lexical identifier. */
export type Identifier = string & { [identifierBrand]: never; [qualifiedNameBrand]: never } export type Identifier = string & { [identifierBrand]: never; [qualifiedNameBrand]: never }
/** A specific subtype of capitalized identifier, used for type and constructor names. */
export type TypeOrConstructorIdentifier = Identifier & { [typeOrConsIdentifierBrand]: never }
/** A string representing a lexical operator. */ /** A string representing a lexical operator. */
export type Operator = string & { [operatorBrand]: never; [qualifiedNameBrand]: never } export type Operator = string & { [operatorBrand]: never; [qualifiedNameBrand]: never }
@ -121,6 +125,11 @@ export function isIdentifier(code: string): code is Identifier {
return is_ident_or_operator(code) === 1 return is_ident_or_operator(code) === 1
} }
export function isTypeOrConsIdentifier(code: string): code is TypeOrConstructorIdentifier {
const isUppercase = (s: string) => s.toUpperCase() === s && s.toLowerCase() !== s
return isIdentifier(code) && code.length > 0 && isUppercase(code[0]!)
}
export function identifier(code: string): Identifier | undefined { export function identifier(code: string): Identifier | undefined {
if (isIdentifier(code)) return code if (isIdentifier(code)) return code
} }

View File

@ -10,6 +10,7 @@ import type {
RawNodeChild, RawNodeChild,
SpanMap, SpanMap,
SyncTokenId, SyncTokenId,
TypeOrConstructorIdentifier,
} from '.' } from '.'
import { import {
MutableModule, MutableModule,
@ -763,6 +764,61 @@ export interface MutableUnaryOprApp extends UnaryOprApp, MutableAst {
} }
applyMixins(MutableUnaryOprApp, [MutableAst]) applyMixins(MutableUnaryOprApp, [MutableAst])
interface AutoscopedIdentifierFields {
operator: NodeChild<SyncTokenId>
identifier: NodeChild<SyncTokenId>
}
export class AutoscopedIdentifier extends Ast {
declare fields: FixedMapView<AstFields & AutoscopedIdentifierFields>
constructor(module: Module, fields: FixedMapView<AstFields & AutoscopedIdentifierFields>) {
super(module, fields)
}
static tryParse(
source: string,
module?: MutableModule,
): Owned<MutableAutoscopedIdentifier> | undefined {
const parsed = parse(source, module)
if (parsed instanceof MutableAutoscopedIdentifier) return parsed
}
static concrete(module: MutableModule, operator: NodeChild<Token>, identifier: NodeChild<Token>) {
const base = module.baseObject('AutoscopedIdentifier')
const fields = composeFieldData(base, {
operator,
identifier,
})
return asOwned(new MutableAutoscopedIdentifier(module, fields))
}
static new(
identifier: TypeOrConstructorIdentifier,
module?: MutableModule,
): Owned<MutableAutoscopedIdentifier> {
const module_ = module || MutableModule.Transient()
const operator = Token.new('..')
const ident = Token.new(identifier, RawAst.Token.Type.Ident)
return this.concrete(module_, unspaced(operator), unspaced(ident))
}
*concreteChildren(_verbatim?: boolean): IterableIterator<RawNodeChild> {
const { operator, identifier } = getAll(this.fields)
yield operator
yield identifier
}
}
export class MutableAutoscopedIdentifier extends AutoscopedIdentifier implements MutableAst {
declare readonly module: MutableModule
declare readonly fields: FixedMap<AstFields & AutoscopedIdentifierFields>
setIdentifier(value: TypeOrConstructorIdentifier) {
const token = Token.new(value, RawAst.Token.Type.Ident)
this.fields.set('identifier', unspaced(token))
}
}
export interface MutableAutoscopedIdentifier extends AutoscopedIdentifier, MutableAst {}
applyMixins(MutableAutoscopedIdentifier, [MutableAst])
interface NegationAppFields { interface NegationAppFields {
operator: NodeChild<SyncTokenId> operator: NodeChild<SyncTokenId>
argument: NodeChild<AstId> argument: NodeChild<AstId>
@ -2369,6 +2425,8 @@ export function materializeMutable(module: MutableModule, fields: FixedMap<AstFi
return new MutableTextLiteral(module, fieldsForType) return new MutableTextLiteral(module, fieldsForType)
case 'UnaryOprApp': case 'UnaryOprApp':
return new MutableUnaryOprApp(module, fieldsForType) return new MutableUnaryOprApp(module, fieldsForType)
case 'AutoscopedIdentifier':
return new MutableAutoscopedIdentifier(module, fieldsForType)
case 'Vector': case 'Vector':
return new MutableVector(module, fieldsForType) return new MutableVector(module, fieldsForType)
case 'Wildcard': case 'Wildcard':
@ -2413,6 +2471,8 @@ export function materialize(module: Module, fields: FixedMapView<AstFields>): As
return new TextLiteral(module, fields_) return new TextLiteral(module, fields_)
case 'UnaryOprApp': case 'UnaryOprApp':
return new UnaryOprApp(module, fields_) return new UnaryOprApp(module, fields_)
case 'AutoscopedIdentifier':
return new AutoscopedIdentifier(module, fields_)
case 'Vector': case 'Vector':
return new Vector(module, fields_) return new Vector(module, fields_)
case 'Wildcard': case 'Wildcard':

View File

@ -65,7 +65,8 @@ export namespace WidgetInput {
input.value instanceof Ast.App || input.value instanceof Ast.App ||
input.value instanceof Ast.Ident || input.value instanceof Ast.Ident ||
input.value instanceof Ast.PropertyAccess || input.value instanceof Ast.PropertyAccess ||
input.value instanceof Ast.OprApp input.value instanceof Ast.OprApp ||
input.value instanceof Ast.AutoscopedIdentifier
) )
} }
} }

View File

@ -636,7 +636,11 @@ export const useGraphStore = defineStore('graph', () => {
exprId = nodeId exprId = nodeId
} }
if (exprId == null) bail(`Cannot find expression located by ${locator}`) if (exprId == null) {
const locatorStr =
typeof locator === 'string' ? locator : `${locator.binding}/${locator.expr}`
bail(`Cannot find expression located by ${locatorStr}`)
}
const update_: ExpressionUpdate = { const update_: ExpressionUpdate = {
expressionId: db.idToExternal(exprId)!, expressionId: db.idToExternal(exprId)!,