Simplify manifest class

Author: spoorcc

dfetch/commands/add.py

@@ -253,7 +253,7 @@ def _finalize_add(
         return
 
     superproject.manifest.append_project_entry(project_entry)
-    superproject.manifest.update_dump()
+    superproject.manifest.dump()
     logger.print_info_line(
         project_entry.name,
         f"Added '{project_entry.name}' to manifest '{superproject.manifest.path}'",

dfetch/commands/freeze.py

@@ -135,5 +135,5 @@ def __call__(self, args: argparse.Namespace) -> None:
                         manifest_updated = True
 
             if manifest_updated:
-                superproject.manifest.update_dump()
+                superproject.manifest.dump()
                 logger.info(f"Updated manifest ({manifest_path}) in {os.getcwd()}")

dfetch/commands/import_.py

@@ -58,11 +58,19 @@ def __call__(self, _: argparse.Namespace) -> None:
                     project.set_remote(remote)
                     break
 
+        single_remote = len(remotes) == 1
+        project_dicts = []
+        for p in projects:
+            d = p.as_yaml()
+            if single_remote:
+                d.pop("remote", None)
+            project_dicts.append(d)
+
         manifest_data = {
             "manifest": {
                 "version": "0.0",
                 "remotes": [r.as_yaml() for r in remotes],
-                "projects": [p.as_yaml() for p in projects],
+                "projects": project_dicts,
             }
         }
         manifest = Manifest.from_yaml(yaml.dump(manifest_data, sort_keys=False))

dfetch/manifest/manifest.py

@@ -61,12 +61,36 @@ def _ensure_unique(seq: list[dict[str, Any]], key: str, context: str) -> None:
 
 
 def _yaml_str(value: str) -> str | SingleQuotedScalarString:
-    """Return SingleQuotedScalarString if value would be misread as non-string."""
-    if not isinstance(yaml.safe_load(value), str):
+    """Return SingleQuotedScalarString if value would be misread as non-string.
+
+    If ``yaml.safe_load`` cannot parse *value* at all (e.g. it starts with a
+    ``%`` directive marker) the value is quoted to be safe.
+    """
+    try:
+        if not isinstance(yaml.safe_load(value), str):
+            return SingleQuotedScalarString(value)
+    except yaml.YAMLError:
         return SingleQuotedScalarString(value)
     return value
 
 
+def _ensure_blank_line_after(ca_map: CommentedMap, key: str) -> None:
+    """Ensure there is exactly one blank line after *key*'s value in *ca_map*.
+
+    Blank lines in ruamel are stored as trailing ``CommentToken`` values at
+    position ``[2]`` of ``ca_map.ca.items[key]``.  If no token exists one is
+    created; if one already exists its trailing newlines are normalised to two
+    (the line ending of the value itself plus the blank line).
+    """
+    items = ca_map.ca.items.get(key, [None, None, None, None])
+    token = items[2]
+    if token is None:
+        items[2] = CommentToken("\n\n", CommentMark(0), None)
+        ca_map.ca.items[key] = items
+    elif not token.value.endswith("\n\n"):
+        token.value = token.value.rstrip("\n") + "\n\n"
+
+
 @dataclass
 class ManifestEntryLocation:
     """Location of an entry in the manifest file."""
@@ -133,10 +157,15 @@ class ManifestDict(TypedDict, total=True):  # pylint: disable=too-many-ancestors
     ]
 
 
-class Manifest:  # pylint: disable=too-many-instance-attributes
+class Manifest:
     """Manifest describing all the modules information.
 
     This class is created from the manifest file a project has.
+
+    ``self._doc`` is the single source of truth: all state is read from and written
+    to the underlying YAML document.  The only cached fields are ``__path``,
+    ``__relative_path``, ``__version`` (immutable after construction), and
+    ``_default_remote_name`` (also immutable: remotes are never added at runtime).
     """
 
     CURRENT_VERSION = "0.0"
@@ -159,50 +188,54 @@ def __init__(
         manifest_data: dict[str, Any] = cast(dict[str, Any], doc.data)["manifest"]
         self.__version: str = str(manifest_data.get("version", self.CURRENT_VERSION))
 
-        # Ensure version is always written as a quoted string by update_dump().
+        # Ensure version is always written as a quoted string by dump().
         doc["manifest"].as_marked_up()["version"] = SingleQuotedScalarString(
             self.__version
         )
 
-        remotes = manifest_data.get("remotes", [])
-        projects = manifest_data["projects"]
-
-        _ensure_unique(remotes, "name", "manifest.remotes")
-        _ensure_unique(projects, "name", "manifest.projects")
-        _ensure_unique(projects, "dst", "manifest.projects")
+        remotes_raw = manifest_data.get("remotes", [])
+        projects_raw = manifest_data["projects"]
 
-        self._remotes, default_remotes = self._determine_remotes(remotes)
+        _ensure_unique(remotes_raw, "name", "manifest.remotes")
+        _ensure_unique(projects_raw, "name", "manifest.projects")
+        _ensure_unique(projects_raw, "dst", "manifest.projects")
 
+        # Determine and cache the default remote name (remotes don't change at runtime).
+        remotes_dict, default_remotes = self._determine_remotes(remotes_raw)
         if not default_remotes:
-            default_remotes = list(self._remotes.values())[0:1]
-
+            default_remotes = list(remotes_dict.values())[0:1]
         self._default_remote_name = (
             "" if not default_remotes else default_remotes[0].name
         )
 
-        self._projects = self._init_projects(projects)
+        # Ensure blank lines appear before 'remotes:' and 'projects:' when dumped.
+        self._ensure_section_spacing()
+
+        # Re-apply quoting to scalars whose style was stripped by strictyaml.
+        self._normalize_string_scalars()
 
-    def _init_projects(
+    def _build_projects(
         self,
         projects: Sequence[
             ProjectEntryDict
             | ProjectEntry
             | dict[str, str | list[str] | dict[str, str]]
         ],
     ) -> dict[str, ProjectEntry]:
-        """Iterate over projects from manifest and initialize ProjectEntries from it.
+        """Build a mapping of name → ProjectEntry from raw project data.
 
         Args:
-            projects (Sequence[
-                Union[ProjectEntryDict, ProjectEntry, Dict[str, Union[str, list[str], dict[str, str]]]]
-            ]): Iterable with projects
+            projects: Iterable of project dicts or ProjectEntry objects.
 
         Raises:
-            RuntimeError: Project unknown
+            KeyError: A project dict is missing ``name``.
+            TypeError: A project name is not a string.
+            RuntimeError: A project references an unknown remote.
 
         Returns:
-            Dict[str, ProjectEntry]: Dictionary with key: Name of project, Value: ProjectEntry
+            Dict mapping project name to ProjectEntry.
         """
+        remotes = {r.name: r for r in self.remotes}
         _projects: dict[str, ProjectEntry] = {}
 
         for project in projects:
@@ -223,11 +256,11 @@ def _init_projects(
 
             if last_project.remote:
                 try:
-                    last_project.set_remote(self._remotes[last_project.remote])
+                    last_project.set_remote(remotes[last_project.remote])
                 except KeyError as exc:
                     raise RuntimeError(
                         f"Remote {last_project.remote} of {last_project.name} wasn't found "
-                        f"in {list(self._remotes.keys())}!",
+                        f"in {list(remotes.keys())}!",
                     ) from exc
 
         return _projects
@@ -310,17 +343,19 @@ def version(self) -> str:
     @property
     def projects(self) -> Sequence[ProjectEntry]:
         """Get a list of Projects from the manifest."""
-        return list(self._projects.values())
+        projects_mu = self._doc["manifest"]["projects"].as_marked_up()
+        return list(self._build_projects(projects_mu).values())
 
     def selected_projects(self, names: Sequence[str]) -> Sequence[ProjectEntry]:
         """Get a list of Projects from the manifest with the given names."""
-        projects = (
-            [p for p in self._projects.values() if p.name in names]
+        all_projects = self.projects
+        result = (
+            [p for p in all_projects if p.name in names]
             if names
-            else list(self._projects.values())
+            else list(all_projects)
         )
-        self._check_all_names_found(names, projects)
-        return projects
+        self._check_all_names_found(names, result)
+        return result
 
     def _check_all_names_found(
         self, names: Sequence[str], projects: Sequence[ProjectEntry]
@@ -331,23 +366,23 @@ def _check_all_names_found(
             return
         found = {project.name for project in projects}
         unfound = [name for name in unique_names if name not in found]
-        possibles = [project.name for project in self._projects.values()]
+        possibles = [project.name for project in self.projects]
         raise RequestedProjectNotFoundError(unfound, possibles)
 
     @property
     def remotes(self) -> Sequence[Remote]:
         """Get a list of Remotes from the manifest."""
-        return list(self._remotes.values())
+        manifest_mu = self._doc["manifest"].as_marked_up()
+        remotes_dict, _ = self._determine_remotes(manifest_mu.get("remotes", []))
+        return list(remotes_dict.values())
 
     def __repr__(self) -> str:
         """Get string representing this object."""
         return str(self._as_dict())
 
     def _as_dict(self) -> dict[str, ManifestDict]:
         """Get this manifest as dict."""
-        remotes: Sequence[RemoteDict] = [
-            remote.as_yaml() for remote in self._remotes.values()
-        ]
+        remotes: Sequence[RemoteDict] = [remote.as_yaml() for remote in self.remotes]
 
         if len(remotes) == 1:
             remotes[0].pop("default", None)
@@ -377,26 +412,17 @@ def _as_dict(self) -> dict[str, ManifestDict]:
             }
         }
 
-    def dump(self, path: str) -> None:
-        """Dump metadata file to correct path."""
-        with open(path, "w+", encoding="utf-8") as manifest_file:
-            yaml.dump(
-                self._as_dict(),
-                manifest_file,
-                Dumper=ManifestDumper,
-                sort_keys=False,
-                line_break=os.linesep,
-            )
-
-    def update_dump(self) -> None:
-        """Dump the manifest to its path, using the original text as a base to preserve formatting and comments."""
-        if not self.__path:
-            raise RuntimeError("Cannot update dump of manifest with no path")
-
-        updated_text = self._doc.as_yaml()
+    def dump(self, path: str | None = None) -> None:
+        """Write the manifest to *path*, preserving formatting and comments.
 
-        with open(self.__path, "w", encoding="utf-8", newline="") as manifest_file:
-            manifest_file.write(updated_text)
+        If *path* is omitted the manifest is written back to the file it was
+        loaded from.  Raises ``RuntimeError`` if no path is available.
+        """
+        target = path or self.__path
+        if not target:
+            raise RuntimeError("Cannot dump manifest with no path")
+        with open(target, "w", encoding="utf-8", newline="") as manifest_file:
+            manifest_file.write(self._doc.as_yaml())
 
     def find_name_in_manifest(self, name: str) -> ManifestEntryLocation:
         """Find the location of a project name in the manifest.
@@ -417,12 +443,57 @@ def find_name_in_manifest(self, name: str) -> ManifestEntryLocation:
         raise RuntimeError(f"{name} was not found in the manifest!")
 
     # ---------------- YAML updates ----------------
+    def _normalize_string_scalars(self) -> None:
+        """Re-apply ``SingleQuotedScalarString`` to any string that would be misread.
+
+        strictyaml strips quoting-style information during schema validation, so
+        a value like ``'176'`` (single-quoted in the source YAML) becomes a plain
+        Python ``str`` in the ruamel layer.  ruamel then serialises it without
+        quotes, producing the integer ``176``.  We walk all top-level string
+        fields in every project and remote entry and restore the quoting wherever
+        ``_yaml_str`` determines it is needed.
+        """
+        manifest_mu = self._doc["manifest"].as_marked_up()
+        for entry in manifest_mu.get("projects", []):
+            for key in list(entry.keys()):
+                if isinstance(entry[key], str):
+                    entry[key] = _yaml_str(entry[key])
+        for entry in manifest_mu.get("remotes", []):
+            for key in list(entry.keys()):
+                if isinstance(entry[key], str):
+                    entry[key] = _yaml_str(entry[key])
+
+    def _ensure_section_spacing(self) -> None:
+        """Ensure blank lines appear before ``remotes:`` and ``projects:`` and between entries.
+
+        Blank lines in ruamel are stored as trailing ``CommentToken`` values on
+        the *last field* of each ``CommentedMap`` entry.  Specifically:
+
+        * Before ``remotes:``       — trailing token on ``version``'s value.
+        * Before ``projects:``      — trailing token on the last field of the
+                                      last remote (or on ``version`` when there
+                                      are no remotes).
+        * Between project entries   — trailing token on the last field of every
+                                      project entry except the final one.
+        """
+        manifest_mu = self._doc["manifest"].as_marked_up()
+        _ensure_blank_line_after(manifest_mu, "version")
+
+        remotes_mu = manifest_mu.get("remotes")
+        if remotes_mu:
+            last_remote = remotes_mu[-1]
+            _ensure_blank_line_after(last_remote, list(last_remote.keys())[-1])
+
+        projects_mu = manifest_mu.get("projects", [])
+        for project in projects_mu[:-1]:  # all entries except the last
+            _ensure_blank_line_after(project, list(project.keys())[-1])
+
     def append_project_entry(self, project_entry: "ProjectEntry") -> None:
         """Append *project_entry* to the projects list in-memory.
 
         The new entry is formatted the same way as the existing YAML in the
         document (2-space indent under ``projects:``).  Call
-        :meth:`update_dump` afterwards to persist the change to disk.
+        :meth:`dump` afterwards to persist the change to disk.
         """
         projects_mu = self._doc["manifest"]["projects"].as_marked_up()
         projects_mu.append(CommentedMap(project_entry.as_yaml()))
@@ -433,7 +504,6 @@ def append_project_entry(self, project_entry: "ProjectEntry") -> None:
             None,
             None,
         ]
-        self._projects[project_entry.name] = ProjectEntry.copy(project_entry)
 
     def update_project_version(self, project: ProjectEntry) -> None:
         """Update a project's version in the manifest in-place, preserving layout, comments, and line endings."""
@@ -534,25 +604,3 @@ def find_remote_for_url(self, remote_url: str) -> Remote | None:
             if target.startswith(remote_base):
                 return remote
         return None
-
-
-class ManifestDumper(yaml.SafeDumper):  # pylint: disable=too-many-ancestors
-    """Dump a manifest YAML.
-
-    HACK: insert blank lines between top-level objects
-    inspired by https://stackoverflow.com/a/44284819/3786245
-    """
-
-    _last_additional_break = 0
-
-    def write_line_break(self, data: Any = None) -> None:
-        """Write a line break."""
-        super().write_line_break(data)  # type: ignore[unused-ignore, no-untyped-call]
-
-        if len(self.indents) == 2 and getattr(self.event, "value", "") != "version":
-            super().write_line_break()  # type: ignore[unused-ignore, no-untyped-call]
-
-        if len(self.indents) == 3 and self._last_additional_break != 2:
-            super().write_line_break()  # type: ignore[unused-ignore, no-untyped-call]
-
-        self._last_additional_break = len(self.indents)