Frame_mgmt

[AdrianGroty]

propagate edits to frame ids across frame values of corresponding text objects
add duplicate-frame, combine-duplicates, self-contain-frames, and delete-orphan-frame-elements commands

[AdrianGroty]

While none of this negates or contradicts any of the existing documentation, it does fundamentally change the behavior of the backing sheet, and the new relationship of frame ids to text objects' frame values may be worth documenting somewhere outside of the .ddw file here included.

[AdrianGroty]

In the process of creating this, I discovered an edge case when creating new frames with new-frame-(before|after), in which creating a new frame before the first or after the last would fail with error when frame id was not an integer. i.e. running new-frame-before from frame foo 0/4 resulted in error; no frame created.

I have attempted to address that issue here by appending -pre|-post to new frame id in such cases.

[saulpw]

This PR is missing stamps.py; was it just not committed?

[AdrianGroty]

It was not omitted through any deliberate choice of mine; I assume it did not yet exist upstream at the time of commit.

[saulbert]

Summary: Adds a new frame_mgmt.py module with five frame-management commands (duplicate-frame-{before,after}, prune-unused, deduplicate-text, deduplicate-frames, self-contain-frames) plus a frame_mgmt.ddw interactive tutorial. Also makes frame-id edits propagate to elements that reference that frame.

The features are useful, but a few blocking problems before this can land.

Blocking

a) __init__.py would be broken after merge. The PR's __init__.py imports .stamps and .save_dur, neither of which exists in this PR or in devottys/master. It also removes from .animation import * while still referencing FramesSheet in vd.addGlobals(...). The branch appears to have been rebased from a divergent fork — the __init__.py changes need to be redone against current devottys/master. Probably the only change needed there is +from .frame_mgmt import *.

b) deduplicate_* crash on real data. Both deduplicate_text and deduplicate_frames compute a row key with frozenset((r.tags or '').split()). In real .ddw files, tags is a JSON list (e.g. samples/bouncyball.ddw has "tags": ["down"]). When tags is a non-empty list, .split() raises AttributeError. Either convert via frozenset(r.tags or ()) (treat as iterable) or normalize at one site.

c) Module-level mutation of DrawingSheet.columns. The setter is installed by walking the class column list at import time and replacing the entry:

for _i, _c in enumerate(DrawingSheet.columns):
    if _c.name == 'id':
        DrawingSheet.columns[_i] = ItemColumn('id', type=str, setter=_link_id_setter)

This (i) re-runs side-effect on every import (idempotent here but smelly), (ii) silently drops any attributes the original id column might gain later, and (iii) makes ordering vs. other plugins fragile. Cleaner: subclass ItemColumn or override via a hook. (Side note: ItemColumn.putValue always does setitemdeep after the custom setter runs, so the setter doesn't need to write row.id itself — that part is correct.)

Significant

d) new_between_frame is duplicated. This function already exists in animation.py:30; the PR's copy adds the -pre/-post edge-case fix mentioned in the PR comment thread. After the __init__.py is fixed to keep animation, both registrations exist and last-import wins. The -pre/-post fix should be applied to animation.py's version in-place rather than forked.

e) CLAUDE.md convention violated for duplicate_frame. @Drawing.api def duplicate_frame(sheet, before=False) — Drawing methods take dwg, not sheet (per CLAUDE.md). Either rename the parameter or, since this is mostly data-model work, move it to @DrawingSheet.api and let the Drawing command delegate.

f) Frame-id rename collisions. _link_id_setter doesn't check whether the new value collides with an existing frame id. Renaming 1 → 2 when 2 exists silently re-targets elements without warning, and ends up with two frame rows sharing an id. Worth a vd.fail or at least vd.warning.

g) self_contain_frames undo double-registers. addRow already registers vd.addUndo(self.rows.remove, row) for each new row. The explicit vd.addUndo(self.deleteBy, lambda r, ids=new_ids: id(r) in ids) is duplicative and on undo will attempt to remove rows that the prior undo already removed.

h) PR description is stale. It lists combine-duplicates and delete-orphan-frame-elements, which are not the actual command names (now deduplicate-text and prune-unused).

Minor

i) Tutorial typo: "remnoved" → "removed" (frame 5 text).
j) duplicate-frame-{before,after} get gz[/gz] bindings; the other four commands are command-palette-only. Worth a sentence in the PR or tutorial about why.
k) _link_id_setter's if r is row or getattr(r, 'type', '') or not getattr(r, 'frame', ''): continue on one line is denser than the rest of the codebase; minor cosmetic.

Nice things

• The interactive .ddw tutorial as living docs is a great pattern.
• Multi-frame element semantics (space-separated frame ids) extending cleanly through self-contain-frames / deduplicate-text is well thought out.
• The -pre/-post suffix for non-integer adjacent frame ids is a useful edge-case fix.
• Defensive getattr(..., '') in the setter is the right instinct given how mixed .frame/.tags types are in this codebase.

---

[this message written by Claude Opus 4.7 and approved by @saulpw]

[saulpw]

frame_mgmt.py has to be imported in init, right?

[AdrianGroty]

Ah, it does; my mistake. I could have sworn I updated init.