From 253e7044415578898f314cd3b86ae9c83d44b64e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Apr 2026 19:30:08 +0000 Subject: [PATCH 1/9] Implement selective and in-place freeze (#1063) - Add optional project name arguments to `dfetch freeze` so users can freeze individual projects rather than all at once. - When the manifest lives inside a git or SVN superproject, freeze now edits the manifest file in-place (preserving comments, blank lines and indentation) instead of creating a `.backup` copy and regenerating from scratch. - Add `update_project_in_manifest_file` and helper functions in `dfetch/manifest/manifest.py` for in-place YAML text editing. - Add BDD feature files: `freeze-specific-projects.feature` and `freeze-inplace.feature`, plus the required step definitions. - Add unit tests for all new in-place editing helpers. - Add CHANGELOG entry for v0.14.0 (unreleased). https://claude.ai/code/session_01Xd8EcAUkSoJo9YZimGzEuA --- CHANGELOG.rst | 6 + dfetch/commands/freeze.py | 69 +++++--- dfetch/manifest/manifest.py | 200 ++++++++++++++++++++++ features/freeze-inplace.feature | 68 ++++++++ features/freeze-specific-projects.feature | 39 +++++ features/steps/manifest_steps.py | 16 ++ tests/test_manifest.py | 176 +++++++++++++++++++ 7 files changed, 552 insertions(+), 22 deletions(-) create mode 100644 features/freeze-inplace.feature create mode 100644 features/freeze-specific-projects.feature diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2bb885f2..133186ce 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,3 +1,9 @@ +Release 0.14.0 (unreleased) +=========================== + +* Allow ``dfetch freeze`` to accept project names to freeze only specific projects (#1063) +* Edit manifest in-place when freezing inside a git or SVN superproject, preserving comments and layout (#1063) + Release 0.13.0 (released 2026-03-30) ==================================== diff --git a/dfetch/commands/freeze.py b/dfetch/commands/freeze.py index d057c488..db61b4b1 100644 --- a/dfetch/commands/freeze.py +++ b/dfetch/commands/freeze.py @@ -18,7 +18,11 @@ When your project becomes stable and you want to rely on a specific version of ``mymodule`` you can run ``dfetch freeze``. -First *DFetch* will rename your old manifest (appended with ``.backup``). +When the manifest lives inside a git or SVN super-project, *DFetch* edits the +manifest file **in-place** so that comments, blank lines and indentation are +preserved. Only the version fields that changed are touched. + +Otherwise *DFetch* first renames your old manifest (appended with ``.backup``). After that a new manifest is generated with all the projects as in your original manifest, but each with the specific version as it currently is on disk. @@ -34,8 +38,16 @@ url: http://git.mycompany.local/mycompany/mymodule tag: v1.0.0 +You can also freeze a subset of projects by listing their names: + +.. code-block:: sh + + dfetch freeze mymodule + .. scenario-include:: ../features/freeze-projects.feature +.. scenario-include:: ../features/freeze-specific-projects.feature + For archive projects, ``dfetch freeze`` adds the hash under the nested ``integrity.hash`` key (e.g. ``integrity.hash: sha256:``) to pin the exact archive content used. This value acts as the version identifier: @@ -44,6 +56,8 @@ .. scenario-include:: ../features/freeze-archive.feature +.. scenario-include:: ../features/freeze-inplace.feature + """ import argparse @@ -55,9 +69,9 @@ import dfetch.project from dfetch import DEFAULT_MANIFEST_NAME from dfetch.log import get_logger -from dfetch.manifest.manifest import Manifest -from dfetch.manifest.project import ProjectEntry +from dfetch.manifest.manifest import Manifest, update_project_in_manifest_file from dfetch.project import create_super_project +from dfetch.project.superproject import NoVcsSuperProject from dfetch.util.util import catch_runtime_exceptions, in_directory logger = get_logger(__name__) @@ -67,24 +81,30 @@ class Freeze(dfetch.commands.command.Command): """Freeze your projects versions in the manifest as they are on disk. Generate a new manifest that has all version as they are on disk. + Optionally pass one or more project names to freeze only those projects. """ @staticmethod def create_menu(subparsers: dfetch.commands.command.SubparserActionType) -> None: """Add the parser menu for this action.""" - dfetch.commands.command.Command.parser(subparsers, Freeze) + parser = dfetch.commands.command.Command.parser(subparsers, Freeze) + parser.add_argument( + "projects", + metavar="", + type=str, + nargs="*", + help="Specific project(s) to freeze (default: all projects in manifest)", + ) def __call__(self, args: argparse.Namespace) -> None: """Perform the freeze.""" - del args # unused - superproject = create_super_project() + use_inplace = not isinstance(superproject, NoVcsSuperProject) exceptions: list[str] = [] - projects: list[ProjectEntry] = [] with in_directory(superproject.root_directory): - for project in superproject.manifest.projects: + for project in superproject.manifest.selected_projects(args.projects): with catch_runtime_exceptions(exceptions) as exceptions: sub_project = dfetch.project.create_sub_project(project) on_disk_version = sub_project.on_disk_version() @@ -106,18 +126,23 @@ def __call__(self, args: argparse.Namespace) -> None: project.name, f"Frozen on version {new_version}", ) + if use_inplace: + update_project_in_manifest_file( + project, superproject.manifest.path + ) - projects.append(project) - - manifest = Manifest( - { - "version": "0.0", - "remotes": superproject.manifest.remotes, - "projects": projects, - } - ) - - shutil.move(DEFAULT_MANIFEST_NAME, DEFAULT_MANIFEST_NAME + ".backup") - - manifest.dump(DEFAULT_MANIFEST_NAME) - logger.info(f"Updated manifest ({DEFAULT_MANIFEST_NAME}) in {os.getcwd()}") + if not use_inplace: + manifest = Manifest( + { + "version": "0.0", + "remotes": superproject.manifest.remotes, + "projects": superproject.manifest.projects, + } + ) + + shutil.move(DEFAULT_MANIFEST_NAME, DEFAULT_MANIFEST_NAME + ".backup") + + manifest.dump(DEFAULT_MANIFEST_NAME) + logger.info( + f"Updated manifest ({DEFAULT_MANIFEST_NAME}) in {os.getcwd()}" + ) diff --git a/dfetch/manifest/manifest.py b/dfetch/manifest/manifest.py index aff40963..8b15cf6d 100644 --- a/dfetch/manifest/manifest.py +++ b/dfetch/manifest/manifest.py @@ -483,3 +483,203 @@ def append_entry_manifest_file( manifest_file.write("\n") for line in new_entry.splitlines(): manifest_file.write(f" {line}\n") + + +def update_project_in_manifest_file( + project: ProjectEntry, + manifest_path: str | Path, +) -> None: + """Update a project's version fields in the manifest file, preserving layout and comments. + + This is used when the manifest is in a version-controlled superproject: instead of + creating a backup and regenerating the file from scratch, the existing file is edited + in-place so that comments and formatting are retained. + + Args: + project: The ``ProjectEntry`` whose version fields have already been updated by + ``freeze_project()``. + manifest_path: Path to the manifest file to update. + """ + path = Path(manifest_path) + text = path.read_text(encoding="utf-8") + updated = _update_project_version_in_text(text, project) + path.write_text(updated, encoding="utf-8") + + +def _yaml_scalar(value: str) -> str: + """Return the YAML inline representation of a scalar string value. + + Strings that look like integers (e.g. SVN revision ``'176'``) are quoted so + that YAML round-trips them back as strings. + """ + dumped: str = yaml.dump(value, default_flow_style=None, allow_unicode=True) + return dumped.strip() + + +def _find_project_block(lines: list[str], project_name: str) -> tuple[int, int, int]: + """Return ``(start, end, item_indent)`` for the named project's YAML block. + + *start* is the index of the ``- name: `` line. + *end* is the exclusive end index (first line that belongs to the next block). + *item_indent* is the column of the ``-`` character. + + Raises: + RuntimeError: if the project name is not found. + """ + start: int | None = None + item_indent = 0 + + for i, line in enumerate(lines): + m = re.match( + r"^(\s*)-\s+name:\s*" + re.escape(project_name) + r"\s*$", + line.rstrip("\n\r"), + ) + if m: + start = i + item_indent = len(m.group(1)) + break + + if start is None: + raise RuntimeError(f"Project '{project_name}' not found in manifest text") + + end = len(lines) + for i in range(start + 1, len(lines)): + line = lines[i] + if not line.strip(): + continue + indent = len(line) - len(line.lstrip()) + if indent <= item_indent: + end = i + break + + return start, end, item_indent + + +def _set_simple_field_in_block( + block: list[str], field_indent: int, field_name: str, yaml_value: str +) -> list[str]: + """Update an existing ``field_name:`` line in *block*, or insert one after the first line. + + *field_indent* is the expected column for fields in this block. + *yaml_value* is the already-serialised YAML scalar (e.g. ``e1fda...`` or ``'176'``). + """ + block = list(block) + field_re = re.compile(r"^(\s+)" + re.escape(field_name) + r":\s*.*$") + + for i, line in enumerate(block): + if field_re.match(line.rstrip("\n\r")): + eol = "\n" if line.endswith("\n") else "" + block[i] = " " * field_indent + field_name + ": " + yaml_value + eol + return block + + # Field not present — insert right after the ``name:`` line (position 1). + new_line = " " * field_indent + field_name + ": " + yaml_value + "\n" + block.insert(1, new_line) + return block + + +def _set_integrity_hash_in_block( + block: list[str], field_indent: int, hash_value: str +) -> list[str]: + """Update or insert ``integrity.hash`` inside *block*.""" + block = list(block) + integrity_re = re.compile(r"^(\s+)integrity:\s*$") + hash_re = re.compile(r"^(\s+)hash:\s*.*$") + + # Locate an existing ``integrity:`` mapping. + integrity_line: int | None = None + for i, line in enumerate(block): + if integrity_re.match(line.rstrip("\n\r")): + integrity_line = i + break + + if integrity_line is not None: + # Try to find an existing ``hash:`` nested inside it. + for i in range(integrity_line + 1, len(block)): + stripped = block[i].strip() + if not stripped: + continue + if hash_re.match(block[i].rstrip("\n\r")): + eol = "\n" if block[i].endswith("\n") else "" + block[i] = " " * (field_indent + 2) + "hash: " + hash_value + eol + return block + # Stop searching once we leave the integrity sub-block. + indent = len(block[i]) - len(block[i].lstrip()) + if indent <= field_indent: + break + # No ``hash:`` found — insert right after ``integrity:``. + block.insert( + integrity_line + 1, " " * (field_indent + 2) + "hash: " + hash_value + "\n" + ) + return block + + # No ``integrity:`` block — append it before any trailing blank lines. + new_lines = [ + " " * field_indent + "integrity:\n", + " " * (field_indent + 2) + "hash: " + hash_value + "\n", + ] + insert_at = len(block) + for i in range(len(block) - 1, -1, -1): + if block[i].strip(): + insert_at = i + 1 + break + block[insert_at:insert_at] = new_lines + return block + + +def _update_project_version_in_text(text: str, project: ProjectEntry) -> str: + """Return *text* with the version fields for *project* updated in-place. + + Only version-related fields (``revision``, ``tag``, ``branch``, + ``integrity.hash``) are touched; all other content — including comments + and indentation — is preserved verbatim. + """ + project_yaml = project.as_yaml() + + # Collect simple version fields that are now set. + fields_to_set: list[tuple[str, str]] = [] + for field in ("revision", "tag", "branch"): + value = project_yaml.get(field) + if value: + fields_to_set.append((field, _yaml_scalar(str(value)))) + + integrity = project_yaml.get("integrity") + integrity_hash: str = ( + integrity["hash"] + if isinstance(integrity, dict) and integrity.get("hash") + else "" + ) + + if not fields_to_set and not integrity_hash: + return text + + lines = text.splitlines(keepends=True) + start, end, item_indent = _find_project_block(lines, project.name) + field_indent = item_indent + 2 + + block = list(lines[start:end]) + + # Track which fields were inserted (not just updated) so we can keep them + # in a sensible order right after the ``name:`` line. + inserted: list[tuple[str, str]] = [] + for field_name, yaml_value in fields_to_set: + field_re = re.compile(r"^(\s+)" + re.escape(field_name) + r":\s*.*$") + found = any(field_re.match(line.rstrip("\n\r")) for line in block) + if found: + block = _set_simple_field_in_block( + block, field_indent, field_name, yaml_value + ) + else: + inserted.append((field_name, yaml_value)) + + # Insert all new fields together right after ``name:`` (position 1). + if inserted: + new_lines = [ + " " * field_indent + fname + ": " + val + "\n" for fname, val in inserted + ] + block[1:1] = new_lines + + if integrity_hash: + block = _set_integrity_hash_in_block(block, field_indent, integrity_hash) + + return "".join(lines[:start] + block + lines[end:]) diff --git a/features/freeze-inplace.feature b/features/freeze-inplace.feature new file mode 100644 index 00000000..069a1303 --- /dev/null +++ b/features/freeze-inplace.feature @@ -0,0 +1,68 @@ +Feature: Freeze manifest in-place inside a version-controlled superproject + + When the manifest lives inside a git or SVN superproject, ``dfetch freeze`` + edits the manifest file **in-place** instead of creating a ``.backup`` copy. + Comments, blank lines, and the original indentation are all preserved. + + Scenario: Git projects are frozen in-place preserving layout + Given a local git repo "superproject" with the manifest + """ + manifest: + version: '0.0' + + projects: + - name: ext/test-repo-tag + url: https://github.com/dfetch-org/test-repo + branch: main + + """ + And all projects are updated in superproject + When I run "dfetch freeze" in superproject + Then the manifest 'dfetch.yaml' in superproject is replaced with + """ + manifest: + version: '0.0' + + projects: + - name: ext/test-repo-tag + revision: e1fda19a57b873eb8e6ae37780594cbb77b70f1a + url: https://github.com/dfetch-org/test-repo + branch: main + + """ + And no file 'dfetch.yaml.backup' exists in superproject + + Scenario: Only selected project is frozen in-place + Given a local git repo "superproject2" with the manifest + """ + manifest: + version: '0.0' + + projects: + - name: ext/test-repo-tag + url: https://github.com/dfetch-org/test-repo + branch: main + + - name: ext/test-repo-tag2 + url: https://github.com/dfetch-org/test-repo + branch: main + + """ + And all projects are updated in superproject2 + When I run "dfetch freeze ext/test-repo-tag" in superproject2 + Then the manifest 'dfetch.yaml' in superproject2 is replaced with + """ + manifest: + version: '0.0' + + projects: + - name: ext/test-repo-tag + revision: e1fda19a57b873eb8e6ae37780594cbb77b70f1a + url: https://github.com/dfetch-org/test-repo + branch: main + + - name: ext/test-repo-tag2 + url: https://github.com/dfetch-org/test-repo + branch: main + + """ diff --git a/features/freeze-specific-projects.feature b/features/freeze-specific-projects.feature new file mode 100644 index 00000000..20416596 --- /dev/null +++ b/features/freeze-specific-projects.feature @@ -0,0 +1,39 @@ +Feature: Freeze specific projects + + *DFetch* can freeze specific projects by name, leaving other projects untouched. + This is useful when only some dependencies need to be pinned. + + Scenario: Single project is frozen while another is skipped + Given the manifest 'dfetch.yaml' + """ + manifest: + version: '0.0' + + projects: + - name: ext/test-repo-tag + url: https://github.com/dfetch-org/test-repo + branch: main + + - name: ext/test-repo-tag2 + url: https://github.com/dfetch-org/test-repo + branch: main + + """ + And all projects are updated + When I run "dfetch freeze ext/test-repo-tag" + Then the manifest 'dfetch.yaml' is replaced with + """ + manifest: + version: '0.0' + + projects: + - name: ext/test-repo-tag + revision: e1fda19a57b873eb8e6ae37780594cbb77b70f1a + url: https://github.com/dfetch-org/test-repo + branch: main + + - name: ext/test-repo-tag2 + url: https://github.com/dfetch-org/test-repo + branch: main + + """ diff --git a/features/steps/manifest_steps.py b/features/steps/manifest_steps.py index 30d0c2f4..5afa41a7 100644 --- a/features/steps/manifest_steps.py +++ b/features/steps/manifest_steps.py @@ -52,6 +52,22 @@ def step_impl(context, name): check_file(name, apply_manifest_substitutions(context, context.text)) +@then("the manifest '{name}' in {directory} is replaced with") +def step_impl(context, name, directory): + """Check a manifest located in a subdirectory.""" + check_file( + os.path.join(directory, name), + apply_manifest_substitutions(context, context.text), + ) + + +@then("no file '{name}' exists in {directory}") +def step_impl(_, name, directory): + """Assert that a file does not exist in the given directory.""" + path = os.path.join(directory, name) + assert not os.path.exists(path), f"Expected '{path}' to not exist, but it does!" + + @given("the manifest '{name}' with the projects:") def step_impl(context, name): projects = "\n".join(f" - name: {row['name']}" for row in context.table) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 87d7ea57..42adbda5 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -14,6 +14,10 @@ ManifestDict, ManifestEntryLocation, RequestedProjectNotFoundError, + _find_project_block, + _set_simple_field_in_block, + _set_integrity_hash_in_block, + _update_project_version_in_text, ) from dfetch.manifest.parse import find_manifest, get_submanifests from dfetch.manifest.project import ProjectEntry @@ -245,3 +249,175 @@ def test_validate_destination_rejects_dotdot() -> None: def test_validate_destination_accepts_relative() -> None: Manifest.validate_destination("external/mylib") # must NOT raise + + +# --------------------------------------------------------------------------- +# In-place manifest text editing helpers +# --------------------------------------------------------------------------- + +_SIMPLE_MANIFEST = """\ +manifest: + version: '0.0' + + projects: + - name: myproject + url: https://example.com/myproject + branch: main +""" + +_TWO_PROJECT_MANIFEST = """\ +manifest: + version: '0.0' + + projects: + - name: first + url: https://example.com/first + branch: main + + - name: second + url: https://example.com/second + branch: develop +""" + + +# --- _find_project_block --------------------------------------------------- + + +def test_find_project_block_single() -> None: + lines = _SIMPLE_MANIFEST.splitlines(keepends=True) + start, end, item_indent = _find_project_block(lines, "myproject") + assert item_indent == 4 + assert lines[start].startswith(" - name: myproject") + # end should point past the block + assert end == len(lines) + + +def test_find_project_block_first_of_two() -> None: + lines = _TWO_PROJECT_MANIFEST.splitlines(keepends=True) + start, end, item_indent = _find_project_block(lines, "first") + assert item_indent == 4 + assert lines[start].startswith(" - name: first") + # The blank line between projects is excluded from the block + block_text = "".join(lines[start:end]) + assert "second" not in block_text + + +def test_find_project_block_second_of_two() -> None: + lines = _TWO_PROJECT_MANIFEST.splitlines(keepends=True) + start, _end, _indent = _find_project_block(lines, "second") + assert lines[start].startswith(" - name: second") + + +def test_find_project_block_not_found() -> None: + lines = _SIMPLE_MANIFEST.splitlines(keepends=True) + with pytest.raises(RuntimeError, match="not found"): + _find_project_block(lines, "nonexistent") + + +# --- _set_simple_field_in_block -------------------------------------------- + + +def test_set_simple_field_updates_existing() -> None: + block = [ + " - name: myproject\n", + " revision: oldrev\n", + " url: https://example.com\n", + ] + result = _set_simple_field_in_block(block, 6, "revision", "newrev") + assert any("revision: newrev" in l for l in result) + assert not any("oldrev" in l for l in result) + + +def test_set_simple_field_inserts_after_name() -> None: + block = [ + " - name: myproject\n", + " url: https://example.com\n", + ] + result = _set_simple_field_in_block(block, 6, "revision", "abc123") + assert result[1] == " revision: abc123\n" + assert result[2] == " url: https://example.com\n" + + +# --- _set_integrity_hash_in_block ------------------------------------------ + + +def test_set_integrity_hash_inserts_when_absent() -> None: + block = [ + " - name: myproject\n", + " url: https://example.com/archive.tar.gz\n", + " vcs: archive\n", + ] + result = _set_integrity_hash_in_block(block, 6, "sha256:abc123") + joined = "".join(result) + assert "integrity:" in joined + assert "hash: sha256:abc123" in joined + + +def test_set_integrity_hash_updates_existing_hash() -> None: + block = [ + " - name: myproject\n", + " url: https://example.com/archive.tar.gz\n", + " vcs: archive\n", + " integrity:\n", + " hash: sha256:old\n", + ] + result = _set_integrity_hash_in_block(block, 6, "sha256:new") + joined = "".join(result) + assert "hash: sha256:new" in joined + assert "sha256:old" not in joined + + +# --- _update_project_version_in_text --------------------------------------- + + +def _make_project(name: str, **kwargs) -> ProjectEntry: + """Helper: build a ProjectEntry with the given fields.""" + data = {"name": name} + data.update(kwargs) + return ProjectEntry(data) # type: ignore[arg-type] + + +def test_update_adds_revision_preserves_layout() -> None: + text = _SIMPLE_MANIFEST + project = _make_project("myproject", revision="deadbeef" * 5, branch="main") + result = _update_project_version_in_text(text, project) + # The layout (4-space indent for "- name:") is preserved. + assert " - name: myproject" in result + # The revision is inserted. + assert "revision:" in result + # Original url line is still there. + assert "url: https://example.com/myproject" in result + + +def test_update_second_project_does_not_touch_first() -> None: + text = _TWO_PROJECT_MANIFEST + project = _make_project( + "second", revision="abc123def456" * 3 + "abcd", branch="develop" + ) + result = _update_project_version_in_text(text, project) + + # Verify the "first" block is unchanged by re-parsing and checking project count + # with "revision" in the result. + assert "revision:" in result # second project got a revision + + # The "first" project block should have no revision field: find its block boundaries + lines = result.splitlines(keepends=True) + first_start, first_end, _ = _find_project_block(lines, "first") + first_block_text = "".join(lines[first_start:first_end]) + assert "revision" not in first_block_text + + +def test_update_noop_when_no_version_fields() -> None: + text = _SIMPLE_MANIFEST + project = _make_project("myproject") # no version fields + result = _update_project_version_in_text(text, project) + assert result == text + + +def test_update_integer_like_revision_is_quoted() -> None: + """SVN revisions look like integers and must be YAML-quoted.""" + text = _SIMPLE_MANIFEST + project = _make_project("myproject", revision="176", branch="trunk") + result = _update_project_version_in_text(text, project) + # The scalar '176' must be quoted in YAML to round-trip as a string. + assert "revision: '176'" in result From d3e238c1b08f4c1ee51cf0e560ccea0cf1c991fc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Apr 2026 19:33:54 +0000 Subject: [PATCH 2/9] Reduce cyclomatic complexity of in-place editing helpers Split _set_integrity_hash_in_block (CC 11) and _update_project_version_in_text (CC 13) into smaller helpers so every new function stays below CC 9: - _update_hash_in_existing_integrity (CC 6) - _append_integrity_block (CC 3) - _set_integrity_hash_in_block (CC 4) - _collect_version_fields (CC 4) - _apply_block_updates (CC 7) - _update_project_version_in_text (CC 2) https://claude.ai/code/session_01Xd8EcAUkSoJo9YZimGzEuA --- dfetch/manifest/manifest.py | 142 +++++++++++++++++++++--------------- 1 file changed, 84 insertions(+), 58 deletions(-) diff --git a/dfetch/manifest/manifest.py b/dfetch/manifest/manifest.py index 8b15cf6d..67134561 100644 --- a/dfetch/manifest/manifest.py +++ b/dfetch/manifest/manifest.py @@ -578,42 +578,33 @@ def _set_simple_field_in_block( return block -def _set_integrity_hash_in_block( - block: list[str], field_indent: int, hash_value: str +def _update_hash_in_existing_integrity( + block: list[str], integrity_line: int, field_indent: int, hash_value: str ) -> list[str]: - """Update or insert ``integrity.hash`` inside *block*.""" - block = list(block) - integrity_re = re.compile(r"^(\s+)integrity:\s*$") + """Update or insert ``hash:`` inside an already-located ``integrity:`` block.""" hash_re = re.compile(r"^(\s+)hash:\s*.*$") - - # Locate an existing ``integrity:`` mapping. - integrity_line: int | None = None - for i, line in enumerate(block): - if integrity_re.match(line.rstrip("\n\r")): - integrity_line = i + for i in range(integrity_line + 1, len(block)): + stripped = block[i].strip() + if not stripped: + continue + if hash_re.match(block[i].rstrip("\n\r")): + eol = "\n" if block[i].endswith("\n") else "" + block[i] = " " * (field_indent + 2) + "hash: " + hash_value + eol + return block + # Stop searching once we leave the integrity sub-block. + if len(block[i]) - len(block[i].lstrip()) <= field_indent: break + # No ``hash:`` found — insert right after ``integrity:``. + block.insert( + integrity_line + 1, " " * (field_indent + 2) + "hash: " + hash_value + "\n" + ) + return block - if integrity_line is not None: - # Try to find an existing ``hash:`` nested inside it. - for i in range(integrity_line + 1, len(block)): - stripped = block[i].strip() - if not stripped: - continue - if hash_re.match(block[i].rstrip("\n\r")): - eol = "\n" if block[i].endswith("\n") else "" - block[i] = " " * (field_indent + 2) + "hash: " + hash_value + eol - return block - # Stop searching once we leave the integrity sub-block. - indent = len(block[i]) - len(block[i].lstrip()) - if indent <= field_indent: - break - # No ``hash:`` found — insert right after ``integrity:``. - block.insert( - integrity_line + 1, " " * (field_indent + 2) + "hash: " + hash_value + "\n" - ) - return block - # No ``integrity:`` block — append it before any trailing blank lines. +def _append_integrity_block( + block: list[str], field_indent: int, hash_value: str +) -> list[str]: + """Append a new ``integrity:\\n hash:`` block before any trailing blank lines.""" new_lines = [ " " * field_indent + "integrity:\n", " " * (field_indent + 2) + "hash: " + hash_value + "\n", @@ -627,21 +618,42 @@ def _set_integrity_hash_in_block( return block -def _update_project_version_in_text(text: str, project: ProjectEntry) -> str: - """Return *text* with the version fields for *project* updated in-place. +def _set_integrity_hash_in_block( + block: list[str], field_indent: int, hash_value: str +) -> list[str]: + """Update or insert ``integrity.hash`` inside *block*.""" + block = list(block) + integrity_re = re.compile(r"^(\s+)integrity:\s*$") - Only version-related fields (``revision``, ``tag``, ``branch``, - ``integrity.hash``) are touched; all other content — including comments - and indentation — is preserved verbatim. - """ - project_yaml = project.as_yaml() + integrity_line: int | None = None + for i, line in enumerate(block): + if integrity_re.match(line.rstrip("\n\r")): + integrity_line = i + break + + if integrity_line is not None: + return _update_hash_in_existing_integrity( + block, integrity_line, field_indent, hash_value + ) + + return _append_integrity_block(block, field_indent, hash_value) - # Collect simple version fields that are now set. - fields_to_set: list[tuple[str, str]] = [] + +def _collect_version_fields( + project_yaml: dict[str, Any], +) -> tuple[list[tuple[str, str]], str]: + """Extract version-related fields from *project_yaml*. + + Returns: + A ``(fields_to_set, integrity_hash)`` pair. *fields_to_set* is a list + of ``(field_name, yaml_scalar)`` tuples for ``revision``, ``tag`` and + ``branch``; *integrity_hash* is the raw hash string or ``""`` when absent. + """ + fields: list[tuple[str, str]] = [] for field in ("revision", "tag", "branch"): value = project_yaml.get(field) if value: - fields_to_set.append((field, _yaml_scalar(str(value)))) + fields.append((field, _yaml_scalar(str(value)))) integrity = project_yaml.get("integrity") integrity_hash: str = ( @@ -649,37 +661,51 @@ def _update_project_version_in_text(text: str, project: ProjectEntry) -> str: if isinstance(integrity, dict) and integrity.get("hash") else "" ) + return fields, integrity_hash - if not fields_to_set and not integrity_hash: - return text - - lines = text.splitlines(keepends=True) - start, end, item_indent = _find_project_block(lines, project.name) - field_indent = item_indent + 2 - block = list(lines[start:end]) - - # Track which fields were inserted (not just updated) so we can keep them - # in a sensible order right after the ``name:`` line. +def _apply_block_updates( + block: list[str], + field_indent: int, + fields_to_set: list[tuple[str, str]], + integrity_hash: str, +) -> list[str]: + """Apply version-field updates to *block* and return the modified block.""" inserted: list[tuple[str, str]] = [] for field_name, yaml_value in fields_to_set: field_re = re.compile(r"^(\s+)" + re.escape(field_name) + r":\s*.*$") - found = any(field_re.match(line.rstrip("\n\r")) for line in block) - if found: + if any(field_re.match(line.rstrip("\n\r")) for line in block): block = _set_simple_field_in_block( block, field_indent, field_name, yaml_value ) else: inserted.append((field_name, yaml_value)) - # Insert all new fields together right after ``name:`` (position 1). if inserted: - new_lines = [ - " " * field_indent + fname + ": " + val + "\n" for fname, val in inserted - ] - block[1:1] = new_lines + block[1:1] = [" " * field_indent + f + ": " + v + "\n" for f, v in inserted] if integrity_hash: block = _set_integrity_hash_in_block(block, field_indent, integrity_hash) + return block + + +def _update_project_version_in_text(text: str, project: ProjectEntry) -> str: + """Return *text* with the version fields for *project* updated in-place. + + Only version-related fields (``revision``, ``tag``, ``branch``, + ``integrity.hash``) are touched; all other content — including comments + and indentation — is preserved verbatim. + """ + fields_to_set, integrity_hash = _collect_version_fields(project.as_yaml()) + + if not fields_to_set and not integrity_hash: + return text + + lines = text.splitlines(keepends=True) + start, end, item_indent = _find_project_block(lines, project.name) + block = _apply_block_updates( + list(lines[start:end]), item_indent + 2, fields_to_set, integrity_hash + ) + return "".join(lines[:start] + block + lines[end:]) From 19b6a97983ce15f8b1208962f6f101623328e1d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Apr 2026 19:40:10 +0000 Subject: [PATCH 3/9] Extract shared _locate_project_name_line helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both find_name_in_manifest (used by SARIF/Jenkins/CodeClimate reporters) and _find_project_block (used by in-place freeze) independently scanned for the '- name: ' line with slightly different regexes. Extract _locate_project_name_line(lines, project_name) -> tuple | None that performs the scan once and returns all four pieces of information both callers need: - line_idx (0-based) - item_indent (column of the '-') - name_col_start / name_col_end (for ManifestEntryLocation) find_name_in_manifest now calls _locate_project_name_line, preserving its public interface (same error message, same ManifestEntryLocation values including the existing "no spaces" edge case). _find_project_block delegates the name scan to the helper and only retains the block-boundary search loop. Side-effect: CC of find_name_in_manifest drops 5→3, _find_project_block drops 7→4. https://claude.ai/code/session_01Xd8EcAUkSoJo9YZimGzEuA --- dfetch/manifest/manifest.py | 66 ++++++++++++++++++++++--------------- tests/test_manifest.py | 36 +++++++++++++++++++- 2 files changed, 75 insertions(+), 27 deletions(-) diff --git a/dfetch/manifest/manifest.py b/dfetch/manifest/manifest.py index 67134561..4a0c6d9c 100644 --- a/dfetch/manifest/manifest.py +++ b/dfetch/manifest/manifest.py @@ -358,18 +358,16 @@ def find_name_in_manifest(self, name: str) -> ManifestEntryLocation: if not self.__text: raise FileNotFoundError("No manifest text available") - for line_nr, line in enumerate(self.__text.splitlines(), start=1): - match = re.search( - rf"^\s+-\s*name:\s*(?P{re.escape(name)})\s*#?.*$", line - ) - - if match: - return ManifestEntryLocation( - line_number=line_nr, - start=int(match.start("name")) + 1, - end=int(match.end("name")), - ) - raise RuntimeError(f"{name} was not found in the manifest!") + result = _locate_project_name_line(self.__text.splitlines(), name) + if result is None: + raise RuntimeError(f"{name} was not found in the manifest!") + + line_idx, _, name_start, name_end = result + return ManifestEntryLocation( + line_number=line_idx + 1, + start=name_start, + end=name_end, + ) # Characters not allowed in a project name (YAML special chars). _UNSAFE_NAME_RE = re.compile(r"[\x00-\x1F\x7F-\x9F:#\[\]{}&*!|>'\"%@`]") @@ -516,6 +514,32 @@ def _yaml_scalar(value: str) -> str: return dumped.strip() +def _locate_project_name_line( + lines: list[str], project_name: str +) -> tuple[int, int, int, int] | None: + """Scan *lines* for the ``- name: `` entry. + + Returns ``(line_idx, item_indent, name_col_start, name_col_end)`` or + ``None`` when the project is not found. + + - ``line_idx``: 0-based index into *lines* + - ``item_indent``: column of the ``-`` character + - ``name_col_start``: 1-based column of the first character of the name value + (compatible with :class:`ManifestEntryLocation`) + - ``name_col_end``: 0-based exclusive end column of the name value + """ + pattern = re.compile( + r"^(?P\s*)-\s*name:\s*(?P" + + re.escape(project_name) + + r")\s*(?:#.*)?$" + ) + for i, line in enumerate(lines): + m = pattern.match(line.rstrip("\n\r")) + if m: + return i, len(m.group("indent")), m.start("name") + 1, m.end("name") + return None + + def _find_project_block(lines: list[str], project_name: str) -> tuple[int, int, int]: """Return ``(start, end, item_indent)`` for the named project's YAML block. @@ -526,22 +550,12 @@ def _find_project_block(lines: list[str], project_name: str) -> tuple[int, int, Raises: RuntimeError: if the project name is not found. """ - start: int | None = None - item_indent = 0 - - for i, line in enumerate(lines): - m = re.match( - r"^(\s*)-\s+name:\s*" + re.escape(project_name) + r"\s*$", - line.rstrip("\n\r"), - ) - if m: - start = i - item_indent = len(m.group(1)) - break - - if start is None: + result = _locate_project_name_line(lines, project_name) + if result is None: raise RuntimeError(f"Project '{project_name}' not found in manifest text") + start, item_indent, _, _ = result + end = len(lines) for i in range(start + 1, len(lines)): line = lines[i] diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 42adbda5..1023f6f5 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -15,8 +15,9 @@ ManifestEntryLocation, RequestedProjectNotFoundError, _find_project_block, - _set_simple_field_in_block, + _locate_project_name_line, _set_integrity_hash_in_block, + _set_simple_field_in_block, _update_project_version_in_text, ) from dfetch.manifest.parse import find_manifest, get_submanifests @@ -280,6 +281,39 @@ def test_validate_destination_accepts_relative() -> None: """ +# --- _locate_project_name_line --------------------------------------------- + + +def test_locate_project_name_line_found() -> None: + lines = _SIMPLE_MANIFEST.splitlines() + result = _locate_project_name_line(lines, "myproject") + assert result is not None + line_idx, item_indent, name_start, name_end = result + assert item_indent == 4 + assert lines[line_idx][name_start - 1 : name_end] == "myproject" + + +def test_locate_project_name_line_not_found() -> None: + lines = _SIMPLE_MANIFEST.splitlines() + assert _locate_project_name_line(lines, "nonexistent") is None + + +def test_locate_is_shared_by_find_name_and_find_block() -> None: + """find_name_in_manifest and _find_project_block agree on the same line.""" + text = _SIMPLE_MANIFEST + manifest = Manifest( + {"version": "0.0", "projects": [{"name": "myproject", "url": "https://x.com"}]}, + text=text, + ) + location = manifest.find_name_in_manifest("myproject") + + lines = text.splitlines(keepends=True) + start, _end, _indent = _find_project_block(lines, "myproject") + + # Both should point to the same line (1-based vs 0-based). + assert location.line_number == start + 1 + + # --- _find_project_block --------------------------------------------------- From 0b6bb85d4c53386d87df37ea83df5859b8f64c95 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Apr 2026 19:50:22 +0000 Subject: [PATCH 4/9] Generalize manifest field manipulation to generic primitives Replace specific-purpose helpers with three building-block functions: - `_find_field`: locate a named field at exact indent within a block - `_update_value`: replace the value on a known line index - `_append_field`: insert a new key-value line at a given position All higher-level helpers (_set_simple_field_in_block, _update_hash_in_existing_integrity, _append_integrity_block, _set_integrity_hash_in_block, _apply_block_updates) are refactored to compose these primitives instead of duplicating regex/index logic. https://claude.ai/code/session_01Xd8EcAUkSoJo9YZimGzEuA --- dfetch/manifest/manifest.py | 121 +++++++++++++++++++++--------------- tests/test_manifest.py | 82 ++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 49 deletions(-) diff --git a/dfetch/manifest/manifest.py b/dfetch/manifest/manifest.py index 4a0c6d9c..42d17517 100644 --- a/dfetch/manifest/manifest.py +++ b/dfetch/manifest/manifest.py @@ -569,67 +569,100 @@ def _find_project_block(lines: list[str], project_name: str) -> tuple[int, int, return start, end, item_indent -def _set_simple_field_in_block( - block: list[str], field_indent: int, field_name: str, yaml_value: str +def _find_field( + block: list[str], + field_name: str, + indent: int, + start: int = 0, + end: int | None = None, +) -> int | None: + """Return the index in *block* of ``field_name:`` at exactly *indent* spaces. + + Searches ``block[start:end]``. Returns ``None`` when not found. + """ + bound = end if end is not None else len(block) + prefix = " " * indent + field_name + ":" + for i in range(start, bound): + stripped = block[i].rstrip("\n\r") + if stripped.startswith(prefix) and ( + len(stripped) == len(prefix) or stripped[len(prefix)] in (" ", "\t") + ): + return i + return None + + +def _update_value( + block: list[str], line_idx: int, field_name: str, yaml_value: str ) -> list[str]: - """Update an existing ``field_name:`` line in *block*, or insert one after the first line. + """Return a copy of *block* with the value on *line_idx* replaced by *yaml_value*.""" + block = list(block) + line = block[line_idx] + indent = len(line) - len(line.lstrip()) + eol = "\n" if line.endswith("\n") else "" + block[line_idx] = " " * indent + field_name + ": " + yaml_value + eol + return block - *field_indent* is the expected column for fields in this block. - *yaml_value* is the already-serialised YAML scalar (e.g. ``e1fda...`` or ``'176'``). + +def _append_field( + block: list[str], + field_name: str, + yaml_value: str, + indent: int, + after: int, +) -> list[str]: + """Return a copy of *block* with ``field_name: yaml_value`` inserted at *after*. + + When *yaml_value* is empty the line is written as ``field_name:`` (no value), + which is the correct YAML form for a mapping key whose children follow. """ block = list(block) - field_re = re.compile(r"^(\s+)" + re.escape(field_name) + r":\s*.*$") + value_part = ": " + yaml_value if yaml_value else ":" + block.insert(after, " " * indent + field_name + value_part + "\n") + return block - for i, line in enumerate(block): - if field_re.match(line.rstrip("\n\r")): - eol = "\n" if line.endswith("\n") else "" - block[i] = " " * field_indent + field_name + ": " + yaml_value + eol - return block - # Field not present — insert right after the ``name:`` line (position 1). - new_line = " " * field_indent + field_name + ": " + yaml_value + "\n" - block.insert(1, new_line) - return block +def _set_simple_field_in_block( + block: list[str], field_indent: int, field_name: str, yaml_value: str +) -> list[str]: + """Update an existing ``field_name:`` line in *block*, or insert one after the first line.""" + idx = _find_field(block, field_name, field_indent) + if idx is not None: + return _update_value(block, idx, field_name, yaml_value) + return _append_field(block, field_name, yaml_value, field_indent, after=1) def _update_hash_in_existing_integrity( block: list[str], integrity_line: int, field_indent: int, hash_value: str ) -> list[str]: """Update or insert ``hash:`` inside an already-located ``integrity:`` block.""" - hash_re = re.compile(r"^(\s+)hash:\s*.*$") + sub_end = len(block) for i in range(integrity_line + 1, len(block)): - stripped = block[i].strip() - if not stripped: - continue - if hash_re.match(block[i].rstrip("\n\r")): - eol = "\n" if block[i].endswith("\n") else "" - block[i] = " " * (field_indent + 2) + "hash: " + hash_value + eol - return block - # Stop searching once we leave the integrity sub-block. - if len(block[i]) - len(block[i].lstrip()) <= field_indent: + if block[i].strip() and len(block[i]) - len(block[i].lstrip()) <= field_indent: + sub_end = i break - # No ``hash:`` found — insert right after ``integrity:``. - block.insert( - integrity_line + 1, " " * (field_indent + 2) + "hash: " + hash_value + "\n" + hash_idx = _find_field( + block, "hash", field_indent + 2, start=integrity_line + 1, end=sub_end + ) + if hash_idx is not None: + return _update_value(block, hash_idx, "hash", hash_value) + return _append_field( + block, "hash", hash_value, field_indent + 2, after=integrity_line + 1 ) - return block def _append_integrity_block( block: list[str], field_indent: int, hash_value: str ) -> list[str]: """Append a new ``integrity:\\n hash:`` block before any trailing blank lines.""" - new_lines = [ - " " * field_indent + "integrity:\n", - " " * (field_indent + 2) + "hash: " + hash_value + "\n", - ] insert_at = len(block) for i in range(len(block) - 1, -1, -1): if block[i].strip(): insert_at = i + 1 break - block[insert_at:insert_at] = new_lines - return block + block = _append_field(block, "integrity", "", field_indent, after=insert_at) + return _append_field( + block, "hash", hash_value, field_indent + 2, after=insert_at + 1 + ) def _set_integrity_hash_in_block( @@ -637,19 +670,11 @@ def _set_integrity_hash_in_block( ) -> list[str]: """Update or insert ``integrity.hash`` inside *block*.""" block = list(block) - integrity_re = re.compile(r"^(\s+)integrity:\s*$") - - integrity_line: int | None = None - for i, line in enumerate(block): - if integrity_re.match(line.rstrip("\n\r")): - integrity_line = i - break - + integrity_line = _find_field(block, "integrity", field_indent) if integrity_line is not None: return _update_hash_in_existing_integrity( block, integrity_line, field_indent, hash_value ) - return _append_integrity_block(block, field_indent, hash_value) @@ -687,11 +712,9 @@ def _apply_block_updates( """Apply version-field updates to *block* and return the modified block.""" inserted: list[tuple[str, str]] = [] for field_name, yaml_value in fields_to_set: - field_re = re.compile(r"^(\s+)" + re.escape(field_name) + r":\s*.*$") - if any(field_re.match(line.rstrip("\n\r")) for line in block): - block = _set_simple_field_in_block( - block, field_indent, field_name, yaml_value - ) + idx = _find_field(block, field_name, field_indent) + if idx is not None: + block = _update_value(block, idx, field_name, yaml_value) else: inserted.append((field_name, yaml_value)) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 1023f6f5..fdbc81a1 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -14,11 +14,14 @@ ManifestDict, ManifestEntryLocation, RequestedProjectNotFoundError, + _append_field, + _find_field, _find_project_block, _locate_project_name_line, _set_integrity_hash_in_block, _set_simple_field_in_block, _update_project_version_in_text, + _update_value, ) from dfetch.manifest.parse import find_manifest, get_submanifests from dfetch.manifest.project import ProjectEntry @@ -372,6 +375,85 @@ def test_set_simple_field_inserts_after_name() -> None: assert result[2] == " url: https://example.com\n" +# --- _find_field ----------------------------------------------------------- + + +def test_find_field_returns_index_when_present() -> None: + block = [ + " - name: myproject\n", + " revision: abc\n", + " url: https://example.com\n", + ] + assert _find_field(block, "revision", 6) == 1 + + +def test_find_field_returns_none_when_absent() -> None: + block = [ + " - name: myproject\n", + " url: https://example.com\n", + ] + assert _find_field(block, "revision", 6) is None + + +def test_find_field_respects_start_end_bounds() -> None: + block = [ + " - name: myproject\n", + " revision: abc\n", + " url: https://example.com\n", + ] + # revision is at index 1, but we start searching at index 2 — should not find it + assert _find_field(block, "revision", 6, start=2) is None + # and with end=1 it is also excluded + assert _find_field(block, "revision", 6, start=0, end=1) is None + + +def test_find_field_ignores_wrong_indent() -> None: + block = [ + " - name: myproject\n", + " revision: abc\n", # indent 4, not 6 + " url: https://example.com\n", + ] + assert _find_field(block, "revision", 6) is None + + +# --- _update_value --------------------------------------------------------- + + +def test_update_value_replaces_inline_value() -> None: + block = [ + " - name: myproject\n", + " revision: old\n", + ] + result = _update_value(block, 1, "revision", "new") + assert result[1] == " revision: new\n" + assert result[0] == block[0] # untouched + + +def test_update_value_preserves_indent() -> None: + block = [" revision: old\n"] + result = _update_value(block, 0, "revision", "newrev") + assert result[0].startswith(" revision:") + + +# --- _append_field --------------------------------------------------------- + + +def test_append_field_inserts_at_position() -> None: + block = [ + " - name: myproject\n", + " url: https://example.com\n", + ] + result = _append_field(block, "revision", "abc123", 6, after=1) + assert result[1] == " revision: abc123\n" + assert result[2] == " url: https://example.com\n" + + +def test_append_field_empty_value_omits_value_part() -> None: + block = [" - name: myproject\n"] + result = _append_field(block, "integrity", "", 6, after=1) + assert result[1] == " integrity:\n" + + # --- _set_integrity_hash_in_block ------------------------------------------ From 0ac98ba62810c822c5f3bbe629996e5f24ff0566 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Apr 2026 00:08:17 +0000 Subject: [PATCH 5/9] Move YAML utilities to dfetch.util.yaml; preserve comments on freeze - Extract yaml_scalar, find_field, update_value, append_field into the new dfetch.util.yaml module so they can be reused outside the manifest - Fix yaml_scalar: use splitlines()[0] instead of strip() to avoid the trailing '\n...' document-end marker that yaml.dump emits for plain strings - Fix update_value: preserve trailing inline comments (e.g. '# note') when replacing a field value, so freeze does not destroy annotations - manifest.py: replace the four duplicate implementations with aliases that delegate to dfetch.util.yaml - Add BDD scenario: inline comments on fields survive an in-place freeze - Add unit tests for comment preservation in update_value and _update_project_version_in_text https://claude.ai/code/session_01Xd8EcAUkSoJo9YZimGzEuA --- dfetch/manifest/manifest.py | 69 ++++------------------------ dfetch/util/yaml.py | 80 +++++++++++++++++++++++++++++++++ features/freeze-inplace.feature | 28 ++++++++++++ tests/test_manifest.py | 29 ++++++++++-- 4 files changed, 143 insertions(+), 63 deletions(-) create mode 100644 dfetch/util/yaml.py diff --git a/dfetch/manifest/manifest.py b/dfetch/manifest/manifest.py index 42d17517..5ada3051 100644 --- a/dfetch/manifest/manifest.py +++ b/dfetch/manifest/manifest.py @@ -33,6 +33,7 @@ from dfetch.log import get_logger from dfetch.manifest.project import ProjectEntry, ProjectEntryDict from dfetch.manifest.remote import Remote, RemoteDict +from dfetch.util.yaml import append_field, find_field, update_value, yaml_scalar logger = get_logger(__name__) @@ -504,14 +505,14 @@ def update_project_in_manifest_file( path.write_text(updated, encoding="utf-8") -def _yaml_scalar(value: str) -> str: - """Return the YAML inline representation of a scalar string value. - - Strings that look like integers (e.g. SVN revision ``'176'``) are quoted so - that YAML round-trips them back as strings. - """ - dumped: str = yaml.dump(value, default_flow_style=None, allow_unicode=True) - return dumped.strip() +# --------------------------------------------------------------------------- +# Private aliases – kept for internal callers; the canonical implementations +# live in dfetch.util.yaml. +# --------------------------------------------------------------------------- +_yaml_scalar = yaml_scalar +_find_field = find_field +_update_value = update_value +_append_field = append_field def _locate_project_name_line( @@ -569,58 +570,6 @@ def _find_project_block(lines: list[str], project_name: str) -> tuple[int, int, return start, end, item_indent -def _find_field( - block: list[str], - field_name: str, - indent: int, - start: int = 0, - end: int | None = None, -) -> int | None: - """Return the index in *block* of ``field_name:`` at exactly *indent* spaces. - - Searches ``block[start:end]``. Returns ``None`` when not found. - """ - bound = end if end is not None else len(block) - prefix = " " * indent + field_name + ":" - for i in range(start, bound): - stripped = block[i].rstrip("\n\r") - if stripped.startswith(prefix) and ( - len(stripped) == len(prefix) or stripped[len(prefix)] in (" ", "\t") - ): - return i - return None - - -def _update_value( - block: list[str], line_idx: int, field_name: str, yaml_value: str -) -> list[str]: - """Return a copy of *block* with the value on *line_idx* replaced by *yaml_value*.""" - block = list(block) - line = block[line_idx] - indent = len(line) - len(line.lstrip()) - eol = "\n" if line.endswith("\n") else "" - block[line_idx] = " " * indent + field_name + ": " + yaml_value + eol - return block - - -def _append_field( - block: list[str], - field_name: str, - yaml_value: str, - indent: int, - after: int, -) -> list[str]: - """Return a copy of *block* with ``field_name: yaml_value`` inserted at *after*. - - When *yaml_value* is empty the line is written as ``field_name:`` (no value), - which is the correct YAML form for a mapping key whose children follow. - """ - block = list(block) - value_part = ": " + yaml_value if yaml_value else ":" - block.insert(after, " " * indent + field_name + value_part + "\n") - return block - - def _set_simple_field_in_block( block: list[str], field_indent: int, field_name: str, yaml_value: str ) -> list[str]: diff --git a/dfetch/util/yaml.py b/dfetch/util/yaml.py new file mode 100644 index 00000000..8f370688 --- /dev/null +++ b/dfetch/util/yaml.py @@ -0,0 +1,80 @@ +"""Generic YAML text-editing utilities. + +These helpers operate on plain lists of text lines and know nothing about +dfetch-specific concepts. They are used by :mod:`dfetch.manifest.manifest` +to edit manifest files in-place while preserving comments and layout. +""" + +import re + +import yaml + + +def yaml_scalar(value: str) -> str: + """Return the YAML inline representation of a scalar string value. + + Strings that look like integers (e.g. SVN revision ``'176'``) are quoted + so that YAML round-trips them back as strings. + """ + dumped: str = yaml.dump(value, default_flow_style=None, allow_unicode=True) + return dumped.splitlines()[0] + + +def find_field( + block: list[str], + field_name: str, + indent: int, + start: int = 0, + end: int | None = None, +) -> int | None: + """Return the index in *block* of ``field_name:`` at exactly *indent* spaces. + + Searches ``block[start:end]``. Returns ``None`` when not found. + """ + bound = end if end is not None else len(block) + prefix = " " * indent + field_name + ":" + for i in range(start, bound): + stripped = block[i].rstrip("\n\r") + if stripped.startswith(prefix) and ( + len(stripped) == len(prefix) or stripped[len(prefix)] in (" ", "\t") + ): + return i + return None + + +def update_value( + block: list[str], line_idx: int, field_name: str, yaml_value: str +) -> list[str]: + """Return a copy of *block* with the value on *line_idx* replaced by *yaml_value*. + + The indentation, line ending, and any trailing comment of the original + line are all preserved so that in-place edits do not destroy annotations. + """ + block = list(block) + line = block[line_idx] + indent = len(line) - len(line.lstrip()) + eol = "\n" if line.endswith("\n") else "" + stripped = line.rstrip("\n\r") + comment_match = re.search(r"(\s+#.*)$", stripped) + comment = comment_match.group(1) if comment_match else "" + block[line_idx] = " " * indent + field_name + ": " + yaml_value + comment + eol + return block + + +def append_field( + block: list[str], + field_name: str, + yaml_value: str, + indent: int, + after: int, +) -> list[str]: + """Return a copy of *block* with ``field_name: yaml_value`` inserted at *after*. + + When *yaml_value* is empty the line is written as ``field_name:`` (no + value), which is the correct YAML form for a mapping key whose children + follow on subsequent lines. + """ + block = list(block) + value_part = ": " + yaml_value if yaml_value else ":" + block.insert(after, " " * indent + field_name + value_part + "\n") + return block diff --git a/features/freeze-inplace.feature b/features/freeze-inplace.feature index 069a1303..1c39112e 100644 --- a/features/freeze-inplace.feature +++ b/features/freeze-inplace.feature @@ -32,6 +32,34 @@ Feature: Freeze manifest in-place inside a version-controlled superproject """ And no file 'dfetch.yaml.backup' exists in superproject + Scenario: Inline comments on fields are preserved after freeze + Given a local git repo "superproject3" with the manifest + """ + manifest: + version: '0.0' + + projects: + - name: ext/test-repo-tag + url: https://github.com/dfetch-org/test-repo # source mirror + branch: main # track the integration branch + + """ + And all projects are updated in superproject3 + When I run "dfetch freeze" in superproject3 + Then the manifest 'dfetch.yaml' in superproject3 is replaced with + """ + manifest: + version: '0.0' + + projects: + - name: ext/test-repo-tag + revision: e1fda19a57b873eb8e6ae37780594cbb77b70f1a + url: https://github.com/dfetch-org/test-repo # source mirror + branch: main # track the integration branch + + """ + And no file 'dfetch.yaml.backup' exists in superproject3 + Scenario: Only selected project is frozen in-place Given a local git repo "superproject2" with the manifest """ diff --git a/tests/test_manifest.py b/tests/test_manifest.py index fdbc81a1..e7a417f2 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -14,15 +14,15 @@ ManifestDict, ManifestEntryLocation, RequestedProjectNotFoundError, - _append_field, - _find_field, _find_project_block, _locate_project_name_line, _set_integrity_hash_in_block, _set_simple_field_in_block, _update_project_version_in_text, - _update_value, ) +from dfetch.util.yaml import append_field as _append_field +from dfetch.util.yaml import find_field as _find_field +from dfetch.util.yaml import update_value as _update_value from dfetch.manifest.parse import find_manifest, get_submanifests from dfetch.manifest.project import ProjectEntry @@ -435,6 +435,12 @@ def test_update_value_preserves_indent() -> None: assert result[0].startswith(" revision:") +def test_update_value_preserves_trailing_comment() -> None: + block = [" branch: main # track the integration branch\n"] + result = _update_value(block, 0, "branch", "main") + assert result[0] == " branch: main # track the integration branch\n" + + # --- _append_field --------------------------------------------------------- @@ -537,3 +543,20 @@ def test_update_integer_like_revision_is_quoted() -> None: result = _update_project_version_in_text(text, project) # The scalar '176' must be quoted in YAML to round-trip as a string. assert "revision: '176'" in result + + +def test_update_preserves_inline_comments_on_fields() -> None: + """Inline comments on existing fields survive an in-place freeze.""" + text = ( + "manifest:\n" + " version: '0.0'\n" + " projects:\n" + " - name: myproject\n" + " url: https://example.com # source mirror\n" + " branch: main # track the integration branch\n" + ) + project = _make_project("myproject", revision="deadbeef" * 5, branch="main") + result = _update_project_version_in_text(text, project) + assert "url: https://example.com # source mirror" in result + assert "branch: main # track the integration branch" in result + assert "revision:" in result From 304918192fcc830020718aacb1c53278c53987c7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Apr 2026 04:32:53 +0000 Subject: [PATCH 6/9] Skip commented-out lines in field lookup and block boundary detection find_field: add an explicit guard to skip lines where the first non-whitespace character is '#'. Previously the startswith-prefix check happened to reject these lines, but the intent was implicit. _find_project_block: skip commented lines when scanning for the end of a project block. A comment at item-indent level (e.g. ' # old pin') was incorrectly treated as a block boundary, causing fields that followed it to be excluded from the block and duplicated on freeze instead of updated. Add unit tests covering: - find_field skips '# field:' and '#field:' (no space) - find_field continues past a commented-out duplicate to find the live field - _find_project_block includes fields that follow a comment at item-indent - _update_project_version_in_text: commented-out field treated as absent - _update_project_version_in_text: comment at item-indent does not break block https://claude.ai/code/session_01Xd8EcAUkSoJo9YZimGzEuA --- dfetch/manifest/manifest.py | 2 + dfetch/util/yaml.py | 6 ++- tests/test_manifest.py | 86 +++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/dfetch/manifest/manifest.py b/dfetch/manifest/manifest.py index 5ada3051..2ee885df 100644 --- a/dfetch/manifest/manifest.py +++ b/dfetch/manifest/manifest.py @@ -562,6 +562,8 @@ def _find_project_block(lines: list[str], project_name: str) -> tuple[int, int, line = lines[i] if not line.strip(): continue + if line.lstrip().startswith("#"): + continue indent = len(line) - len(line.lstrip()) if indent <= item_indent: end = i diff --git a/dfetch/util/yaml.py b/dfetch/util/yaml.py index 8f370688..3df752b2 100644 --- a/dfetch/util/yaml.py +++ b/dfetch/util/yaml.py @@ -29,12 +29,16 @@ def find_field( ) -> int | None: """Return the index in *block* of ``field_name:`` at exactly *indent* spaces. - Searches ``block[start:end]``. Returns ``None`` when not found. + Searches ``block[start:end]``. Commented-out lines (where the first + non-whitespace character is ``#``) are skipped and never matched. + Returns ``None`` when not found. """ bound = end if end is not None else len(block) prefix = " " * indent + field_name + ":" for i in range(start, bound): stripped = block[i].rstrip("\n\r") + if stripped.lstrip().startswith("#"): + continue if stripped.startswith(prefix) and ( len(stripped) == len(prefix) or stripped[len(prefix)] in (" ", "\t") ): diff --git a/tests/test_manifest.py b/tests/test_manifest.py index e7a417f2..668efe07 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -351,6 +351,20 @@ def test_find_project_block_not_found() -> None: _find_project_block(lines, "nonexistent") +def test_find_project_block_comment_at_item_indent_does_not_end_block() -> None: + """A comment at the same indent level as '- name:' must not split the block.""" + manifest = ( + " - name: myproject\n" + " url: https://example.com\n" + " # revision: old-rev <- comment at item-indent\n" + " branch: main\n" + ) + lines = manifest.splitlines(keepends=True) + start, end, item_indent = _find_project_block(lines, "myproject") + block_text = "".join(lines[start:end]) + assert "branch: main" in block_text + + # --- _set_simple_field_in_block -------------------------------------------- @@ -416,6 +430,36 @@ def test_find_field_ignores_wrong_indent() -> None: assert _find_field(block, "revision", 6) is None +def test_find_field_skips_commented_out_field() -> None: + """A '# field: value' line must not be matched — field is considered absent.""" + block = [ + " - name: myproject\n", + " # branch: main\n", # commented-out + " url: https://example.com\n", + ] + assert _find_field(block, "branch", 6) is None + + +def test_find_field_skips_commented_field_no_space() -> None: + """'#field: value' (no space after #) must also not be matched.""" + block = [ + " - name: myproject\n", + " #branch: main\n", + " url: https://example.com\n", + ] + assert _find_field(block, "branch", 6) is None + + +def test_find_field_finds_real_field_past_commented_one() -> None: + """The live field after a commented-out duplicate is matched.""" + block = [ + " - name: myproject\n", + " # branch: old\n", + " branch: new\n", + ] + assert _find_field(block, "branch", 6) == 2 + + # --- _update_value --------------------------------------------------------- @@ -560,3 +604,45 @@ def test_update_preserves_inline_comments_on_fields() -> None: assert "url: https://example.com # source mirror" in result assert "branch: main # track the integration branch" in result assert "revision:" in result + + +def test_update_commented_out_field_is_appended_not_matched() -> None: + """A commented-out version field must be treated as absent; the real value is appended.""" + text = ( + "manifest:\n" + " version: '0.0'\n" + " projects:\n" + " - name: myproject\n" + " url: https://example.com\n" + " # branch: old-branch\n" + " branch: main\n" + ) + project = _make_project("myproject", revision="deadbeef" * 5, branch="main") + result = _update_project_version_in_text(text, project) + # Commented-out line must survive unchanged + assert " # branch: old-branch" in result + # The live branch line keeps its comment-free value + assert " branch: main" in result + # revision is inserted as a new field, not used to update the comment + assert result.count("branch:") == 2 # comment + live field + assert "revision:" in result + + +def test_update_comment_at_item_indent_does_not_break_block() -> None: + """A comment at item-indent level inside a block must not end the block early.""" + text = ( + "manifest:\n" + " version: '0.0'\n" + " projects:\n" + " - name: myproject\n" + " url: https://example.com\n" + " # old pinned version\n" + " branch: main\n" + ) + project = _make_project("myproject", revision="deadbeef" * 5, branch="main") + result = _update_project_version_in_text(text, project) + # The comment at item-indent is preserved verbatim + assert " # old pinned version" in result + # branch is updated in-place, not duplicated + assert result.count("branch:") == 1 + assert "revision:" in result From e51247d5f4f982e91955dee4e46173b02f8640b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Apr 2026 04:55:07 +0000 Subject: [PATCH 7/9] Fix three in-place manifest editing bugs integrity sub_end scan skipped comment lines (manifest.py) The loop in _update_hash_in_existing_integrity that computes sub_end treated comment-only lines as block boundaries, so a '# note' line inside an integrity: mapping caused the subsequent hash: line to be missed. The loop now skips blank and comment-only lines before comparing indentation. Stale version keys not removed on in-place freeze (manifest.py) _apply_block_updates only added/updated fields; keys absent from project.as_yaml() (e.g. revision after unfreeze, or branch after switching to tag) were left in the manifest as stale entries. Added _remove_stale_version_fields and _remove_integrity_block helpers that delete version keys not present in the new project data before applying updates. Removed the early-return in _update_project_version_in_text so cleanup always runs. CRLF files collapsed to LF (yaml.py) update_value always wrote "\n"; append_field also hard-coded "\n". update_value now detects "\r\n" vs "\n" from the source line. append_field uses a new _detect_eol() helper that inspects the block's existing lines to infer the file's EOL convention. https://claude.ai/code/session_01Xd8EcAUkSoJo9YZimGzEuA --- dfetch/manifest/manifest.py | 72 +++++++++++++++++++++++++++------ dfetch/util/yaml.py | 23 ++++++++--- tests/test_manifest.py | 79 +++++++++++++++++++++++++++++++++++-- 3 files changed, 153 insertions(+), 21 deletions(-) diff --git a/dfetch/manifest/manifest.py b/dfetch/manifest/manifest.py index 2ee885df..e8353f11 100644 --- a/dfetch/manifest/manifest.py +++ b/dfetch/manifest/manifest.py @@ -588,7 +588,10 @@ def _update_hash_in_existing_integrity( """Update or insert ``hash:`` inside an already-located ``integrity:`` block.""" sub_end = len(block) for i in range(integrity_line + 1, len(block)): - if block[i].strip() and len(block[i]) - len(block[i].lstrip()) <= field_indent: + line = block[i] + if not line.strip() or line.lstrip().startswith("#"): + continue + if len(line) - len(line.lstrip()) <= field_indent: sub_end = i break hash_idx = _find_field( @@ -629,6 +632,45 @@ def _set_integrity_hash_in_block( return _append_integrity_block(block, field_indent, hash_value) +_VERSION_KEYS: tuple[str, ...] = ("revision", "tag", "branch") + + +def _remove_integrity_block(block: list[str], field_indent: int) -> list[str]: + """Remove the ``integrity:`` mapping and all its children from *block*.""" + integrity_idx = _find_field(block, "integrity", field_indent) + if integrity_idx is None: + return block + sub_end = len(block) + for i in range(integrity_idx + 1, len(block)): + line = block[i] + if not line.strip() or line.lstrip().startswith("#"): + continue + if len(line) - len(line.lstrip()) <= field_indent: + sub_end = i + break + block = list(block) + del block[integrity_idx:sub_end] + return block + + +def _remove_stale_version_fields( + block: list[str], + field_indent: int, + keys_to_keep: set[str], + keep_integrity: bool, +) -> list[str]: + """Delete version-related keys from *block* that are absent from *keys_to_keep*.""" + for key in _VERSION_KEYS: + if key not in keys_to_keep: + idx = _find_field(block, key, field_indent) + if idx is not None: + block = list(block) + del block[idx] + if not keep_integrity: + block = _remove_integrity_block(block, field_indent) + return block + + def _collect_version_fields( project_yaml: dict[str, Any], ) -> tuple[list[tuple[str, str]], str]: @@ -660,7 +702,17 @@ def _apply_block_updates( fields_to_set: list[tuple[str, str]], integrity_hash: str, ) -> list[str]: - """Apply version-field updates to *block* and return the modified block.""" + """Apply version-field updates to *block* and return the modified block. + + Stale version keys (those no longer present in *fields_to_set* / + *integrity_hash*) are deleted so the in-place result matches what the + backup-and-regenerate path would produce. + """ + keys_to_keep = {name for name, _ in fields_to_set} + block = _remove_stale_version_fields( + block, field_indent, keys_to_keep, bool(integrity_hash) + ) + inserted: list[tuple[str, str]] = [] for field_name, yaml_value in fields_to_set: idx = _find_field(block, field_name, field_indent) @@ -669,8 +721,8 @@ def _apply_block_updates( else: inserted.append((field_name, yaml_value)) - if inserted: - block[1:1] = [" " * field_indent + f + ": " + v + "\n" for f, v in inserted] + for pos, (field_name, yaml_value) in enumerate(inserted, start=1): + block = _append_field(block, field_name, yaml_value, field_indent, after=pos) if integrity_hash: block = _set_integrity_hash_in_block(block, field_indent, integrity_hash) @@ -681,19 +733,15 @@ def _apply_block_updates( def _update_project_version_in_text(text: str, project: ProjectEntry) -> str: """Return *text* with the version fields for *project* updated in-place. - Only version-related fields (``revision``, ``tag``, ``branch``, - ``integrity.hash``) are touched; all other content — including comments - and indentation — is preserved verbatim. + Version-related fields (``revision``, ``tag``, ``branch``, + ``integrity.hash``) are added, updated, or removed to match the project's + current state. All other content — including comments and indentation — + is preserved verbatim. """ fields_to_set, integrity_hash = _collect_version_fields(project.as_yaml()) - - if not fields_to_set and not integrity_hash: - return text - lines = text.splitlines(keepends=True) start, end, item_indent = _find_project_block(lines, project.name) block = _apply_block_updates( list(lines[start:end]), item_indent + 2, fields_to_set, integrity_hash ) - return "".join(lines[:start] + block + lines[end:]) diff --git a/dfetch/util/yaml.py b/dfetch/util/yaml.py index 3df752b2..df647925 100644 --- a/dfetch/util/yaml.py +++ b/dfetch/util/yaml.py @@ -46,18 +46,29 @@ def find_field( return None +def _detect_eol(block: list[str]) -> str: + """Return the line-ending style used in *block*, defaulting to ``\\n``.""" + for line in block: + if line.endswith("\r\n"): + return "\r\n" + if line.endswith("\n"): + return "\n" + return "\n" + + def update_value( block: list[str], line_idx: int, field_name: str, yaml_value: str ) -> list[str]: """Return a copy of *block* with the value on *line_idx* replaced by *yaml_value*. - The indentation, line ending, and any trailing comment of the original - line are all preserved so that in-place edits do not destroy annotations. + The indentation, line ending (LF or CRLF), and any trailing comment of + the original line are all preserved so that in-place edits do not destroy + annotations or alter the file's line-ending convention. """ block = list(block) line = block[line_idx] indent = len(line) - len(line.lstrip()) - eol = "\n" if line.endswith("\n") else "" + eol = "\r\n" if line.endswith("\r\n") else "\n" if line.endswith("\n") else "" stripped = line.rstrip("\n\r") comment_match = re.search(r"(\s+#.*)$", stripped) comment = comment_match.group(1) if comment_match else "" @@ -76,9 +87,11 @@ def append_field( When *yaml_value* is empty the line is written as ``field_name:`` (no value), which is the correct YAML form for a mapping key whose children - follow on subsequent lines. + follow on subsequent lines. The line-ending style (LF or CRLF) is inferred + from the existing lines in *block*. """ block = list(block) value_part = ": " + yaml_value if yaml_value else ":" - block.insert(after, " " * indent + field_name + value_part + "\n") + eol = _detect_eol(block) + block.insert(after, " " * indent + field_name + value_part + eol) return block diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 668efe07..7a7d152b 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -573,11 +573,16 @@ def test_update_second_project_does_not_touch_first() -> None: assert "revision" not in first_block_text -def test_update_noop_when_no_version_fields() -> None: - text = _SIMPLE_MANIFEST - project = _make_project("myproject") # no version fields +def test_update_stale_version_keys_removed_when_project_has_none() -> None: + """Stale version keys in the manifest are removed when the project carries none.""" + text = _SIMPLE_MANIFEST # contains branch: main + project = _make_project("myproject") # no version fields at all result = _update_project_version_in_text(text, project) - assert result == text + assert "branch:" not in result + assert "revision:" not in result + assert "tag:" not in result + # Non-version fields are untouched + assert "url:" in result def test_update_integer_like_revision_is_quoted() -> None: @@ -646,3 +651,69 @@ def test_update_comment_at_item_indent_does_not_break_block() -> None: # branch is updated in-place, not duplicated assert result.count("branch:") == 1 assert "revision:" in result + + +def test_update_stale_revision_removed_when_project_switches_to_tag() -> None: + """When a project changes from revision to tag, the stale revision key is deleted.""" + text = ( + "manifest:\n" + " version: '0.0'\n" + " projects:\n" + " - name: myproject\n" + " url: https://example.com\n" + " revision: deadbeefdeadbeef\n" + " branch: main\n" + ) + project = _make_project("myproject", tag="v1.0.0") + result = _update_project_version_in_text(text, project) + assert "tag: v1.0.0" in result + assert "revision:" not in result + assert "branch:" not in result + + +# --------------------------------------------------------------------------- +# EOL preservation +# --------------------------------------------------------------------------- + + +def test_update_value_preserves_crlf() -> None: + block = [" revision: old\r\n"] + result = _update_value(block, 0, "revision", "new") + assert result[0].endswith("\r\n") + assert result[0] == " revision: new\r\n" + + +def test_append_field_inherits_crlf_from_block() -> None: + block = [ + " - name: myproject\r\n", + " url: https://example.com\r\n", + ] + result = _append_field(block, "revision", "abc", 6, after=1) + assert result[1].endswith("\r\n"), "inserted line should use CRLF to match block" + + +def test_append_field_defaults_to_lf_on_empty_block() -> None: + result = _append_field([], "revision", "abc", 6, after=0) + assert result[0].endswith("\n") + assert not result[0].endswith("\r\n") + + +# --------------------------------------------------------------------------- +# integrity sub_end comment fix +# --------------------------------------------------------------------------- + + +def test_integrity_hash_updated_past_comment_in_integrity_block() -> None: + """A comment inside an integrity: block must not cut off the hash: search.""" + block = [ + " - name: myproject\n", + " integrity:\n", + " # checksum added by CI\n", + " hash: sha256:oldvalue\n", + " url: https://example.com\n", + ] + result = _set_integrity_hash_in_block(block, 6, "sha256:newvalue") + joined = "".join(result) + assert "hash: sha256:newvalue" in joined + assert "sha256:oldvalue" not in joined + assert "# checksum added by CI" in joined From f0016131ee8180bfea4d90be82fff816679f8dfb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Apr 2026 05:22:02 +0000 Subject: [PATCH 8/9] Fix pyright errors and remove duplication in yaml.py Sequence[str] for read-only inputs (manifest.py + yaml.py) All functions that accept a block/lines parameter but never mutate it in-place now declare Sequence[str] instead of list[str]. This resolves 8 pyright errors in tests where str.splitlines() on a LiteralString produced list[LiteralString], which is invariant and could not be passed as list[str]. Eliminate duplicated EOL detection (yaml.py) _detect_eol and update_value both duplicated the endswith("\r\n") / endswith("\n") pattern. Extracted into _line_eol(line) -> str and wired both callers through it. Cleaner copy pattern in _remove_* helpers (manifest.py) _remove_integrity_block and _remove_stale_version_fields used conditional list() copies. Both now copy unconditionally at the top (result = list(block)) for clarity and consistent return type. https://claude.ai/code/session_01Xd8EcAUkSoJo9YZimGzEuA --- dfetch/manifest/manifest.py | 46 +++++++++++++++++--------------- dfetch/util/yaml.py | 53 ++++++++++++++++++++++--------------- 2 files changed, 55 insertions(+), 44 deletions(-) diff --git a/dfetch/manifest/manifest.py b/dfetch/manifest/manifest.py index e8353f11..03ec4532 100644 --- a/dfetch/manifest/manifest.py +++ b/dfetch/manifest/manifest.py @@ -516,7 +516,7 @@ def update_project_in_manifest_file( def _locate_project_name_line( - lines: list[str], project_name: str + lines: Sequence[str], project_name: str ) -> tuple[int, int, int, int] | None: """Scan *lines* for the ``- name: `` entry. @@ -541,7 +541,9 @@ def _locate_project_name_line( return None -def _find_project_block(lines: list[str], project_name: str) -> tuple[int, int, int]: +def _find_project_block( + lines: Sequence[str], project_name: str +) -> tuple[int, int, int]: """Return ``(start, end, item_indent)`` for the named project's YAML block. *start* is the index of the ``- name: `` line. @@ -573,7 +575,7 @@ def _find_project_block(lines: list[str], project_name: str) -> tuple[int, int, def _set_simple_field_in_block( - block: list[str], field_indent: int, field_name: str, yaml_value: str + block: Sequence[str], field_indent: int, field_name: str, yaml_value: str ) -> list[str]: """Update an existing ``field_name:`` line in *block*, or insert one after the first line.""" idx = _find_field(block, field_name, field_indent) @@ -583,7 +585,7 @@ def _set_simple_field_in_block( def _update_hash_in_existing_integrity( - block: list[str], integrity_line: int, field_indent: int, hash_value: str + block: Sequence[str], integrity_line: int, field_indent: int, hash_value: str ) -> list[str]: """Update or insert ``hash:`` inside an already-located ``integrity:`` block.""" sub_end = len(block) @@ -605,7 +607,7 @@ def _update_hash_in_existing_integrity( def _append_integrity_block( - block: list[str], field_indent: int, hash_value: str + block: Sequence[str], field_indent: int, hash_value: str ) -> list[str]: """Append a new ``integrity:\\n hash:`` block before any trailing blank lines.""" insert_at = len(block) @@ -620,7 +622,7 @@ def _append_integrity_block( def _set_integrity_hash_in_block( - block: list[str], field_indent: int, hash_value: str + block: Sequence[str], field_indent: int, hash_value: str ) -> list[str]: """Update or insert ``integrity.hash`` inside *block*.""" block = list(block) @@ -635,40 +637,40 @@ def _set_integrity_hash_in_block( _VERSION_KEYS: tuple[str, ...] = ("revision", "tag", "branch") -def _remove_integrity_block(block: list[str], field_indent: int) -> list[str]: +def _remove_integrity_block(block: Sequence[str], field_indent: int) -> list[str]: """Remove the ``integrity:`` mapping and all its children from *block*.""" - integrity_idx = _find_field(block, "integrity", field_indent) + result = list(block) + integrity_idx = _find_field(result, "integrity", field_indent) if integrity_idx is None: - return block - sub_end = len(block) - for i in range(integrity_idx + 1, len(block)): - line = block[i] + return result + sub_end = len(result) + for i in range(integrity_idx + 1, len(result)): + line = result[i] if not line.strip() or line.lstrip().startswith("#"): continue if len(line) - len(line.lstrip()) <= field_indent: sub_end = i break - block = list(block) - del block[integrity_idx:sub_end] - return block + del result[integrity_idx:sub_end] + return result def _remove_stale_version_fields( - block: list[str], + block: Sequence[str], field_indent: int, keys_to_keep: set[str], keep_integrity: bool, ) -> list[str]: """Delete version-related keys from *block* that are absent from *keys_to_keep*.""" + result = list(block) for key in _VERSION_KEYS: if key not in keys_to_keep: - idx = _find_field(block, key, field_indent) + idx = _find_field(result, key, field_indent) if idx is not None: - block = list(block) - del block[idx] + del result[idx] if not keep_integrity: - block = _remove_integrity_block(block, field_indent) - return block + result = _remove_integrity_block(result, field_indent) + return result def _collect_version_fields( @@ -697,7 +699,7 @@ def _collect_version_fields( def _apply_block_updates( - block: list[str], + block: Sequence[str], field_indent: int, fields_to_set: list[tuple[str, str]], integrity_hash: str, diff --git a/dfetch/util/yaml.py b/dfetch/util/yaml.py index df647925..d8af5ee9 100644 --- a/dfetch/util/yaml.py +++ b/dfetch/util/yaml.py @@ -6,6 +6,7 @@ """ import re +from collections.abc import Sequence import yaml @@ -20,8 +21,26 @@ def yaml_scalar(value: str) -> str: return dumped.splitlines()[0] +def _line_eol(line: str) -> str: + """Return the line-ending sequence of *line*, or ``""`` if it has none.""" + if line.endswith("\r\n"): + return "\r\n" + if line.endswith("\n"): + return "\n" + return "" + + +def _detect_eol(block: Sequence[str]) -> str: + """Return the line-ending style used in *block*, defaulting to ``\\n``.""" + for line in block: + eol = _line_eol(line) + if eol: + return eol + return "\n" + + def find_field( - block: list[str], + block: Sequence[str], field_name: str, indent: int, start: int = 0, @@ -46,18 +65,8 @@ def find_field( return None -def _detect_eol(block: list[str]) -> str: - """Return the line-ending style used in *block*, defaulting to ``\\n``.""" - for line in block: - if line.endswith("\r\n"): - return "\r\n" - if line.endswith("\n"): - return "\n" - return "\n" - - def update_value( - block: list[str], line_idx: int, field_name: str, yaml_value: str + block: Sequence[str], line_idx: int, field_name: str, yaml_value: str ) -> list[str]: """Return a copy of *block* with the value on *line_idx* replaced by *yaml_value*. @@ -65,19 +74,19 @@ def update_value( the original line are all preserved so that in-place edits do not destroy annotations or alter the file's line-ending convention. """ - block = list(block) - line = block[line_idx] + result = list(block) + line = result[line_idx] indent = len(line) - len(line.lstrip()) - eol = "\r\n" if line.endswith("\r\n") else "\n" if line.endswith("\n") else "" + eol = _line_eol(line) stripped = line.rstrip("\n\r") comment_match = re.search(r"(\s+#.*)$", stripped) comment = comment_match.group(1) if comment_match else "" - block[line_idx] = " " * indent + field_name + ": " + yaml_value + comment + eol - return block + result[line_idx] = " " * indent + field_name + ": " + yaml_value + comment + eol + return result def append_field( - block: list[str], + block: Sequence[str], field_name: str, yaml_value: str, indent: int, @@ -90,8 +99,8 @@ def append_field( follow on subsequent lines. The line-ending style (LF or CRLF) is inferred from the existing lines in *block*. """ - block = list(block) + result = list(block) value_part = ": " + yaml_value if yaml_value else ":" - eol = _detect_eol(block) - block.insert(after, " " * indent + field_name + value_part + eol) - return block + eol = _detect_eol(result) + result.insert(after, " " * indent + field_name + value_part + eol) + return result From e1a96e1ef684e387f5d95727269ccc05f503e030 Mon Sep 17 00:00:00 2001 From: Ben Date: Fri, 3 Apr 2026 06:04:45 +0000 Subject: [PATCH 9/9] fix issues --- dfetch/manifest/manifest.py | 2 +- dfetch/util/yaml.py | 2 +- doc/conf.py | 8 +++++++- tests/test_manifest.py | 4 ++-- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/dfetch/manifest/manifest.py b/dfetch/manifest/manifest.py index 03ec4532..5d1fae6e 100644 --- a/dfetch/manifest/manifest.py +++ b/dfetch/manifest/manifest.py @@ -609,7 +609,7 @@ def _update_hash_in_existing_integrity( def _append_integrity_block( block: Sequence[str], field_indent: int, hash_value: str ) -> list[str]: - """Append a new ``integrity:\\n hash:`` block before any trailing blank lines.""" + r"""Append a new ``integrity:\\n hash:`` block before any trailing blank lines.""" insert_at = len(block) for i in range(len(block) - 1, -1, -1): if block[i].strip(): diff --git a/dfetch/util/yaml.py b/dfetch/util/yaml.py index d8af5ee9..7ea4b2ca 100644 --- a/dfetch/util/yaml.py +++ b/dfetch/util/yaml.py @@ -31,7 +31,7 @@ def _line_eol(line: str) -> str: def _detect_eol(block: Sequence[str]) -> str: - """Return the line-ending style used in *block*, defaulting to ``\\n``.""" + r"""Return the line-ending style used in *block*, defaulting to ``\\n``.""" for line in block: eol = _line_eol(line) if eol: diff --git a/doc/conf.py b/doc/conf.py index e295ab66..2a933c0f 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -250,7 +250,13 @@ # (source start file, target name, title, # author, documentclass [howto, manual, or own class]). latex_documents = [ - (master_doc, f"dfetch-{__version__}.tex", "Dfetch Documentation", "Dfetch", "manual"), + ( + master_doc, + f"dfetch-{__version__}.tex", + "Dfetch Documentation", + "Dfetch", + "manual", + ), ] diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 7a7d152b..c2faf51d 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -20,11 +20,11 @@ _set_simple_field_in_block, _update_project_version_in_text, ) +from dfetch.manifest.parse import find_manifest, get_submanifests +from dfetch.manifest.project import ProjectEntry from dfetch.util.yaml import append_field as _append_field from dfetch.util.yaml import find_field as _find_field from dfetch.util.yaml import update_value as _update_value -from dfetch.manifest.parse import find_manifest, get_submanifests -from dfetch.manifest.project import ProjectEntry BASIC_MANIFEST = """ manifest: