From abe4af5dacd27d6c6efeabaefda026398306ba89 Mon Sep 17 00:00:00 2001 From: Jackson Ferguson Date: Fri, 25 Sep 2026 15:13:31 -0700 Subject: [PATCH 1/3] fix(sync): regenerate a generated file in the run that drops its region When a region inside a generated file (such as a template's justfile recipes) stopped being declared, the generated writer skipped the whole file for that run and left the region step to retract it, so a changed generated text only landed one sync later and sync --check failed once right after a real sync. The writer now retracts omitted regions itself before merging: unedited ones are removed and cut from the whole-file baseline, deleted ones are forgotten, and settled ones are taken or kept. A region kept with its edit is the user's, so it sits out the three-way merge and is appended after it. Only a region still waiting for its decision holds the file. --- docs/development/semantic-reconciliation.md | 14 ++- src/protostar/appends.py | 77 ++++++++++++++++- src/protostar/reconciliation.py | 96 ++++++++++++++++----- tests/test_generated_execution.py | 81 +++++++++++++++++ 4 files changed, 239 insertions(+), 29 deletions(-) diff --git a/docs/development/semantic-reconciliation.md b/docs/development/semantic-reconciliation.md index 41403c4a..182bf33e 100644 --- a/docs/development/semantic-reconciliation.md +++ b/docs/development/semantic-reconciliation.md @@ -398,10 +398,16 @@ appends) record the complete desired text and also retain individual region texts. If overlapping edits prevent the whole-file merge, clean region updates can still apply independently. A pre-existing unowned generated target can own a newly appended region without -acquiring whole-file ownership. When a previously managed region is omitted, merge -mode skips whole-file regeneration for that run, because regenerating without the -region would drop it without its decision; the region step retracts it (and from -the whole-file baseline too), and the next run regenerates. +acquiring whole-file ownership. When a previously managed region is omitted, the +generated writer retracts it before regenerating, so the file converges in the +same run: an unedited region is removed from the file and cut from the whole-file +baseline, one the user already deleted is forgotten, and a settled one is taken +or kept. A region kept with its local edit is the user's, so it sits out the +three-way merge (a generated line changed next to it would otherwise overlap) and +is appended after the merged text. While an edited omitted region waits for its +decision, the writer leaves the file alone and the region step reports it; the +file regenerates in the run that settles it. A target where Protostar owns only +regions (`regions` policy) is left to the region step. ## PR G resolver and derived-artifact boundary diff --git a/src/protostar/appends.py b/src/protostar/appends.py index ee7bc1ad..8f24b581 100644 --- a/src/protostar/appends.py +++ b/src/protostar/appends.py @@ -1,6 +1,7 @@ """Generic marker-block file append engine.""" import re +from collections.abc import Iterable from dataclasses import dataclass, replace from pathlib import Path @@ -22,6 +23,9 @@ __all__ = [ "RegionResult", "append_marker_blocks", + "attach_regions", + "cut_regions", + "detach_regions", "get_comment_markers", ] @@ -105,6 +109,74 @@ class RegionResult: preserved: tuple[MergeConflict, ...] = () +def _marker(filepath: Path, tag: str, end: bool = False) -> str: + """Returns a region's begin or end marker in the file's comment syntax.""" + c_start, c_end = get_comment_markers(filepath) + prefix = "endregion" if end else "region" + suffix = f" {c_end}" if c_end else "" + return f"{c_start} {prefix}: protostar {tag}{suffix}".strip() + + +def cut_regions(text: str, identities: Iterable[str], filepath: Path) -> str: + """Removes regions from a text whatever they hold, such as a recorded baseline. + + Args: + text: Text whose regions are well formed. + identities: The regions to remove; absent ones are skipped. + filepath: The file, which selects the comment syntax of the markers. + + Returns: + The text without those regions, spaced as appending left it. + """ + for identity in identities: + tag = region_tag(identity) + begin, end = _marker(filepath, tag), _marker(filepath, tag, True) + if begin in text: + start = text.index(begin) + text = _cut(text, start, text.index(end, start) + len(end)) + return text + + +def detach_regions( + text: str, identities: Iterable[str], filepath: Path +) -> tuple[str, tuple[str, ...]]: + """Takes regions out of a text, to put back with ``attach_regions``. + + Args: + text: Text whose regions are well formed. + identities: The regions to take out; absent ones are skipped. + filepath: The file, which selects the comment syntax of the markers. + + Returns: + The text without those regions, and each region's framed text in order. + """ + blocks: list[tuple[int, str]] = [] + for identity in identities: + tag = region_tag(identity) + begin, end = _marker(filepath, tag), _marker(filepath, tag, True) + if begin in text: + start = text.index(begin) + blocks.append((start, text[start : text.index(end, start) + len(end)])) + detached = cut_regions(text, identities, filepath) + return detached, tuple(block for _, block in sorted(blocks)) + + +def attach_regions(text: str, blocks: Iterable[str]) -> str: + """Appends framed regions to a text the way a new region is appended. + + Args: + text: The text to extend. + blocks: Framed region texts, in order. + + Returns: + The text with each region appended after a blank line. + """ + for block in blocks: + separator = "" if not text else ("\n" if text.endswith("\n") else "\n\n") + text += separator + block + "\n" + return text + + def _cut(text: str, start: int, stop: int) -> str: """Removes a region and the line break after it, and the blank line it needed.""" if text[stop : stop + 2] == "\r\n": @@ -149,12 +221,9 @@ def append_marker_blocks( Returns: The reconciled content, applied region texts, and refused regions. """ - c_start, c_end = get_comment_markers(filepath) def marker(tag: str, end: bool = False) -> str: - prefix = "endregion" if end else "region" - suffix = f" {c_end}" if c_end else "" - return f"{c_start} {prefix}: protostar {tag}{suffix}".strip() + return _marker(filepath, tag, end) active: str | None = None seen: set[str] = set() diff --git a/src/protostar/reconciliation.py b/src/protostar/reconciliation.py index 137bab44..ee2259a1 100644 --- a/src/protostar/reconciliation.py +++ b/src/protostar/reconciliation.py @@ -10,7 +10,12 @@ from pathlib import Path from typing import cast -from .appends import append_marker_blocks +from .appends import ( + append_marker_blocks, + attach_regions, + cut_regions, + detach_regions, +) from .config import UserConfig from .dependencies import ( normalized_requirement, @@ -1049,16 +1054,13 @@ def _append_files(self) -> None: if record is not None and text_baseline is not None: # A generated file's text holds its regions; one that was cut # leaves that text too. - cut = { - r.id: r.baseline + cut = [ + r.id for r in record.regions if r.id not in region_result.baselines and r.baseline not in region_result.content - } - if cut: - text_baseline = append_marker_blocks( - text_baseline, [], target, baselines=cut - ).content + ] + text_baseline = cut_regions(text_baseline, cut, target) if region_result.baselines or ( record is not None and record.policy is FilePolicy.TEXT ): @@ -1218,11 +1220,14 @@ def _write_generated(self, target: Path, content: str) -> None: hint="Keep the tracked file policy unchanged.", ) contributions = self.manifest.filesystem.regions.get(target.as_posix(), []) - if record is not None and any( - r.id not in {c.id for c in contributions} for r in record.regions - ): - # Regenerating without an omitted region would drop it unasked. The - # region step retracts it first, so the next run regenerates. + declared = {c.id for c in contributions} + omitted = ( + {r.id: r.baseline for r in record.regions if r.id not in declared} + if record is not None + else {} + ) + if omitted and record is not None and record.policy is FilePolicy.REGIONS: + # Protostar owns only the regions here; the region step retracts them. return framed = append_marker_blocks( content, @@ -1255,30 +1260,79 @@ def _write_generated(self, target: Path, content: str) -> None: ) ) return + base = record.baseline if record else None + released: str | None = None + detached = "" + kept: tuple[str, ...] = () + if omitted: + # Regenerating without an omitted region would drop it unasked, + # so it is retracted first, and the file regenerates in this run. + try: + retraction = append_marker_blocks( + local.decode("utf-8") if local is not None else "", + [], + target, + baselines=omitted, + resolutions=self.resolutions, + ) + except UnicodeError as error: + raise FileSystemError( + "read generated file", str(target), error + ) from error + if retraction.conflicts: + # An edited region waits for its decision in the region + # step, which reports it; the file regenerates once settled. + return + self._report((), retraction.resolved) + if local is not None: + released = retraction.content + # A region kept with its local edit is the user's now. It + # sits out the merge, so a generated line changed next to + # it does not conflict, and is put back after it. + detached, kept = detach_regions(released, omitted, target) + local = detached.encode("utf-8") + base = cut_regions(base, omitted, target) if base is not None else None result = reconcile_text( local, content, - record.baseline if record else None, + base, MergeLocation(target.as_posix()), overwrite=self.manifest.collision_strategy is CollisionStrategy.OVERWRITE, resolutions=self.resolutions, ) self._report(result.conflicts, result.resolved, preserved=result.preserved) - if result.content is not None: - self.fs.write_text(target, result.content) - if result.baseline is not None: - regions = {r.id: r.baseline for r in record.regions} if record else {} + if released is None: + if result.content is not None: + self.fs.write_text(target, result.content) + else: + merged = ( + released + if result.conflicts + else attach_regions( + result.content if result.content is not None else detached, + kept, + ) + ) + if merged.encode("utf-8") != self.workspace.read_bytes(target): + self.fs.write_text(target, merged) + baseline = result.baseline if result.baseline is not None else base + if baseline is not None and (result.baseline is not None or omitted): + regions = ( + {r.id: r.baseline for r in record.regions if r.id in declared} + if record + else {} + ) if result.baseline == content: regions.update(framed.baselines) self.candidate_state = self.candidate_state.with_file( FileState( target.as_posix(), FilePolicy.TEXT, - result.baseline, + baseline, regions=tuple( - RegionState(region_tag(identity), identity, baseline) - for identity, baseline in regions.items() + RegionState(region_tag(identity), identity, text) + for identity, text in regions.items() ), ) ) diff --git a/tests/test_generated_execution.py b/tests/test_generated_execution.py index 1903e331..17763181 100644 --- a/tests/test_generated_execution.py +++ b/tests/test_generated_execution.py @@ -512,6 +512,87 @@ def setup(e): assert not record.regions +def test_generated_region_omission_regenerates_in_the_same_run( + tmp_path, monkeypatch, mocker +): + """A dropped region and a changed generated text converge in one run.""" + monkeypatch.chdir(tmp_path) + justfile_regions(mocker, "base v1\n", "recipe v1") + mocker.patch("protostar.reconciliation.generate_justfile", return_value="base v2\n") + + def generated(e): + e.manifest.tooling.wants_just = True + + run(mocker, generated) + + assert Path("justfile").read_text() == "base v2\n" + (record,) = deserialize_state(Path("protostar.lock").read_text()).files + assert record.baseline == "base v2\n" + assert not record.regions + assert not run(mocker, generated).journal.touched_paths + + +def _omitted_region_review(mocker, resolutions=None): + from protostar.preparation import prepare_review + + manifest = EnvironmentManifest() + manifest.tooling.wants_just = True + return manifest, prepare_review( + manifest, UserConfig(), resolutions=resolutions or {} + ) + + +def test_an_edited_omitted_region_holds_the_generated_file( + tmp_path, monkeypatch, mocker +): + """An edited region waits for its decision; the file regenerates once settled.""" + monkeypatch.chdir(tmp_path) + target = Path("justfile") + justfile_regions(mocker, "base v1\n", "recipe v1") + target.write_text(target.read_text().replace("recipe v1", "my recipe")) + mocker.patch("protostar.reconciliation.generate_justfile", return_value="base v2\n") + + _, review = _omitted_region_review(mocker) + (conflict,) = review.conflicts + assert conflict.reason is ConflictReason.RETRACTED + assert conflict.location.identity == "template:recipes" + assert not review.edits + + +@pytest.mark.parametrize( + ("choice", "kept"), + [(ResolutionChoice.LOCAL, True), (ResolutionChoice.DESIRED, False)], +) +def test_a_settled_omitted_region_regenerates_in_the_same_run( + tmp_path, monkeypatch, mocker, choice, kept +): + monkeypatch.chdir(tmp_path) + target = Path("justfile") + justfile_regions(mocker, "base v1\n", "recipe v1") + target.write_text(target.read_text().replace("recipe v1", "my recipe")) + mocker.patch("protostar.reconciliation.generate_justfile", return_value="base v2\n") + _, review = _omitted_region_review(mocker) + (conflict,) = review.conflicts + + manifest, review = _omitted_region_review(mocker, {conflict.id: choice}) + assert not review.conflicts + assert [c.id for c in review.resolved] == [conflict.id] + executor = SystemExecutor(manifest, UserConfig(), review=review) + mocker.patch.object(executor, "_check_ide_extensions") + executor.execute() + + text = target.read_text() + assert text.startswith("base v2\n") + assert ("my recipe" in text) is kept + (record,) = deserialize_state(Path("protostar.lock").read_text()).files + assert record.baseline == "base v2\n" + assert not record.regions + # The kept region is the user's: later runs leave it and ask nothing. + _, again = _omitted_region_review(mocker) + assert not again.conflicts + assert not again.edits + + def test_agents_md_region_merges_updates_and_protects_edits( tmp_path, monkeypatch, mocker ): From 9ab360d19a353e36a757ab450d7f336c0edd5027 Mon Sep 17 00:00:00 2001 From: Jackson Ferguson Date: Fri, 25 Sep 2026 15:14:10 -0700 Subject: [PATCH 2/3] feat(sync): release generated files whose tool is switched off Turning just or Docker off left justfile and Dockerfile behind, owned forever: their writers return early when the tool is off, the same hole #340 closed for structured documents. An owned generated file that no tool generates and no declared region targets is now released the way a seed is: deleted when its text matches the whole-file baseline, forgotten when already deleted, and otherwise a retracted conflict for the whole file, whose local choice keeps it as the user's and desired deletes it. --- docs/development/semantic-reconciliation.md | 8 +++ docs/usage/lifecycle.md | 4 +- src/protostar/manifest.py | 18 +++++-- src/protostar/preparation.py | 1 + src/protostar/reconciliation.py | 57 ++++++++++++++++++++- tests/test_generated_execution.py | 32 ++++++++++++ tests/test_lifecycle.py | 52 +++++++++++++++++++ 7 files changed, 166 insertions(+), 6 deletions(-) diff --git a/docs/development/semantic-reconciliation.md b/docs/development/semantic-reconciliation.md index 182bf33e..bb1802a6 100644 --- a/docs/development/semantic-reconciliation.md +++ b/docs/development/semantic-reconciliation.md @@ -409,6 +409,14 @@ decision, the writer leaves the file alone and the region step reports it; the file regenerates in the run that settles it. A target where Protostar owns only regions (`regions` policy) is left to the region step. +A generated file whose tool is switched off (`EnvironmentManifest.generated_files` +no longer lists it) and that receives no declared region is released the way a +seed is, by `Reconciliation._release_undeclared_generated`: deleted when its text +matches the whole-file baseline, forgotten when already deleted, and otherwise a +`retracted` conflict for the whole file, whose `local` choice keeps it as the +user's and `desired` deletes it. Generated text has no units to keep apart, so +the decision is never split into hunks. + ## PR G resolver and derived-artifact boundary System tasks establish the local project first (`uv init` when needed). Structured diff --git a/docs/usage/lifecycle.md b/docs/usage/lifecycle.md index 8e89e4f9..6f00c775 100644 --- a/docs/usage/lifecycle.md +++ b/docs/usage/lifecycle.md @@ -131,7 +131,9 @@ A whole configuration file whose tool you turn off, such as `zensical.toml`, `.github/codecov.yml`, `.readthedocs.yaml`, a workflow, the hook configuration, Renovate's, or the VS Code settings, is retracted key by key the same way. When nothing of Protostar's is left, the file is deleted if it holds nothing else and -kept with only your own keys if it does. +kept with only your own keys if it does. A generated `justfile` or `Dockerfile` +whose tool you turn off is deleted when unedited; an edited one is kept with a +`retracted` conflict for the whole file. Seeded project metadata such as `[project].name` and dependency-group includes are never retracted. Workflow files are merged by job and by step name, so your own jobs, steps, triggers, and inputs stay. diff --git a/src/protostar/manifest.py b/src/protostar/manifest.py index 3a1c07f0..128c1e63 100644 --- a/src/protostar/manifest.py +++ b/src/protostar/manifest.py @@ -685,6 +685,19 @@ def declared_documents(self) -> set[str]: path for target in targets for path in self.document_locations(target).paths } + def generated_files(self) -> set[str]: + """Returns the text files a tool generates whole in this run. + + Returns: + Workspace-relative POSIX paths. + """ + files: set[str] = set() + if self.tooling.wants_just: + files.add("justfile") + if self.tooling.wants_docker: + files.add(DOCKERFILE) + return files + def target_files(self) -> set[Path]: """Returns all concrete workspace file paths that this manifest intends to create or mutate. @@ -721,11 +734,8 @@ def target_files(self) -> set[Path]: if self.tooling.wants_release: targets.add(Path(github_workflows.RELEASE_TARGET)) - if self.tooling.wants_just: - targets.add(Path("justfile")) - + targets.update(Path(path) for path in self.generated_files()) if self.tooling.wants_docker: - targets.add(Path(DOCKERFILE)) targets.add(Path(".dockerignore")) return targets diff --git a/src/protostar/preparation.py b/src/protostar/preparation.py index 75208b97..3d0fc8b0 100644 --- a/src/protostar/preparation.py +++ b/src/protostar/preparation.py @@ -418,6 +418,7 @@ def prepare_review( decisions._release_undeclared_seeds() decisions._settle_retired() decisions._release_undeclared_documents() + decisions._release_undeclared_generated() decisions._create_directories() decisions._write_injected_files() decisions._write_pre_commit_config() diff --git a/src/protostar/reconciliation.py b/src/protostar/reconciliation.py index ee2259a1..f8844b63 100644 --- a/src/protostar/reconciliation.py +++ b/src/protostar/reconciliation.py @@ -96,7 +96,7 @@ deserialize_state, encode_toml_baseline, ) -from .text_merge import reconcile_text +from .text_merge import is_edited, reconcile_text from .toml_ast import ( TomlDocumentSpec, TomlReconciliation, @@ -701,6 +701,61 @@ def _release_undeclared_documents(self) -> None: "retract configuration", record.path, error ) from error + def _release_undeclared_generated(self) -> None: + """Lets go of each owned generated file that nothing declares any more. + + A tool switched off stops generating its file. It goes the way a seed + does: deleted when unedited, and otherwise a ``retracted`` conflict for + the whole file, since generated text has no units to keep apart. The + conflict's ``local`` choice keeps the file as the user's; ``desired`` + deletes it. A file already deleted is forgotten. A file that still + receives a declared region is declared, and keeps its generated text. + """ + declared = self.manifest.generated_files() | { + Path(render_template(path, self.interpolation_context)).as_posix() + for path in self.manifest.filesystem.regions + } + for record in [ + r + for r in self.candidate_state.files + if r.policy is FilePolicy.TEXT and r.path not in declared + ]: + target = Path(record.path) + enforce_path_jail(target, Path.cwd()) + self._validate_node(target) + if not self.workspace.exists(target): + self.candidate_state = self.candidate_state.without_file(record.path) + continue + local = self.workspace.read_bytes(target) + if is_edited(local, record.baseline or ""): + conflict = MergeConflict( + MergeLocation(record.path), + ConflictReason.RETRACTED, + ConflictSides( + record.baseline or MISSING, + local.decode("utf-8", "replace"), + MISSING, + line=0, + ), + ) + settled = conflict.settle(self.resolutions) + if settled is None: + self._report((conflict,), ()) + continue + self._report((), (settled,)) + if settled.resolution is ResolutionChoice.LOCAL: + self.candidate_state = self.candidate_state.without_file( + record.path + ) + continue + try: + self.fs.remove_file(target) + except OSError as error: + raise FileSystemError( + "remove generated file", record.path, error + ) from error + self.candidate_state = self.candidate_state.without_file(record.path) + def _release_document(self, path: str) -> None: """Forgets a structured document's ownership and the hook pins it held.""" self.candidate_state = replace( diff --git a/tests/test_generated_execution.py b/tests/test_generated_execution.py index 17763181..be74eacf 100644 --- a/tests/test_generated_execution.py +++ b/tests/test_generated_execution.py @@ -37,6 +37,10 @@ def setup(executor): "protostar.reconciliation.Reconciliation._write_ci_workflow", lambda decisions: decisions._write_generated(target, value), ) + # Declared as generated, so the file is not released as undeclared. + mocker.patch.object( + EnvironmentManifest, "generated_files", return_value={target.as_posix()} + ) return run(mocker, setup) @@ -593,6 +597,34 @@ def test_a_settled_omitted_region_regenerates_in_the_same_run( assert not again.edits +def test_a_dockerfile_leaves_when_docker_is_switched_off(tmp_path, monkeypatch, mocker): + monkeypatch.chdir(tmp_path) + run(mocker, lambda e: setattr(e.manifest.tooling, "wants_docker", True)) + assert Path("Dockerfile").exists() + + run(mocker, lambda e: None) + + assert not Path("Dockerfile").exists() + state = deserialize_state(Path("protostar.lock").read_text()) + assert "Dockerfile" not in {r.path for r in state.files} + + +def test_a_generated_file_that_still_receives_a_region_stays( + tmp_path, monkeypatch, mocker +): + """A declared region keeps its file declared, generated text and all.""" + monkeypatch.chdir(tmp_path) + justfile_regions(mocker, "base\n", "recipe") + + def region_only(e): + e.manifest.filesystem.add_region( + "justfile", "recipe", identity="template:recipes" + ) + + assert not run(mocker, region_only).journal.touched_paths + assert Path("justfile").read_text().startswith("base\n") + + def test_agents_md_region_merges_updates_and_protects_edits( tmp_path, monkeypatch, mocker ): diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index 2c90577c..edf03ccf 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -1262,6 +1262,58 @@ def test_sync_reports_a_retracted_document_removal(project, monkeypatch, capsys) assert not Path(".github/renovate.json").exists() +def test_a_generated_file_whose_tool_is_switched_off_is_released(project, mocker): + """An unedited justfile leaves with just; one already deleted is forgotten.""" + mocker.patch( + "protostar.system.ProcessRunner.run", autospec=True, side_effect=_any_resolved + ) + reprepare = _released(project, "just = true\n", "just = false\n") + assert Path("justfile").exists() + reprepare().apply() + + assert not Path("justfile").exists() + assert "justfile" not in {r.path for r in _state().files} + + reprepare = _released(project, "just = true\n", "just = false\n") + Path("justfile").unlink() + prepared = reprepare() + assert not prepared.review.conflicts + prepared.apply() + assert "justfile" not in {r.path for r in _state().files} + + +@pytest.mark.parametrize( + ("choice", "kept"), + [(ResolutionChoice.LOCAL, True), (ResolutionChoice.DESIRED, False)], +) +def test_an_edited_generated_file_whose_tool_is_switched_off_is_retracted( + project, mocker, choice, kept +): + from protostar.lifecycle import prepare_project + + mocker.patch( + "protostar.system.ProcessRunner.run", autospec=True, side_effect=_any_resolved + ) + reprepare = _released(project, "just = true\n", "just = false\n") + edited = Path("justfile").read_text() + "\nmine:\n echo mine\n" + Path("justfile").write_text(edited) + prepared = reprepare() + + (conflict,) = prepared.review.conflicts + assert conflict.location.file == "justfile" + assert conflict.reason is ConflictReason.RETRACTED + prepared.apply() + assert Path("justfile").read_text() == edited + assert "justfile" in {r.path for r in _state().files} + + retracted = prepare_project() + (conflict,) = retracted.review.conflicts + retracted.resolve({conflict.id: choice}).apply() + assert Path("justfile").exists() is kept + assert "justfile" not in {r.path for r in _state().files} + assert not prepare_project().review.conflicts + + def _add_resolved(_runner, command, *, timeout): """Stands in for `uv add` and `uv lock` on the main group.""" import tomlkit From 0b5d6d03ffb1f1032975b1956ce3d2186b36342c Mon Sep 17 00:00:00 2001 From: Jackson Ferguson Date: Fri, 25 Sep 2026 15:55:05 -0700 Subject: [PATCH 3/3] fix(sync): preserve CRLF line endings when cutting and attaching regions --- src/protostar/appends.py | 19 ++++++++++++++--- tests/test_appends.py | 46 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/protostar/appends.py b/src/protostar/appends.py index 8f24b581..44bcbf2c 100644 --- a/src/protostar/appends.py +++ b/src/protostar/appends.py @@ -172,8 +172,13 @@ def attach_regions(text: str, blocks: Iterable[str]) -> str: The text with each region appended after a blank line. """ for block in blocks: - separator = "" if not text else ("\n" if text.endswith("\n") else "\n\n") - text += separator + block + "\n" + newline = "\r\n" if "\r\n" in text or "\r\n" in block else "\n" + separator = ( + "" + if not text + else (newline if text.endswith(("\r\n", "\n")) else newline + newline) + ) + text += separator + block + newline return text @@ -185,8 +190,16 @@ def _cut(text: str, start: int, stop: int) -> str: stop += 1 head, tail = text[:start], text[stop:] # Appending put one blank line before the region; one is enough. - if head.endswith("\n\n") and (not tail or tail.startswith("\n")): + if head.endswith(("\r\n\r\n", "\n\r\n")) and ( + not tail or tail.startswith(("\r\n", "\n")) + ): + head = head[:-2] + elif head.endswith(("\r\n\n", "\n\n")) and ( + not tail or tail.startswith(("\r\n", "\n")) + ): head = head[:-1] + elif not head and tail.startswith("\r\n"): + tail = tail[2:] elif not head and tail.startswith("\n"): tail = tail[1:] return head + tail diff --git a/tests/test_appends.py b/tests/test_appends.py index e386955b..d21e870f 100644 --- a/tests/test_appends.py +++ b/tests/test_appends.py @@ -1,7 +1,12 @@ from pathlib import Path -from protostar.appends import append_marker_blocks, get_comment_markers -from protostar.intent import AppendContribution +from protostar.appends import ( + append_marker_blocks, + attach_regions, + detach_regions, + get_comment_markers, +) +from protostar.intent import AppendContribution, region_tag from protostar.merge import ConflictReason, ResolutionChoice @@ -185,3 +190,40 @@ def test_a_deleted_region_nothing_declares_is_forgotten(): assert result.content == "export A=1\n" assert result.baselines == {} + + +def test_detach_and_attach_regions_preserves_crlf(): + tag = region_tag("template:recipes") + text = ( + f"base v1\r\n\r\n" + f"# region: protostar {tag}\r\n" + f"recipe v1\r\n" + f"# endregion: protostar {tag}\r\n" + ) + filepath = Path("justfile") + omitted = {"template:recipes": "recipe v1"} + detached, kept = detach_regions(text, omitted, filepath) + assert detached == "base v1\r\n" + re_attached = attach_regions("base v2\r\n", kept) + assert re_attached == ( + f"base v2\r\n\r\n" + f"# region: protostar {tag}\r\n" + f"recipe v1\r\n" + f"# endregion: protostar {tag}\r\n" + ) + + +def test_an_omitted_region_retracts_cleanly_under_crlf(): + tag = region_tag("gone") + content = ( + f"export A=1\r\n\r\n" + f"# region: protostar {tag}\r\n" + f"export B=2\r\n" + f"# endregion: protostar {tag}\r\n" + ) + baselines = { + "gone": f"# region: protostar {tag}\nexport B=2\n# endregion: protostar {tag}" + } + result = append_marker_blocks(content, [], Path(".envrc"), baselines=baselines) + assert result.content == "export A=1\r\n" + assert result.baselines == {}