Refactor issue_key function to sort issues in a human-friendly way

src/towncrier/_builder.py

@@ -5,11 +5,12 @@
 from __future__ import annotations
 
 import os
+import re
 import textwrap
 
 from collections import defaultdict
 from pathlib import Path
-from typing import Any, DefaultDict, Iterable, Iterator, Mapping, Sequence
+from typing import Any, DefaultDict, Iterable, Iterator, Mapping, NamedTuple, Sequence
 
 from jinja2 import Template
 
@@ -180,18 +181,45 @@ def split_fragments(
     return output
 
 
-def issue_key(issue: str) -> tuple[int, str]:
-    # We want integer issues to sort as integers, and we also want string
-    # issues to sort as strings. We arbitrarily put string issues before
-    # integer issues (hopefully no-one uses both at once).
-    try:
-        return (int(issue), "")
-    except Exception:
-        # Maybe we should sniff strings like "gh-10" -> (10, "gh-10")?
-        return (-1, issue)
+class IssueParts(NamedTuple):
+    is_digit: bool
+    has_digit: bool
+    non_digit_part: str
+    number: int
 
 
-def entry_key(entry: tuple[str, Sequence[str]]) -> tuple[str, list[tuple[int, str]]]:
+def issue_key(issue: str) -> IssueParts:
+    """
+    Used to sort issues in a human-friendly way.
+
+    Issues are grouped their non-integer part, then sorted by their integer part.
+
+    For backwards compatible consistency, issues without no number are sorted first and
+    digit only issues are sorted last.
+
+    For example::
+
+    >>> sorted(["2", "#11", "#3", "gh-10", "gh-4", "omega", "alpha"], key=issue_key)
+    ['alpha', 'omega', '#3', '#11', 'gh-4', 'gh-10', '2']
+    """
+    if issue.isdigit():
+        return IssueParts(
+            is_digit=True, has_digit=True, non_digit_part="", number=int(issue)
+        )
+    match = re.search(r"\d+", issue)
+    if not match:
+        return IssueParts(
+            is_digit=False, has_digit=False, non_digit_part=issue, number=-1
+        )
+    return IssueParts(
+        is_digit=False,
+        has_digit=True,
+        non_digit_part=issue[: match.start()] + issue[match.end() :],
+        number=int(match.group()),
+    )
+
+
+def entry_key(entry: tuple[str, Sequence[str]]) -> tuple[str, list[IssueParts]]:
     content, issues = entry
     # Orphan news fragments (those without any issues) should sort last by content.
     return "" if issues else content, [issue_key(issue) for issue in issues]

src/towncrier/newsfragments/+c8459360.feature.rst

@@ -0,0 +1,2 @@
+Issues are now correctly sorted by issue number even if they have non-digit characters.
+For example, "- some issue (gh-2, gh-10)".

src/towncrier/test/test_builder.py

@@ -1,9 +1,11 @@
 # Copyright (c) Povilas Kanapickas, 2019
 # See LICENSE for details.
 
+from textwrap import dedent
+
 from twisted.trial.unittest import TestCase
 
-from .._builder import parse_newfragment_basename
+from .._builder import parse_newfragment_basename, render_fragments
 
 
 class TestParseNewsfragmentBasename(TestCase):
@@ -132,3 +134,55 @@ def test_orphan_all_digits(self):
             parse_newfragment_basename("+123.feature", ["feature"]),
             ("+123", "feature", 0),
         )
+
+
+class TestIssueOrdering(TestCase):
+    template = dedent("""
+    {% for section_name, category in sections.items() %}
+    {% if section_name %}# {{ section_name }}{% endif %}
+    {%- for category_name, issues in category.items() %}
+    ## {{ category_name }}
+    {% for issue, numbers in issues.items() %}
+    - {{ issue }}{% if numbers %} ({{ numbers|join(', ') }}){% endif %}
+
+    {% endfor %}
+    {% endfor -%}
+    {% endfor -%}
+    """)
+
+    def render(self, fragments):
+        return render_fragments(
+            template=self.template,
+            issue_format=None,
+            fragments=fragments,
+            definitions={},
+            underlines=[],
+            wrap=False,
+            versiondata={},
+        )
+
+    def test_ordering(self):
+        """
+        Issues are ordered by their number, not lexicographically.
+        """
+        output = self.render(
+            {
+                "": {
+                    "feature": {
+                        "Added Cheese": ["10", "gh-3", "4"],
+                        "Added Fish": [],
+                        "Added Bread": [],
+                        "Added Milk": ["gh-1"],
+                        "Added Eggs": ["gh-2", "random"],
+                    }
+                }
+            },
+        )
+        assert output == dedent("""
+            ## feature
+            - Added Eggs (random, gh-2)
+            - Added Milk (gh-1)
+            - Added Cheese (gh-3, #4, #10)
+            - Added Bread
+            - Added Fish
+""")