Header removals wait for the code editor to be left: autosave saves with drop=false, so follow_header adds and renames from the header but keeps a port or setting whose parameter is gone, and leaving the editor (blur or closing the panel) saves once more with removals; source saves of a flow run one at a time
This commit is contained in:
@@ -674,7 +674,7 @@ def read_node_source(
|
|||||||
|
|
||||||
|
|
||||||
def _follow_header(
|
def _follow_header(
|
||||||
controller: FlowController, name: str, node_id: str, code: str
|
controller: FlowController, name: str, node_id: str, code: str, drop: bool
|
||||||
) -> FlowDef | None:
|
) -> FlowDef | None:
|
||||||
"""Bring a function node's ports and settings in line with its header.
|
"""Bring a function node's ports and settings in line with its header.
|
||||||
|
|
||||||
@@ -694,7 +694,7 @@ def _follow_header(
|
|||||||
or definition.origin
|
or definition.origin
|
||||||
):
|
):
|
||||||
return None
|
return None
|
||||||
followed = follow_header(node, code)
|
followed = follow_header(node, code, drop)
|
||||||
if followed == node:
|
if followed == node:
|
||||||
return None
|
return None
|
||||||
definition.nodes = [followed if n.id == node_id else n for n in definition.nodes]
|
definition.nodes = [followed if n.id == node_id else n for n in definition.nodes]
|
||||||
@@ -716,6 +716,7 @@ async def save_node_source(
|
|||||||
source: NodeSource,
|
source: NodeSource,
|
||||||
controller: FlowControllerDep,
|
controller: FlowControllerDep,
|
||||||
user: CurrentUser,
|
user: CurrentUser,
|
||||||
|
drop: bool = True,
|
||||||
) -> Any:
|
) -> Any:
|
||||||
"""Save a node's source as an unpublished edit and report whether it loads.
|
"""Save a node's source as an unpublished edit and report whether it loads.
|
||||||
|
|
||||||
@@ -730,7 +731,9 @@ async def save_node_source(
|
|||||||
|
|
||||||
A node with its own code takes its input ports and settings from the
|
A node with its own code takes its input ports and settings from the
|
||||||
header of ``process`` (see `follow_header`), and the answer carries the
|
header of ``process`` (see `follow_header`), and the answer carries the
|
||||||
node when that changed it.
|
node when that changed it. ``drop=false`` is the editor's autosave, which
|
||||||
|
adds and renames from the header but keeps what it no longer names until
|
||||||
|
the editor is left.
|
||||||
"""
|
"""
|
||||||
definition = _read_flow(controller, name)
|
definition = _read_flow(controller, name)
|
||||||
ref = _source_ref(definition, node_id)
|
ref = _source_ref(definition, node_id)
|
||||||
@@ -746,7 +749,7 @@ async def save_node_source(
|
|||||||
controller.store.write_node_source, name, node_id, source.code, True
|
controller.store.write_node_source, name, node_id, source.code, True
|
||||||
)
|
)
|
||||||
followed = await run_in_threadpool(
|
followed = await run_in_threadpool(
|
||||||
_follow_header, controller, name, node_id, source.code
|
_follow_header, controller, name, node_id, source.code, drop
|
||||||
)
|
)
|
||||||
node_def = next((n for n in definition.nodes if n.id == node_id), None)
|
node_def = next((n for n in definition.nodes if n.id == node_id), None)
|
||||||
device = (
|
device = (
|
||||||
|
|||||||
@@ -1837,7 +1837,7 @@ def _signature_gap(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def follow_header(node: NodeDef, code: str) -> NodeDef:
|
def follow_header(node: NodeDef, code: str, drop: bool = True) -> NodeDef:
|
||||||
"""The node with its input ports and settings read off ``def process(...)``.
|
"""The node with its input ports and settings read off ``def process(...)``.
|
||||||
|
|
||||||
The inverse of the canvas's scaffold, which writes the input ports first,
|
The inverse of the canvas's scaffold, which writes the input ports first,
|
||||||
@@ -1845,7 +1845,9 @@ def follow_header(node: NodeDef, code: str) -> NodeDef:
|
|||||||
keeps its kind. A new one is a setting when it has a default, which becomes
|
keeps its kind. A new one is a setting when it has a default, which becomes
|
||||||
its value, or when it follows a setting, and an input port otherwise. One
|
its value, or when it follows a setting, and an input port otherwise. One
|
||||||
name swapped for one other is a rename, so the port keeps its message and
|
name swapped for one other is a rename, so the port keeps its message and
|
||||||
the setting its value. A declared name the header no longer has is dropped.
|
the setting its value. A declared name the header no longer has is dropped,
|
||||||
|
unless ``drop`` is false: code saved while it is still being typed keeps
|
||||||
|
those, so a pause mid-edit does not cost a port its message.
|
||||||
|
|
||||||
A header this cannot read for certain leaves the node as it is: a syntax
|
A header this cannot read for certain leaves the node as it is: a syntax
|
||||||
error, no top-level ``def process``, a decorator, ``*args``, ``**kwargs``,
|
error, no top-level ``def process``, a decorator, ``*args``, ``**kwargs``,
|
||||||
@@ -1907,6 +1909,13 @@ def follow_header(node: NodeDef, code: str) -> NodeDef:
|
|||||||
after_setting = True
|
after_setting = True
|
||||||
else:
|
else:
|
||||||
requires.append(MessageSpec(port=name))
|
requires.append(MessageSpec(port=name))
|
||||||
|
if not drop:
|
||||||
|
kept = set(gone) - set(renamed.values())
|
||||||
|
# Where they were, so that typing the name back changes nothing.
|
||||||
|
for index, spec in enumerate(node.requires):
|
||||||
|
if spec.port in kept:
|
||||||
|
requires.insert(index, spec)
|
||||||
|
params |= {key: settings[key] for key in kept if key in settings}
|
||||||
# Ports the header cannot name, such as a row not named yet, stay.
|
# Ports the header cannot name, such as a row not named yet, stay.
|
||||||
requires += [spec for spec in node.requires if not written(spec.port)]
|
requires += [spec for spec in node.requires if not written(spec.port)]
|
||||||
return node.model_copy(update={"requires": requires, "params": params})
|
return node.model_copy(update={"requires": requires, "params": params})
|
||||||
|
|||||||
@@ -580,6 +580,13 @@ def test_saving_a_header_moves_the_node_ports_in_the_draft(
|
|||||||
assert response["version"] == saved["definition"]["version"] + 1
|
assert response["version"] == saved["definition"]["version"] + 1
|
||||||
logger = next(n for n in draft["definition"]["nodes"] if n["id"] == "logger")
|
logger = next(n for n in draft["definition"]["nodes"] if n["id"] == "logger")
|
||||||
assert logger["params"] == {"gain": 2}
|
assert logger["params"] == {"gain": 2}
|
||||||
|
# The editor's autosave: a parameter typed away keeps its port for now.
|
||||||
|
paused = client.put(
|
||||||
|
f"{PREFIX}/header/nodes/logger/source?drop=false",
|
||||||
|
headers=superuser_token_headers,
|
||||||
|
json={"code": "def process(reading, gain=2):\n return {}\n"},
|
||||||
|
).json()
|
||||||
|
assert paused["node"] is None
|
||||||
|
|
||||||
client.delete(f"{PREFIX}/header", headers=superuser_token_headers)
|
client.delete(f"{PREFIX}/header", headers=superuser_token_headers)
|
||||||
|
|
||||||
|
|||||||
@@ -192,6 +192,21 @@ def test_a_renamed_parameter_keeps_the_message_and_a_dropped_one_goes():
|
|||||||
assert dropped.params == {"factor": 3}
|
assert dropped.params == {"factor": 3}
|
||||||
|
|
||||||
|
|
||||||
|
def test_without_drop_a_missing_parameter_stays_and_the_rest_follows():
|
||||||
|
node = _flow(
|
||||||
|
[*READING, MessageSpec(name="garden.level", port="depth")], factor=3
|
||||||
|
).nodes[0]
|
||||||
|
|
||||||
|
paused = follow_header(node, "def process(depth): pass\n", drop=False)
|
||||||
|
renamed = follow_header(node, "def process(value, depth, factor): pass\n", False)
|
||||||
|
added = follow_header(node, "def process(depth, gain=2): pass\n", drop=False)
|
||||||
|
|
||||||
|
assert paused == node
|
||||||
|
assert renamed.requires[0] == READING[0].model_copy(update={"port": "value"})
|
||||||
|
assert [spec.port for spec in added.requires] == ["reading", "depth"]
|
||||||
|
assert added.params == {"factor": 3, "gain": 2}
|
||||||
|
|
||||||
|
|
||||||
def test_a_new_parameter_after_a_setting_is_a_setting():
|
def test_a_new_parameter_after_a_setting_is_a_setting():
|
||||||
node = _flow(READING, factor=3).nodes[0]
|
node = _flow(READING, factor=3).nodes[0]
|
||||||
|
|
||||||
|
|||||||
+4
-3
@@ -41,9 +41,10 @@ settings are kept in step, in both directions.
|
|||||||
- Saving the code reads that line back. A name the node already has keeps
|
- Saving the code reads that line back. A name the node already has keeps
|
||||||
its kind. A new parameter becomes a setting when it has a default, which
|
its kind. A new parameter becomes a setting when it has a default, which
|
||||||
becomes its value, or when it comes after a setting. Any other new
|
becomes its value, or when it comes after a setting. Any other new
|
||||||
parameter becomes an input port. Removing a parameter removes its port or
|
parameter becomes an input port. Renaming a single one renames the port
|
||||||
setting, and renaming a single one renames the port (keeping its message)
|
(keeping its message) or the setting (keeping its value). Removing a
|
||||||
or the setting (keeping its value).
|
parameter removes its port or setting once you leave the editor, so one
|
||||||
|
deleted mid-edit and typed back keeps its message.
|
||||||
- Shared code, code from `fluksio sync`, and a header with a decorator,
|
- Shared code, code from `fluksio sync`, and a header with a decorator,
|
||||||
`*args`, `**kwargs`, a bare `*` or `/` are left alone in both directions.
|
`*args`, `**kwargs`, a bare `*` or `/` are left alone in both directions.
|
||||||
|
|
||||||
|
|||||||
@@ -1119,11 +1119,14 @@ export class FlowsService {
|
|||||||
*
|
*
|
||||||
* A node with its own code takes its input ports and settings from the
|
* A node with its own code takes its input ports and settings from the
|
||||||
* header of ``process`` (see `follow_header`), and the answer carries the
|
* header of ``process`` (see `follow_header`), and the answer carries the
|
||||||
* node when that changed it.
|
* node when that changed it. ``drop=false`` is the editor's autosave, which
|
||||||
|
* adds and renames from the header but keeps what it no longer names until
|
||||||
|
* the editor is left.
|
||||||
* @param data The data for the request.
|
* @param data The data for the request.
|
||||||
* @param data.name
|
* @param data.name
|
||||||
* @param data.nodeId
|
* @param data.nodeId
|
||||||
* @param data.requestBody
|
* @param data.requestBody
|
||||||
|
* @param data.drop
|
||||||
* @returns NodeSourceSaved Successful Response
|
* @returns NodeSourceSaved Successful Response
|
||||||
* @throws ApiError
|
* @throws ApiError
|
||||||
*/
|
*/
|
||||||
@@ -1135,6 +1138,9 @@ export class FlowsService {
|
|||||||
name: data.name,
|
name: data.name,
|
||||||
node_id: data.nodeId
|
node_id: data.nodeId
|
||||||
},
|
},
|
||||||
|
query: {
|
||||||
|
drop: data.drop
|
||||||
|
},
|
||||||
body: data.requestBody,
|
body: data.requestBody,
|
||||||
mediaType: 'application/json',
|
mediaType: 'application/json',
|
||||||
errors: {
|
errors: {
|
||||||
|
|||||||
@@ -1849,6 +1849,7 @@ export type FlowsReadNodeSourceData = {
|
|||||||
export type FlowsReadNodeSourceResponse = (NodeSource);
|
export type FlowsReadNodeSourceResponse = (NodeSource);
|
||||||
|
|
||||||
export type FlowsSaveNodeSourceData = {
|
export type FlowsSaveNodeSourceData = {
|
||||||
|
drop?: boolean;
|
||||||
name: string;
|
name: string;
|
||||||
nodeId: string;
|
nodeId: string;
|
||||||
requestBody: NodeSource;
|
requestBody: NodeSource;
|
||||||
|
|||||||
@@ -865,7 +865,15 @@ function FlowEditorInner({
|
|||||||
})
|
})
|
||||||
|
|
||||||
const sourceMutation = useMutation({
|
const sourceMutation = useMutation({
|
||||||
mutationFn: async ({ nodeId, code }: { nodeId: string; code: string }) => {
|
mutationFn: async ({
|
||||||
|
nodeId,
|
||||||
|
code,
|
||||||
|
drop,
|
||||||
|
}: {
|
||||||
|
nodeId: string
|
||||||
|
code: string
|
||||||
|
drop?: boolean
|
||||||
|
}) => {
|
||||||
// The server reads the ports off the header and compares them with its
|
// The server reads the ports off the header and compares them with its
|
||||||
// draft, so the ports on the canvas have to be there first.
|
// draft, so the ports on the canvas have to be there first.
|
||||||
await flush()
|
await flush()
|
||||||
@@ -873,8 +881,12 @@ function FlowEditorInner({
|
|||||||
name: flowName,
|
name: flowName,
|
||||||
nodeId,
|
nodeId,
|
||||||
requestBody: { code },
|
requestBody: { code },
|
||||||
|
drop,
|
||||||
})
|
})
|
||||||
},
|
},
|
||||||
|
// One at a time: the save that drops a port must not land before the
|
||||||
|
// autosave that kept it.
|
||||||
|
scope: { id: `source:${flowName}` },
|
||||||
onSuccess: (saved) => {
|
onSuccess: (saved) => {
|
||||||
const followed = saved.node
|
const followed = saved.node
|
||||||
if (followed && saved.version) {
|
if (followed && saved.version) {
|
||||||
@@ -1454,8 +1466,9 @@ function FlowEditorInner({
|
|||||||
onToggleExpand={() => setEditorExpanded((wide) => !wide)}
|
onToggleExpand={() => setEditorExpanded((wide) => !wide)}
|
||||||
onChange={updateNode}
|
onChange={updateNode}
|
||||||
onRenameMessage={proposeRename}
|
onRenameMessage={proposeRename}
|
||||||
onSaveSource={(code) => {
|
onSaveSource={(code, drop) => {
|
||||||
if (selected) sourceMutation.mutate({ nodeId: selected.id, code })
|
if (selected)
|
||||||
|
sourceMutation.mutate({ nodeId: selected.id, code, drop })
|
||||||
}}
|
}}
|
||||||
// Sharing rewrites the stored document, so the canvas takes the
|
// Sharing rewrites the stored document, so the canvas takes the
|
||||||
// server's copy rather than keeping its own.
|
// server's copy rather than keeping its own.
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import Editor from "@monaco-editor/react"
|
import Editor from "@monaco-editor/react"
|
||||||
import { useEffect, useMemo, useState } from "react"
|
import { useEffect, useMemo, useRef, useState } from "react"
|
||||||
|
|
||||||
import { useTheme } from "@/components/theme-provider"
|
import { useTheme } from "@/components/theme-provider"
|
||||||
import { Skeleton } from "@/components/ui/skeleton"
|
import { Skeleton } from "@/components/ui/skeleton"
|
||||||
@@ -13,12 +13,15 @@ import { monacoFontFamily, setupMonaco } from "./monacoSetup"
|
|||||||
export default function NodeEditor({
|
export default function NodeEditor({
|
||||||
value,
|
value,
|
||||||
onChange,
|
onChange,
|
||||||
|
onBlur,
|
||||||
language = "python",
|
language = "python",
|
||||||
readOnly = false,
|
readOnly = false,
|
||||||
autoFocus = false,
|
autoFocus = false,
|
||||||
}: {
|
}: {
|
||||||
value: string
|
value: string
|
||||||
onChange: (next: string) => void
|
onChange: (next: string) => void
|
||||||
|
/** Focus left the editor, its find and suggest widgets included. */
|
||||||
|
onBlur?: () => void
|
||||||
language?: "python" | "json"
|
language?: "python" | "json"
|
||||||
/** Generated code: readable, and not this panel's to change. */
|
/** Generated code: readable, and not this panel's to change. */
|
||||||
readOnly?: boolean
|
readOnly?: boolean
|
||||||
@@ -27,6 +30,8 @@ export default function NodeEditor({
|
|||||||
}) {
|
}) {
|
||||||
const { resolvedTheme } = useTheme()
|
const { resolvedTheme } = useTheme()
|
||||||
const [ready, setReady] = useState(false)
|
const [ready, setReady] = useState(false)
|
||||||
|
const blur = useRef(onBlur)
|
||||||
|
blur.current = onBlur
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
setupMonaco()
|
setupMonaco()
|
||||||
@@ -43,7 +48,10 @@ export default function NodeEditor({
|
|||||||
theme={resolvedTheme === "dark" ? "fluksio-dark" : "fluksio-light"}
|
theme={resolvedTheme === "dark" ? "fluksio-dark" : "fluksio-light"}
|
||||||
value={value}
|
value={value}
|
||||||
onChange={(next) => onChange(next ?? "")}
|
onChange={(next) => onChange(next ?? "")}
|
||||||
onMount={autoFocus ? (editor) => editor.focus() : undefined}
|
onMount={(editor) => {
|
||||||
|
editor.onDidBlurEditorWidget(() => blur.current?.())
|
||||||
|
if (autoFocus) editor.focus()
|
||||||
|
}}
|
||||||
loading={<Skeleton className="h-full w-full rounded-md" />}
|
loading={<Skeleton className="h-full w-full rounded-md" />}
|
||||||
options={{
|
options={{
|
||||||
readOnly,
|
readOnly,
|
||||||
|
|||||||
@@ -1409,7 +1409,8 @@ function PanelBody({
|
|||||||
expanded: boolean
|
expanded: boolean
|
||||||
onChange: (next: NodeDef_Input) => void
|
onChange: (next: NodeDef_Input) => void
|
||||||
onRenameMessage: (previous: string, next: string) => void
|
onRenameMessage: (previous: string, next: string) => void
|
||||||
onSaveSource: (code: string) => void
|
/** `drop` is false while the code is still being typed; see `settle`. */
|
||||||
|
onSaveSource: (code: string, drop: boolean) => void
|
||||||
onShared: () => void
|
onShared: () => void
|
||||||
onToggleExpand: () => void
|
onToggleExpand: () => void
|
||||||
}) {
|
}) {
|
||||||
@@ -1431,6 +1432,7 @@ function PanelBody({
|
|||||||
// only overwrite the first.
|
// only overwrite the first.
|
||||||
const portMove = useRef<[string, string | null] | null>(null)
|
const portMove = useRef<[string, string | null] | null>(null)
|
||||||
const timer = useRef<ReturnType<typeof setTimeout> | null>(null)
|
const timer = useRef<ReturnType<typeof setTimeout> | null>(null)
|
||||||
|
// Code typed since the editor was last left.
|
||||||
const pending = useRef<string | null>(null)
|
const pending = useRef<string | null>(null)
|
||||||
const save = useRef(onSaveSource)
|
const save = useRef(onSaveSource)
|
||||||
save.current = onSaveSource
|
save.current = onSaveSource
|
||||||
@@ -1441,7 +1443,22 @@ function PanelBody({
|
|||||||
clearTimeout(timer.current)
|
clearTimeout(timer.current)
|
||||||
timer.current = null
|
timer.current = null
|
||||||
}
|
}
|
||||||
if (pending.current !== null) save.current(pending.current)
|
if (pending.current !== null) save.current(pending.current, false)
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The editor was left, so the code is saved once more, now dropping the
|
||||||
|
* ports and settings its header no longer names. Saves while typing only
|
||||||
|
* add and rename, so a parameter deleted mid-edit keeps its port and
|
||||||
|
* message until then, and typing it back changes nothing.
|
||||||
|
*/
|
||||||
|
const settle = () => {
|
||||||
|
if (timer.current) {
|
||||||
|
clearTimeout(timer.current)
|
||||||
|
timer.current = null
|
||||||
|
}
|
||||||
|
if (pending.current !== null) save.current(pending.current, true)
|
||||||
|
pending.current = null
|
||||||
}
|
}
|
||||||
|
|
||||||
const editCode = (next: string) => {
|
const editCode = (next: string) => {
|
||||||
@@ -1450,7 +1467,7 @@ function PanelBody({
|
|||||||
if (timer.current) clearTimeout(timer.current)
|
if (timer.current) clearTimeout(timer.current)
|
||||||
timer.current = setTimeout(() => {
|
timer.current = setTimeout(() => {
|
||||||
timer.current = null
|
timer.current = null
|
||||||
save.current(next)
|
save.current(next, false)
|
||||||
}, 1000)
|
}, 1000)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1520,13 +1537,13 @@ function PanelBody({
|
|||||||
["mod+s"],
|
["mod+s"],
|
||||||
)
|
)
|
||||||
|
|
||||||
// Closing the panel must not lose the last keystrokes.
|
// Closing the panel leaves the editor too, and must not lose the last
|
||||||
|
// keystrokes.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
return () => {
|
return () => {
|
||||||
if (timer.current) {
|
if (timer.current) clearTimeout(timer.current)
|
||||||
clearTimeout(timer.current)
|
if (pending.current !== null) save.current(pending.current, true)
|
||||||
if (pending.current !== null) save.current(pending.current)
|
pending.current = null
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}, [])
|
}, [])
|
||||||
|
|
||||||
@@ -1704,6 +1721,7 @@ function PanelBody({
|
|||||||
<NodeEditor
|
<NodeEditor
|
||||||
value={code ?? source?.code ?? ""}
|
value={code ?? source?.code ?? ""}
|
||||||
onChange={editCode}
|
onChange={editCode}
|
||||||
|
onBlur={settle}
|
||||||
readOnly={Boolean(origin) || !mayEditCode}
|
readOnly={Boolean(origin) || !mayEditCode}
|
||||||
/>
|
/>
|
||||||
</Suspense>
|
</Suspense>
|
||||||
@@ -1745,7 +1763,7 @@ export function NodePanel({
|
|||||||
expanded: boolean
|
expanded: boolean
|
||||||
onChange: (next: NodeDef_Input) => void
|
onChange: (next: NodeDef_Input) => void
|
||||||
onRenameMessage: (previous: string, next: string) => void
|
onRenameMessage: (previous: string, next: string) => void
|
||||||
onSaveSource: (code: string) => void
|
onSaveSource: (code: string, drop: boolean) => void
|
||||||
onShared: () => void
|
onShared: () => void
|
||||||
onToggleExpand: () => void
|
onToggleExpand: () => void
|
||||||
onClose: () => void
|
onClose: () => void
|
||||||
|
|||||||
Reference in New Issue
Block a user