Various fixes and improvements

.pre-commit-config.yaml

@@ -4,19 +4,19 @@
 
 repos:
   - repo: https://github.com/asottile/pyupgrade
-    rev: v2.32.1
+    rev: v2.37.1
     hooks:
       - id: pyupgrade
   - repo: https://github.com/psf/black
-    rev: 22.3.0
+    rev: 22.6.0
     hooks:
       - id: black
   - repo: https://github.com/pre-commit/mirrors-prettier
-    rev: v2.6.2
+    rev: v2.7.1
     hooks:
       - id: prettier
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.2.0
+    rev: v4.3.0
     hooks:
       - id: check-added-large-files
       - id: check-ast
@@ -44,7 +44,7 @@ repos:
       - id: isort
         args: [--profile, black]
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v0.960
+    rev: v0.961
     hooks:
       - id: mypy
         args: [--show-error-codes, --ignore-missing-imports]

specfile/exceptions.py

@@ -7,8 +7,6 @@
 class SpecfileException(Exception):
     """Something went wrong during our execution."""
 
-    pass
-
 
 class RPMException(SpecfileException):
     """Exception related to RPM."""
@@ -31,3 +29,11 @@ class MacroRemovalException(SpecfileException):
 
 class MacroOptionsException(SpecfileException):
     """Exception related to processing macro options."""
+
+
+class DuplicateSourceException(SpecfileException):
+    """Exception related to adding a duplicate source."""
+
+
+class SourceNumberException(SpecfileException):
+    """Exception related to source numbers."""

specfile/sourcelist.py

@@ -23,6 +23,11 @@ def __init__(self, location: str, comments: Comments) -> None:
         self.location = location
         self.comments = comments.copy()
 
+    def __eq__(self, other: object) -> bool:
+        if not isinstance(other, SourcelistEntry):
+            return NotImplemented
+        return self.location == other.location and self.comments == other.comments
+
     def __repr__(self) -> str:
         comments = repr(self.comments)
         return f"SourcelistEntry('{self.location}', {comments})"

specfile/sources.py

@@ -8,7 +8,7 @@
 from pathlib import Path
 from typing import Iterable, List, Optional, Tuple, Union, cast, overload
 
-from specfile.exceptions import SpecfileException
+from specfile.exceptions import DuplicateSourceException
 from specfile.rpm import Macros
 from specfile.sourcelist import Sourcelist, SourcelistEntry
 from specfile.tags import Comments, Tag, Tags
@@ -100,14 +100,25 @@ def _extract_number(self) -> Optional[str]:
     @property
     def number(self) -> int:
         """Source number."""
-        return self._number or int(self._extract_number() or 0)
+        if self._number is not None:
+            return self._number
+        return int(self._extract_number() or 0)
 
     @property
     def number_digits(self) -> int:
-        """Number of digits in the source number."""
-        if self._number:
+        """
+        Gets number of digits in the source number.
+
+        Returns 0 if the source has no number, 1 if the source number
+        has no leading zeros and the actual number of digits if there are
+        any leading zeros.
+        """
+        if self._number is not None:
             return 0
-        return len(self._extract_number() or "")
+        number = self._extract_number()
+        if not number:
+            return 0
+        return len(number) if number.startswith("0") else 1
 
     @property
     def location(self) -> str:
@@ -378,13 +389,14 @@ def _get_tag_format(
             Tuple in the form of (name, separator).
         """
         prefix = self.PREFIX.capitalize()
-        if self._detect_implicit_numbering():
+        if number_digits_override is not None:
+            number_digits = number_digits_override
+        else:
+            number_digits = reference.number_digits
+        if self._detect_implicit_numbering() or number_digits == 0:
             suffix = ""
         else:
-            if number_digits_override is not None:
-                suffix = f"{number:0{number_digits_override}}"
-            else:
-                suffix = f"{number:0{reference.number_digits}}"
+            suffix = f"{number:0{number_digits}}"
         name = f"{prefix}{suffix}"
         diff = len(reference._tag.name) - len(name)
         if diff >= 0:
@@ -404,23 +416,34 @@ def _get_initial_tag_setup(self, number: int = 0) -> Tuple[int, str, str]:
             Tuple in the form of (index, name, separator).
         """
         prefix = self.PREFIX.capitalize()
-        if self._default_to_implicit_numbering:
+        if (
+            self._default_to_implicit_numbering
+            or self._default_source_number_digits == 0
+        ):
             suffix = ""
         else:
             suffix = f"{number:0{self._default_source_number_digits}}"
         return len(self._tags) if self._tags else 0, f"{prefix}{suffix}", ": "
 
-    def _deduplicate_tag_names(self) -> None:
-        """Eliminates duplicate numbers in source tag names."""
+    def _deduplicate_tag_names(self, start: int = 0) -> None:
+        """
+        Eliminates duplicate numbers in source tag names.
+
+        Args:
+            start: Starting index, defaults to the first source tag.
+        """
         tags = self._get_tags()
         if not tags:
             return
-        tag_sources = sorted(list(zip(*tags))[0], key=lambda ts: ts.number)
+        tag_sources = list(zip(*tags[start:]))[0]
         for ts0, ts1 in zip(tag_sources, tag_sources[1:]):
-            if ts1.number <= ts0.number:
-                ts1._tag.name, ts1._tag._separator = self._get_tag_format(
-                    ts0, ts0.number + 1
-                )
+            if ts1.number == ts0.number:
+                if ts1._number is not None:
+                    ts1._number = ts0.number + 1
+                else:
+                    ts1._tag.name, ts1._tag._separator = self._get_tag_format(
+                        ts1, ts0.number + 1
+                    )
 
     def insert(self, i: int, location: str) -> None:
         """
@@ -431,11 +454,11 @@ def insert(self, i: int, location: str) -> None:
             location: Location of the new source.
 
         Raises:
-            SpecfileException if duplicates are disallowed and there
-            already is a source with the same location.
+            DuplicateSourceException if duplicates are disallowed and there
+              already is a source with the same location.
         """
         if not self._allow_duplicates and location in self:
-            raise SpecfileException(f"Source '{location}' already exists")
+            raise DuplicateSourceException(f"Source '{location}' already exists")
         items = self._get_items()
         if i > len(items):
             i = len(items)
@@ -453,7 +476,7 @@ def insert(self, i: int, location: str) -> None:
                     index,
                     Tag(name, location, Macros.expand(location), separator, Comments()),
                 )
-                self._deduplicate_tag_names()
+                self._deduplicate_tag_names(i)
             else:
                 container.insert(
                     index,
@@ -480,11 +503,11 @@ def insert_numbered(self, number: int, location: str) -> int:
             Index of the newly inserted source.
 
         Raises:
-            SpecfileException if duplicates are disallowed and there
-            already is a source with the same location.
+            DuplicateSourceException if duplicates are disallowed and there
+              already is a source with the same location.
         """
         if not self._allow_duplicates and location in self:
-            raise SpecfileException(f"Source '{location}' already exists")
+            raise DuplicateSourceException(f"Source '{location}' already exists")
         tags = self._get_tags()
         if tags:
             # find the nearest source tag
@@ -501,7 +524,7 @@ def insert_numbered(self, number: int, location: str) -> int:
         self._tags.insert(
             index, Tag(name, location, Macros.expand(location), separator, Comments())
         )
-        self._deduplicate_tag_names()
+        self._deduplicate_tag_names(i)
         return i
 
     def remove(self, location: str) -> None:
@@ -515,6 +538,21 @@ def remove(self, location: str) -> None:
             if source.location == location:
                 del container[index]
 
+    def remove_numbered(self, number: int) -> None:
+        """
+        Removes a source by number.
+
+        Args:
+            number: Number of the source to be removed.
+        """
+        items = self._get_items()
+        try:
+            container, index = next((c, i) for s, c, i in items if s.number == number)
+        except StopIteration:
+            pass
+        else:
+            del container[index]
+
     def count(self, location: str) -> int:
         """
         Counts sources by location.

specfile/specfile.py

@@ -12,7 +12,7 @@
 import arrow
 
 from specfile.changelog import Changelog, ChangelogEntry
-from specfile.exceptions import SpecfileException
+from specfile.exceptions import SourceNumberException, SpecfileException
 from specfile.prep import Prep
 from specfile.rpm import RPM, Macros
 from specfile.sections import Sections
@@ -416,3 +416,39 @@ def set_version_and_release(self, version: str, release: str = "1") -> None:
         with self.tags() as tags:
             tags.version.value = version
             tags.release.value = self._get_updated_release(tags.release.value, release)
+
+    def add_patch(
+        self,
+        location: str,
+        number: Optional[int] = None,
+        comment: Optional[str] = None,
+        initial_number: int = 0,
+        number_digits: int = 4,
+    ) -> None:
+        """
+        Adds a patch.
+
+        Args:
+            location: Patch location (filename or URL).
+            number: Patch number. It will be auto-assigned if not specified.
+              If specified, it must be higher than any existing patch number.
+            comment: Associated comment.
+            initial_number: Auto-assigned number to start with if there are no patches.
+            number_digits: Number of digits in the patch number.
+
+        Raises:
+            SourceNumberException when the specified patch number is not higher
+              than any existing patch number.
+        """
+        with self.patches(default_source_number_digits=number_digits) as patches:
+            highest_number = max((p.number for p in patches), default=-1)
+            if number is not None:
+                if number <= highest_number:
+                    raise SourceNumberException(
+                        "Patch number must be higher than any existing patch number"
+                    )
+            else:
+                number = max(highest_number + 1, initial_number)
+            index = patches.insert_numbered(number, location)
+            if comment:
+                patches[index].comments.extend(comment.splitlines())

specfile/tags.py

@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: MIT
 
 import collections
+import itertools
 import re
 from typing import Any, Iterable, List, Optional, Union, overload
 
@@ -124,6 +125,11 @@ def __contains__(self, item: object) -> bool:
             return item in [c.text for c in self.data]
         return item in self.data
 
+    @property
+    def raw(self) -> List[str]:
+        """List of comment texts"""
+        return [c.text for c in self.data]
+
     @overload
     def __getitem__(self, i: SupportsIndex) -> Comment:
         pass
@@ -340,14 +346,29 @@ def __getitem__(self, i):
 
     def __delitem__(self, i: Union[SupportsIndex, slice]) -> None:
         def delete(index):
-            preceding_lines = self.data[index].comments._preceding_lines.copy()
+            preceding_lines = self.data[index].comments._preceding_lines[:]
             del self.data[index]
+            # preserve preceding lines of the deleted tag but compress empty lines
             if index < len(self.data):
-                self.data[index].comments._preceding_lines = (
-                    preceding_lines + self.data[index].comments._preceding_lines
-                )
+                lines = self.data[index].comments._preceding_lines
             else:
-                self._remainder = preceding_lines + self._remainder
+                lines = self._remainder
+            delimiter = []
+            if preceding_lines and not preceding_lines[-1] or lines and not lines[0]:
+                delimiter.append("")
+            lines[:] = (
+                list(
+                    reversed(
+                        list(
+                            itertools.dropwhile(
+                                lambda l: not l, reversed(preceding_lines)
+                            )
+                        )
+                    )
+                )
+                + delimiter
+                + list(itertools.dropwhile(lambda l: not l, lines))
+            )
 
         if isinstance(i, slice):
             for index in reversed(range(len(self.data))[i]):

tests/constants.py

@@ -12,5 +12,6 @@
 SPEC_AUTOPATCH = DATA_DIR / "spec_autopatch"
 SPEC_PATCHLIST = DATA_DIR / "spec_patchlist"
 SPEC_MULTIPLE_SOURCES = DATA_DIR / "spec_multiple_sources"
+SPEC_COMMENTED_PATCHES = DATA_DIR / "spec_commented_patches"
 
 SPECFILE = "test.spec"

tests/data/spec_commented_patches/patch0.patch

@@ -0,0 +1,18 @@
+From e5e63915ae9cfb5a40bccbd53514f211b5c8e63b Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Nikola=20Forr=C3=B3?= <nforro@redhat.com>
+Date: Wed, 16 Mar 2022 10:29:59 +0100
+Subject: [PATCH 1/7] patch0
+
+---
+ test.txt | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/test.txt b/test.txt
+index 9daeafb..dec2cbe 100644
+--- a/test.txt
++++ b/test.txt
+@@ -1 +1,2 @@
+ test
++test
+--
+2.35.1

tests/data/spec_commented_patches/patch1.patch

@@ -0,0 +1,19 @@
+From 10cd6fe9acec1233c3456871f26b122df901d160 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Nikola=20Forr=C3=B3?= <nforro@redhat.com>
+Date: Wed, 16 Mar 2022 10:30:15 +0100
+Subject: [PATCH 2/7] patch1
+
+---
+ test.txt | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/test.txt b/test.txt
+index dec2cbe..0867e73 100644
+--- a/test.txt
++++ b/test.txt
+@@ -1,2 +1,3 @@
+ test
+ test
++test
+--
+2.35.1