fix(cli): timeline delete --start/--end removes only that range (#5867)

* fix(cli): timeline delete --start/--end removes only that range

timeline delete accepted --start and --end but never read them, so a range
delete removed the whole clip. It now cuts out only that range, the same way
split, split, delete would, and keeps the parts before and after.

Every edit verb shared one flag set, so a verb silently ignored flags it does
not use. Each verb now declares only the options it reads, from one table that
also checks apply-plan keys, so an unused flag or key is refused before
anything is written.

* refactor(cli): name the per-verb option table instead of commenting it
This commit is contained in:
Miguel Ángel
2026-10-19 01:23:00 +00:00
committed by GitHub
parent 2a14cbc924
commit d2b8ea7644
5 changed files with 206 additions and 32 deletions
+5 -3
View File
@@ -1125,6 +1125,7 @@ npx hyperframes timeline split '#outro' 6.5
npx hyperframes timeline set '#music' volume=0.5
npx hyperframes timeline duplicate '#intro' --at 3
npx hyperframes timeline delete '#outro' --json
npx hyperframes timeline delete '#outro' --start 6 --end 7 # cut one second out
```
| Command | What it does |
@@ -1132,7 +1133,7 @@ npx hyperframes timeline delete '#outro' --json
| `move <ref> <time>` | Start the clip at `<time>`, keeping its length |
| `trim <ref>` | Change the clip's `--start`, `--end`, or `--duration` (at least one) |
| `split <ref> <time>` | Cut the clip in two at `<time>` |
| `delete <ref>` | Remove the clip |
| `delete <ref>` | Remove the clip, or only its `--start` to `--end` range, keeping the parts before and after |
| `set <ref> <field>=<value>` | Set `volume`, `rate` (playback rate) or `track` (a whole number) |
| `duplicate <ref>` | Copy the clip to `--at <time>` (default: right after the clip) |
| `apply <file>` | Apply a JSON array of edits, such as `[{"verb": "move", "ref": "#intro", "time": "+1"}]`, all or nothing. Pass `-` to read it from stdin. |
@@ -1141,13 +1142,14 @@ npx hyperframes timeline delete '#outro' --json
A time is seconds (`4.5`), an offset from the clip's start (`+1`, `-0.5`),
frames (`30f`), `end`, or an anchor on another clip: `after:#intro`,
`before:#intro`, `start-of:#intro` or `end-of:#intro`.
`before:#intro`, `start-of:#intro` or `end-of:#intro`. A flag an edit command does not
use is refused before anything is written.
| Flag | Applies to | Description |
| ------------- | -------------------------------------------------------------- | ------------------------------------------------------------- |
| `--plan` | `move`, `trim`, `split`, `delete`, `set`, `duplicate`, `apply` | Print the result and the diff without writing |
| `--overwrite` | `move`, `trim` | Allow the clip to overlap a neighbour on its track |
| `--snap` | `move`, `trim`, `split`, `duplicate` | Round times to whole frames. Needs `data-fps` on the project. |
| `--snap` | `move`, `trim`, `split`, `delete`, `duplicate` | Round times to whole frames. Needs `data-fps` on the project. |
| `--at` | `duplicate` | Where the copy starts |
| `--dir` | all edit commands | Project directory (default: current) |
| `--json` | all edit commands | Machine-readable result, with the receipt `undo` takes |
+18 -9
View File
@@ -1,4 +1,4 @@
import { defineCommand } from "citty";
import { defineCommand, type ArgDef } from "citty";
import type { Example } from "./_examples.js";
import { describeProject } from "../timeline/describeProject.js";
import { formatTimeline } from "../timeline/formatTimeline.js";
@@ -7,7 +7,7 @@ import { resolveProject } from "../utils/project.js";
import { withMeta } from "../utils/updateCheck.js";
import { runApply, runIds, runUndo } from "../timeline/a2Commands.js";
import { runMutation } from "../timeline/a2MutationCommand.js";
import type { MutationVerb } from "../timeline/a2Shared.js";
import { FIELDS_READ_BY_VERB, type MutationVerb } from "../timeline/a2Shared.js";
import { resolveExtraPositionals } from "../utils/reject-extra-positionals.js";
import { assertKnownFlags } from "../utils/reject-unknown-flags.js";
@@ -17,21 +17,30 @@ export const examples: Example[] = [
["Delete a clip and return a receipt", "hyperframes timeline delete '#hero' --json"],
];
const MUTATION_OPTIONS: Record<string, ArgDef> = {
time: { type: "positional", required: true },
at: { type: "string" },
start: { type: "string" },
end: { type: "string" },
duration: { type: "string" },
volume: { type: "string" },
rate: { type: "string" },
track: { type: "string" },
overwrite: { type: "boolean", default: false },
};
function mutationCommand(verb: MutationVerb) {
const fields = FIELDS_READ_BY_VERB[verb];
return defineCommand({
meta: { name: verb, description: `${verb} a timeline clip` },
args: {
ref: { type: "positional", required: true },
time: { type: "positional", required: verb === "move" || verb === "split" },
at: { type: "string" },
...(verb === "set" && { assignment: { type: "positional", required: false } }),
...Object.fromEntries(fields.map((field) => [field, MUTATION_OPTIONS[field]!])),
dir: { type: "string" },
start: { type: "string" },
end: { type: "string" },
duration: { type: "string" },
plan: { type: "boolean", default: false },
json: { type: "boolean", default: false },
overwrite: { type: "boolean", default: false },
snap: { type: "boolean", default: false },
...(verb !== "set" && { snap: { type: "boolean", default: false } }),
},
async run({ args }) {
await runMutation(verb, args);
+12
View File
@@ -18,6 +18,7 @@ import {
diff,
fpsFor,
isRecord,
FIELDS_READ_BY_VERB,
mutationConflict,
positional,
publicReceipt,
@@ -104,6 +105,17 @@ function preparePlanEdit(
fix: "use move, trim, split, delete, set, or duplicate",
};
}
const fields = FIELDS_READ_BY_VERB[edit.verb as MutationVerb];
const unused = Object.keys(edit).find(
(key) => key !== "verb" && key !== "ref" && !fields.includes(key),
);
if (unused) {
return {
ok: false,
reason: `${edit.verb} does not use ${unused}`,
fix: `remove ${unused}; ${edit.verb} takes ${fields.join(", ")}`,
};
}
const resolved = resolveRef(timeline, edit.ref);
if (!resolved.ok) return resolved;
const before = sourceByFile.get(resolved.row.file);
+97 -20
View File
@@ -4,7 +4,11 @@ import {
removeElementFromHtml,
splitElementInHtml,
} from "@hyperframes/studio-server";
import type { AppliedFileMutation, PatchOperation } from "@hyperframes/studio-server";
import type {
AppliedFileMutation,
PatchOperation,
SourceMutationTarget,
} from "@hyperframes/studio-server";
import { fpsToNumber, parseFpsWithDefault } from "@hyperframes/core";
import { isInsideSpan, sameInstant, spansShareTime } from "@hyperframes/core/clip-facts";
import { readCompositionFps } from "../utils/compositionFps.js";
@@ -17,6 +21,15 @@ import { parseSetAssignments, type SetAssignment } from "./a2Mutations.js";
export type MutationVerb = "move" | "trim" | "split" | "delete" | "set" | "duplicate";
export const FIELDS_READ_BY_VERB: Record<MutationVerb, readonly string[]> = {
move: ["time", "overwrite"],
trim: ["start", "end", "duration", "overwrite"],
split: ["time"],
delete: ["start", "end"],
set: ["volume", "rate", "track"],
duplicate: ["at"],
};
export type MutationDecision =
| { ok: true; after: string; nextStart: number; nextDuration: number }
| { ok: false; reason: string; fix: string };
@@ -196,24 +209,14 @@ function splitMutation(context: MutationContext, args: Record<string, unknown>):
const expression = typeof args.time === "string" ? args.time : "";
const time = parseMutationTime(context, expression, "pass a valid time expression");
if (!time.ok) return time;
const split = splitElementInHtml(
const split = splitSource(
context,
context.before,
context.resolved.target,
time.seconds,
nextFreeSplitId(context.before, splitBaseId(context.row)),
{
start: context.row.start,
duration: context.row.duration,
track: context.row.trackIndex,
},
context.row.duration,
);
if (!split.matched || !split.newId) {
return {
ok: false,
reason: `${context.ref} cannot be split at ${time.seconds}`,
fix: "choose a time inside the clip",
};
}
if (!split.ok) return split;
return {
ok: true,
after: split.html,
@@ -222,6 +225,30 @@ function splitMutation(context: MutationContext, args: Record<string, unknown>):
};
}
function splitSource(
context: MutationContext,
source: string,
target: SourceMutationTarget,
seconds: number,
duration: number,
): { ok: true; html: string; newId: string } | { ok: false; reason: string; fix: string } {
const split = splitElementInHtml(
source,
target,
seconds,
nextFreeSplitId(source, splitBaseId(context.row)),
{ start: context.row.start, duration, track: context.row.trackIndex },
);
if (!split.matched || !split.newId) {
return {
ok: false,
reason: `${context.ref} cannot be split at ${seconds}`,
fix: "choose a time inside the clip",
};
}
return { ok: true, html: split.html, newId: split.newId };
}
function trimMutation(context: MutationContext, args: Record<string, unknown>): MutationDecision {
const bounds = trimBounds(context, args);
if (!bounds.ok) return bounds;
@@ -321,15 +348,65 @@ function trimDuration(context: MutationContext, expression: string | undefined)
return parseMutationTime(context, expression, "pass a valid duration");
}
function deleteMutation(context: MutationContext): MutationDecision {
function deleteMutation(context: MutationContext, args: Record<string, unknown>): MutationDecision {
const { row, resolved } = context;
const range = deleteRange(context, args);
if (!range.ok) return range;
let html = context.before;
if (!sameInstant(range.end, row.end)) {
const tail = splitSource(context, html, resolved.target, range.end, row.duration);
if (!tail.ok) return tail;
html = tail.html;
}
let removed = resolved.target;
if (!sameInstant(range.start, row.start)) {
const middle = splitSource(context, html, resolved.target, range.start, range.end - row.start);
if (!middle.ok) return middle;
html = middle.html;
removed = { id: middle.newId };
}
return {
ok: true,
after: removeElementFromHtml(context.before, context.resolved.target),
nextStart: context.row.start,
nextDuration: context.row.duration,
after: removeElementFromHtml(html, removed),
nextStart: row.start,
nextDuration: row.duration,
};
}
function deleteRange(
context: MutationContext,
args: Record<string, unknown>,
): { ok: true; start: number; end: number } | { ok: false; reason: string; fix: string } {
const { row } = context;
const bound = (value: unknown, fallback: number): ParsedMutationTime =>
typeof value === "string"
? parseMutationTime(context, value, "pass a valid time expression")
: { ok: true, seconds: fallback };
const start = bound(args.start, row.start);
if (!start.ok) return start;
const end = bound(args.end, row.end);
if (!end.ok) return end;
const inside = (seconds: number) =>
sameInstant(seconds, row.start) ||
sameInstant(seconds, row.end) ||
(seconds > row.start && seconds < row.end);
if (!inside(start.seconds) || !inside(end.seconds)) {
return {
ok: false,
reason: `delete range ${start.seconds}-${end.seconds} is not inside ${context.ref} (${row.start}-${row.end})`,
fix: "pass --start and --end inside the clip",
};
}
if (end.seconds <= start.seconds || sameInstant(start.seconds, end.seconds)) {
return {
ok: false,
reason: "delete range must be positive",
fix: "pass an --end later than --start",
};
}
return { ok: true, start: start.seconds, end: end.seconds };
}
function setOperation(assignment: SetAssignment): PatchOperation {
const property =
assignment.field === "volume"
@@ -403,7 +480,7 @@ export function decideMutation(
case "split":
return splitMutation(context, args);
case "delete":
return deleteMutation(context);
return deleteMutation(context, args);
case "set":
return setMutation(context, args);
case "duplicate":
@@ -212,6 +212,80 @@ describe("timeline edit command", () => {
}
});
it("deletes only the --start/--end range of a video clip, keeping both sides", () => {
const dir = mkdtempSync(join(tmpdir(), "hf-timeline-range-delete-"));
try {
const indexPath = join(dir, "index.html");
writeFileSync(
indexPath,
`<div data-composition-id="main" data-duration="12"><video id="clip" src="a.mp4" data-start="1" data-duration="4" data-track-index="0"></video></div>`,
);
const planned = run(dir, "delete", "#clip", "--start", "2", "--end", "3", "--plan");
expect(planned.status, planned.stderr).toBe(0);
const kept = [
expect.objectContaining({ ref: "#clip", start: 1, end: 2 }),
expect.objectContaining({ ref: "#clip-2", start: 3, end: 5 }),
];
expect(JSON.parse(planned.stdout).after).toEqual(kept);
const applied = run(dir, "delete", "#clip", "--start", "2", "--end", "3");
expect(applied.status, applied.stderr).toBe(0);
expect(JSON.parse(applied.stdout).after).toEqual(kept);
const html = readFileSync(indexPath, "utf8");
const tag = (id: string) => html.match(new RegExp(`<[^>]*\\sid="${id}"[^>]*>`))?.[0];
expect(tag("clip-2")).toContain('data-media-start="2"');
expect(html).not.toContain('id="clip-3"');
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
it("refuses a delete range that reaches outside the clip, without writing", () => {
const dir = project();
try {
const before = readFileSync(join(dir, "index.html"), "utf8");
const result = run(dir, "delete", "#clip", "--start", "2", "--end", "4");
expect(result.status).toBe(2);
expect(result.stderr).toContain("is not inside #clip");
expect(readFileSync(join(dir, "index.html"), "utf8")).toBe(before);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
it.each([
["--at", ["delete", "#clip", "--at", "2"]],
["--start", ["split", "#clip", "2", "--start", "1"]],
["--duration", ["move", "#clip", "+1", "--duration", "1"]],
["--overwrite", ["duplicate", "#clip", "--overwrite"]],
["--end", ["set", "#clip", "volume=0.5", "--end", "2"]],
])("refuses a flag its verb does not use (%s), without writing", (flag, args) => {
const dir = project();
try {
const before = readFileSync(join(dir, "index.html"), "utf8");
const result = run(dir, ...args);
expect(result.status).not.toBe(0);
expect(result.stderr).toContain(`Unknown flag: ${flag}`);
expect(readFileSync(join(dir, "index.html"), "utf8")).toBe(before);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
it("refuses an apply edit with a key its verb does not use, without writing", () => {
const dir = project();
try {
const before = readFileSync(join(dir, "index.html"), "utf8");
const planPath = join(dir, "edits.json");
writeFileSync(planPath, JSON.stringify([{ verb: "delete", ref: "#clip", at: "2" }]));
const result = run(dir, "apply", planPath);
expect(result.status).toBe(2);
expect(result.stderr).toContain("delete does not use at");
expect(readFileSync(join(dir, "index.html"), "utf8")).toBe(before);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
it("uses the next free numeric split suffix on repeat", () => {
const dir = project();
try {