From cdf37302d741e87fbaef574b90ec2f1e041cd69a Mon Sep 17 00:00:00 2001 From: Esa Kataja Date: Wed, 29 Jul 2026 15:06:45 +0300 Subject: [PATCH] Ask before reopening a bundle that carries no cuts A bundle without a source block cannot be round-tripped, but its PDF can still be cut from scratch. Offer that instead of refusing: confirm, run detection, and line the markers up by position when the slice counts match exactly. --- docs/bundle-format.md | 8 ++ docs/guide.md | 9 ++ noteman_slicer/bundle.py | 185 ++++++++++++++++++++++++++------------- noteman_slicer/cli.py | 43 ++++++++- tests/test_markers.py | 44 +++++++++- 5 files changed, 223 insertions(+), 66 deletions(-) diff --git a/docs/bundle-format.md b/docs/bundle-format.md index 56de2db..fb464ec 100644 --- a/docs/bundle-format.md +++ b/docs/bundle-format.md @@ -228,6 +228,14 @@ The archived document is the source of truth for reopening: the slice images are output, and a consumer that reopens a bundle re-renders them rather than importing them. +`source` is optional, so a bundle without one is still valid — it is simply not +reopenable, and a consumer should say so rather than pretend otherwise. What it +can still recover from such a bundle is the title block, and, if it cuts the +document again and happens to find exactly as many slices, the markers: the +slices array is in reading order, so it lines up with any other list in reading +order. One slice more or fewer and it does not, which is why that is a fallback +and not the design. + ## Engraving Most slices are photographs of print: an image and nothing more. A slice that diff --git a/docs/guide.md b/docs/guide.md index 14cb118..80aa830 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -117,6 +117,15 @@ It refuses to overwrite a PDF or project file that is already there, since the obvious place to unpack is exactly where someone's unfinished work lives — pass `--pdf elsewhere.pdf` or `--force` if you mean it. +A bundle from a producer that does not record its cuts can still be opened, but +it is a fresh session rather than a round trip: it asks first, then cuts the PDF +from scratch with detection. The title block always comes back. Markers and +re-engraved systems land only if detection happens to find exactly as many +slices as the bundle has — the slices are in reading order on both sides, so +they can be lined up, but one system found or missed would shift every marker +onto the wrong slice, so in that case they are left off entirely and it says so. +`--detect` answers the question in advance, for scripts. + ## Re-engraving a slice (optional, needs LilyPond) When a system is beyond rescue — a bad scan, a wrong transposition, a passage diff --git a/noteman_slicer/bundle.py b/noteman_slicer/bundle.py index e017ad6..c2ec474 100644 --- a/noteman_slicer/bundle.py +++ b/noteman_slicer/bundle.py @@ -203,48 +203,20 @@ def write(project: Project, source: Source, path: Path) -> Path: return path -def read(path: Path, into: Path | None = None, *, force: bool = False) -> tuple[Project, Path]: - """Unpack a bundle back into an editable project. Returns it and its PDF. +class NoCuts(ValueError): + """The bundle carries no `source` block, so its cuts cannot be restored.""" - The archived PDF is written out beside the bundle and becomes the project's - source again, because the PDF is what the pipeline renders from — the slice - images in the zip are output, and are discarded rather than re-imported. - The cuts, skew, levels and content rectangles come from the manifest's - `source` block, so this is a real round trip rather than a re-detection - that happens to land nearby. A bundle written without one cannot give them - back, and that is refused rather than guessed at. - """ - from .project import Cut, Marker, Page, Project, Replacement, Voice, default_path, hash_file - - path = Path(path) - with zipfile.ZipFile(path) as zf: - names = set(zf.namelist()) - if "song.json" not in names: - raise ValueError(f"{path.name} is not a bundle: no song.json") - manifest = json.loads(zf.read("song.json")) - if manifest.get("v") != FORMAT_VERSION: - raise ValueError(f"unsupported bundle version {manifest.get('v')!r}") - geometry = manifest.get("source") - if not geometry: - raise ValueError( - f"{path.name} carries no cuts — it was written by a producer that " - "does not record them, so the score would have to be cut again" - ) - pdf_name = geometry.get("file", "original.pdf") - if pdf_name not in names: - raise ValueError(f"{path.name} names {pdf_name} but does not contain it") - pdf_bytes = zf.read(pdf_name) - - pages_json = geometry["pages"] - slices = manifest["slices"] +def _pages_from(geometry: dict, slices: list[dict]): + """Rebuild the pages from a manifest's source geometry.""" + from .project import Cut, Page # Every slot on a page exists; the ones no slice claims were discarded. # That is the one thing the bundle states by omission rather than directly, # since shipping a discarded slice's image would defeat discarding it. - claimed = {(s["page"], s["slot"]): i for i, s in enumerate(slices)} + claimed = {(s["page"], s["slot"]) for s in slices} pages = [] - for i, page in enumerate(pages_json): + for i, page in enumerate(geometry["pages"]): cuts = [Cut([tuple(p) for p in cut]) for cut in page["cuts"]] count = len(cuts) + 1 pages.append( @@ -259,24 +231,51 @@ def read(path: Path, into: Path | None = None, *, force: bool = False) -> tuple[ levels=tuple(page["levels"]) if page.get("levels") else None, ) ) + return pages - position_of = {index: position for position, index in claimed.items()} - for entry in slices: - page, slot = entry["page"], entry["slot"] - pages[page].bars[slot] = entry.get("bar") - pages[page].markers[slot] = [ + +def _pages_from_detection(pdf: Path): + """Cut the PDF again from scratch, for a bundle that recorded no geometry.""" + from .detect import detect_page + from .pdf import open_source, page_raster + from .project import Project + + source = open_source(pdf) + detections, heights = [], [] + try: + for i in range(len(source)): + gray = page_raster(source, i) + detections.append(detect_page(gray)) + heights.append(gray.shape[0]) + finally: + source.close() + return Project.from_detection(pdf, detections, heights) + + +def _restore(project, slices: list[dict], positions: list[tuple[int, int]]) -> None: + """Put each slice's bar number, markers and engraving back on its slot.""" + from .project import Marker, Replacement, Voice + + for entry, (page, slot) in zip(slices, positions): + project.pages[page].bars[slot] = entry.get("bar") + project.pages[page].markers[slot] = [ Marker( type=marker["type"], label=marker.get("label"), # Back from an array index to the (page, slot) the editor works # in — the inverse of what export does. - destination=position_of.get(marker.get("destination")), + destination=( + positions[marker["destination"]] + if marker.get("destination") is not None + and marker["destination"] < len(positions) + else None + ), ) for marker in entry.get("markers", []) ] engraving = entry.get("engraving") if engraving and engraving.get("lang") == "lilypond": - pages[page].replacements[slot] = Replacement( + project.pages[page].replacements[slot] = Replacement( voices=[ Voice( clef=v.get("clef", "treble"), @@ -287,6 +286,63 @@ def read(path: Path, into: Path | None = None, *, force: bool = False) -> tuple[ ], print_time=engraving.get("print_time", False), ) + for entry in slices: + # Key and time are per slice in the bundle and per song here; the first + # engraving that states them is as good a song default as exists. + engraving = entry.get("engraving") or {} + if engraving.get("key"): + project.key = engraving["key"] + project.time = engraving.get("time", project.time) + break + + +def has_cuts(path: Path) -> bool: + """Whether this bundle records the geometry its slices were cut with. + + Worth asking before unpacking, since the answer decides whether reopening + is a round trip or a fresh session with the same PDF. + """ + with zipfile.ZipFile(Path(path)) as zf: + if "song.json" not in zf.namelist(): + return False + return bool(json.loads(zf.read("song.json")).get("source")) + + +def read( + path: Path, into: Path | None = None, *, force: bool = False, detect: bool = False +) -> tuple[Project, Path]: + """Unpack a bundle back into an editable project. Returns it and its PDF. + + The archived PDF is written out beside the bundle and becomes the project's + source again, because the PDF is what the pipeline renders from — the slice + images in the zip are output, and are discarded rather than re-imported. + + The cuts, skew, levels and content rectangles come from the manifest's + `source` block, so this is a real round trip rather than a re-detection + that happens to land nearby. A bundle written without one raises `NoCuts`; + `detect` says to cut the PDF from scratch instead, which is a different + thing and worth a caller asking about first. + """ + from .project import Project, default_path, hash_file + + path = Path(path) + with zipfile.ZipFile(path) as zf: + names = set(zf.namelist()) + if "song.json" not in names: + raise ValueError(f"{path.name} is not a bundle: no song.json") + manifest = json.loads(zf.read("song.json")) + if manifest.get("v") != FORMAT_VERSION: + raise ValueError(f"unsupported bundle version {manifest.get('v')!r}") + geometry = manifest.get("source") + if not geometry and not detect: + raise NoCuts( + f"{path.name} carries no cuts — it was written by a producer that " + "does not record them" + ) + pdf_name = (geometry or {}).get("file", "original.pdf") + if pdf_name not in names: + raise ValueError(f"{path.name} names {pdf_name} but does not contain it") + pdf_bytes = zf.read(pdf_name) # Unpacking writes two files. Refuse to land on either if it is already # there: the obvious place to open a bundle is next to the score it came @@ -300,25 +356,32 @@ def read(path: Path, into: Path | None = None, *, force: bool = False) -> tuple[ ) target.write_bytes(pdf_bytes) - metadata = { + slices = manifest["slices"] + if geometry: + pages_json = geometry["pages"] + first = pages_json[0] if pages_json else {} + project = Project( + source=target, + source_hash=hash_file(target), + pages=_pages_from(geometry, slices), + content_rect=tuple(first.get("content", (0.0, 0.0, 1.0, 1.0))), + levels=tuple(first.get("levels", (0, 255))), + path=default_path(target), + ) + positions = [(s["page"], s["slot"]) for s in slices] + else: + project = _pages_from_detection(target) + project.path = default_path(target) + # Detection's slices are in reading order and so are the bundle's, so + # they can be lined up — but only if there are exactly as many. One + # system found or missed shifts every marker onto the wrong slice, + # which is worse than not placing them at all. + positions = project.kept_slices() + if len(positions) != len(slices): + positions = [] + + project.metadata = { field: str(manifest[field]) for field in METADATA_FIELDS if manifest.get(field) is not None } - first = pages_json[0] if pages_json else {} - project = Project( - source=target, - source_hash=hash_file(target), - pages=pages, - content_rect=tuple(first.get("content", (0.0, 0.0, 1.0, 1.0))), - levels=tuple(first.get("levels", (0, 255))), - metadata=metadata, - path=default_path(target), - ) - # Key and time are per slice in the bundle and per song here; the first - # engraving that states them is as good a song default as exists. - for entry in slices: - engraving = entry.get("engraving") or {} - if engraving.get("key"): - project.key = engraving["key"] - project.time = engraving.get("time", project.time) - break + _restore(project, slices[: len(positions)], positions) return project, target diff --git a/noteman_slicer/cli.py b/noteman_slicer/cli.py index 2c0cd41..fe5433f 100644 --- a/noteman_slicer/cli.py +++ b/noteman_slicer/cli.py @@ -102,20 +102,50 @@ def _export(args: argparse.Namespace) -> int: return 0 +def _confirm(question: str) -> bool: + """Ask before doing something the caller did not ask for. No tty, no.""" + if not sys.stdin.isatty(): + print(f"{question} (not a terminal — pass --detect to say yes)") + return False + return input(f"{question} [y/N] ").strip().lower() in ("y", "yes") + + def _open(args: argparse.Namespace) -> int: from . import bundle from .editor import launch + zip_path, where = Path(args.zip), Path(args.pdf) if args.pdf else None try: - project, pdf = bundle.read( - Path(args.zip), Path(args.pdf) if args.pdf else None, force=args.force - ) + cuts = bundle.has_cuts(zip_path) + except (OSError, ValueError) as error: + print(f"cannot open: {error}") + return 1 + + if not cuts and not args.detect: + print(f"{zip_path.name} carries no cuts — it was written by a producer that") + print("does not record them. Its PDF can be cut again from scratch, but that") + print("is a fresh session: the cuts will be detection's, and the markers land") + print("only if detection happens to find the same number of slices.") + if not _confirm("Open it that way?"): + return 1 + + try: + project, pdf = bundle.read(zip_path, where, force=args.force, detect=not cuts) except (ValueError, KeyError) as error: print(f"cannot open: {error}") return 1 saved = project.save() - print(f"{pdf.name}: {len(project.pages)} pages, {len(project.kept_slices())} slices") + kept = project.kept_slices() + print(f"{pdf.name}: {len(project.pages)} pages, {len(kept)} slices") + if not cuts: + marked = sum(len(m) for page in project.pages for m in page.markers) + print(" cut from scratch by detection — check every cut before exporting") + print( + f" {marked} markers placed by position" + if marked + else " markers not placed: detection found a different number of slices" + ) print(f" project written to {saved.name}") if args.no_edit: return 0 @@ -182,6 +212,11 @@ def main(argv: list[str] | None = None) -> int: opn.add_argument( "--force", action="store_true", help="overwrite an existing PDF or project file" ) + opn.add_argument( + "--detect", + action="store_true", + help="for a bundle with no cuts: cut its PDF from scratch, without asking", + ) opn.set_defaults(func=_open) ed = sub.add_parser("edit", help="open the editor") diff --git a/tests/test_markers.py b/tests/test_markers.py index 52ee6c6..c2ca3a9 100644 --- a/tests/test_markers.py +++ b/tests/test_markers.py @@ -165,6 +165,38 @@ def main() -> int: assert back.bars[second] == 7 assert back.replacements[second].voices[0].lyrics == "la la" + # A bundle from a producer that records no cuts: refused by default, and + # cut from scratch by detection when the caller says so. The slices are in + # reading order either way, so markers can be lined up by position — but + # only when detection finds exactly as many. + plain = tmp / "plain.zip" + with zipfile.ZipFile(out) as src, zipfile.ZipFile(plain, "w") as dst: + for name in src.namelist(): + data = src.read(name) + if name == "song.json": + manifest = json.loads(data) + manifest.pop("source") + for entry in manifest["slices"]: + entry.pop("page", None) + entry.pop("slot", None) + data = json.dumps(manifest).encode() + dst.writestr(name, data) + assert bundle.has_cuts(out) and not bundle.has_cuts(plain) + try: + bundle.read(plain, tmp / "nocuts.pdf") + except bundle.NoCuts as error: + assert "no cuts" in str(error), error + else: + raise AssertionError("a bundle without cuts should not open silently") + + cut_again, again_pdf = bundle.read(plain, tmp / "nocuts.pdf", detect=True) + assert again_pdf.exists() + assert cut_again.metadata["title"] == "Test song", "the title block still comes back" + assert len(cut_again.kept_slices()) == len(reloaded.kept_slices()) + placed = [m for page in cut_again.pages for slot in page.markers for m in slot] + assert len(placed) == 3, placed + assert placed[0].label == "A" + # Unpacking never lands on files that are already there. try: bundle.read(out, tmp / "reopened.pdf") @@ -174,7 +206,17 @@ def main() -> int: raise AssertionError("reopening over an existing PDF should be refused") source.close() - for f in (pdf, out, saved, unpacked, default_path(unpacked), default_path(pdf)): + for f in ( + pdf, + out, + plain, + saved, + unpacked, + again_pdf, + default_path(unpacked), + default_path(again_pdf), + default_path(pdf), + ): f.unlink(missing_ok=True) tmp.rmdir() print("ok")