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

) * Refactor issue_key function to sort issues in a human-friendly way * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Rename newsfragment * Small improvement to test to show how text with numeric issues are sorted * Update src/towncrier/_builder.py docstring grammar Co-authored-by: Adi Roiban <[email protected]> * clarify new behaviour in newsfragment * Add some docstrings/comments to tests * linelength fix * Clarify news fragments vs tickets Co-authored-by: Adi Roiban <[email protected]> * Consistent use of "issue" rather than "ticket" * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * typo --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Adi Roiban <[email protected]>
twisted · Jun 13, 2024 · be32e6b · be32e6b
1 parent 0b7f542
commit be32e6b
Show file tree

Hide file tree

Showing 9 changed files with 163 additions and 55 deletions.
diff --git a/NEWS.rst b/NEWS.rst
@@ -15,9 +15,9 @@ Bugfixes
 --------
 
 - ``build`` now treats a missing fragments directory the same as an empty one, consistent with other operations. (`#538 <https://github.com/twisted/towncrier/issues/538>`_)
-- Fragments with filenames like `fix-1.2.3.feature` are now associated with the ticket `fix-1.2.3`.
-  In previous versions they were incorrectly associated to ticket `3`. (`#562 <https://github.com/twisted/towncrier/issues/562>`_)
-- Orphan newsfragments containing numeric values are no longer accidentally associated to tickets. In previous versions the orphan marker was ignored and the newsfragment was associated to a ticket having the last numerical value from the filename. (`#562 <https://github.com/twisted/towncrier/issues/562>`_)
+- Fragments with filenames like `fix-1.2.3.feature` are now associated with the issue `fix-1.2.3`.
+  In previous versions they were incorrectly associated to issue `3`. (`#562 <https://github.com/twisted/towncrier/issues/562>`_)
+- Orphan newsfragments containing numeric values are no longer accidentally associated to issues. In previous versions the orphan marker was ignored and the newsfragment was associated to an issue having the last numerical value from the filename. (`#562 <https://github.com/twisted/towncrier/issues/562>`_)
 
 
 Misc
@@ -248,7 +248,7 @@ towncrier 21.3.0.rc1 (2021-03-21)
 Features
 --------
 
-- Ticket number from file names will be stripped down to avoid ticket links such as ``#007``. (`#126 <https://github.com/twisted/towncrier/issues/126>`_)
+- Issue number from file names will be stripped down to avoid issue links such as ``#007``. (`#126 <https://github.com/twisted/towncrier/issues/126>`_)
 - Allow definition of the project ``version`` and ``name`` in the configuration file.
   This allows use of towncrier seamlessly with non-Python projects. (`#165 <https://github.com/twisted/towncrier/issues/165>`_)
 - Improve news fragment file name parsing to allow using file names like

diff --git a/README.rst b/README.rst
@@ -30,7 +30,7 @@ Philosophy
 That is, by duplicating what has changed from the "developer log" (which may contain complex information about the original issue, how it was fixed, who authored the fix, and who reviewed the fix) into a "news fragment" (a small file containing just enough information to be useful to end users), ``towncrier`` can produce a digest of the changes which is valuable to those who may wish to use the software.
 These fragments are also commonly called "topfiles" or "newsfiles".
 
-``towncrier`` works best in a development system where all merges involve closing a ticket.
+``towncrier`` works best in a development system where all merges involve closing an issue.
 
 To get started, check out our `tutorial <https://towncrier.readthedocs.io/en/latest/tutorial.html>`_!
 

diff --git a/docs/customization/newsfile.rst b/docs/customization/newsfile.rst
@@ -4,7 +4,7 @@ Customizing the News File Output
 Adding Content Above ``towncrier``
 ----------------------------------
 
-If you wish to have content at the top of the news file (for example, to say where you can find the tickets), you can use a special rST comment to tell ``towncrier`` to only update after it.
+If you wish to have content at the top of the news file (for example, to say where you can find the issues), you can use a special rST comment to tell ``towncrier`` to only update after it.
 In your existing news file (e.g. ``NEWS.rst``), add the following line above where you want ``towncrier`` to put content:
 
 .. code-block:: restructuredtext

diff --git a/docs/tutorial.rst b/docs/tutorial.rst
@@ -89,24 +89,24 @@ The five default types are:
 - ``bugfix``: Signifying a bug fix.
 - ``doc``: Signifying a documentation improvement.
 - ``removal``: Signifying a deprecation or removal of public API.
-- ``misc``: A ticket has been closed, but it is not of interest to users.
+- ``misc``: An issue has been closed, but it is not of interest to users.
 
-When you create a news fragment, the filename consists of the ticket ID (or some other unique identifier) as well as the 'type'.
+When you create a news fragment, the filename consists of the issue/ticket ID (or some other unique identifier) as well as the 'type'.
 ``towncrier`` does not care about the fragment's suffix.
 
 You can create those fragments either by hand, or using the ``towncrier create`` command.
 Let's create some example news fragments to demonstrate::
 
    $ echo 'Fixed a thing!' > src/myproject/newsfragments/1234.bugfix
    $ towncrier create --content 'Can also be ``rst`` as well!' 3456.doc.rst
-   # You can associate multiple ticket numbers with a news fragment by giving them the same contents.
+   # You can associate multiple issue numbers with a news fragment by giving them the same contents.
    $ towncrier create --content 'Can also be ``rst`` as well!' 7890.doc.rst
    $ echo 'The final part is ignored, so set it to whatever you want.' > src/myproject/newsfragments/8765.removal.txt
    $ echo 'misc is special, and does not put the contents of the file in the newsfile.' > src/myproject/newsfragments/1.misc
    $ towncrier create --edit 2.misc.rst  # starts an editor
-   $ towncrier create -c "Orphan fragments have no ticket ID." +random.bugfix.rst
+   $ towncrier create -c "Orphan fragments have no issue ID." +random.bugfix.rst
 
-For orphan news fragments (those that don't need to be linked to any ticket ID or other identifier), start the file name with ``+``.
+For orphan news fragments (those that don't need to be linked to any issue ID or other identifier), start the file name with ``+``.
 The content will still be included in the release notes, at the end of the category corresponding to the file extension::
 
    $ echo 'Fixed an unreported thing!' > src/myproject/newsfragments/+anything.bugfix
@@ -132,7 +132,7 @@ You should get an output similar to this::
    --------
 
    - Fixed a thing! (#1234)
-   - Orphan fragments have no ticket ID.
+   - Orphan fragments have no issue ID.
 
 
    Improved Documentation
@@ -167,7 +167,7 @@ To produce the news file for real, run::
 This command will remove the news files (with ``git rm``) and append the built news to the filename specified in ``pyproject.toml``, and then stage the news file changes (with ``git add``).
 It leaves committing the changes up to the user.
 
-If you wish to have content at the top of the news file (for example, to say where you can find the tickets), put your text above a rST comment that says::
+If you wish to have content at the top of the news file (for example, to say where you can find the issues), put your text above a rST comment that says::
 
   .. towncrier release notes start
 

diff --git a/src/towncrier/_builder.py b/src/towncrier/_builder.py
@@ -5,16 +5,17 @@
 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
 
 
-# Returns ticket, category and counter or (None, None, None) if the basename
+# Returns issue, category and counter or (None, None, None) if the basename
 # could not be parsed or doesn't contain a valid category.
 def parse_newfragment_basename(
     basename: str, frag_type_names: Iterable[str]
@@ -33,21 +34,21 @@ def parse_newfragment_basename(
         if parts[i] in frag_type_names:
             # Current part is a valid category according to given definitions.
             category = parts[i]
-            # Use all previous parts as the ticket number.
+            # Use all previous parts as the issue number.
             # NOTE: This allows news fragment names like fix-1.2.3.feature or
-            # something-cool.feature.ext for projects that don't use ticket
+            # something-cool.feature.ext for projects that don't use issue
             # numbers in news fragment names.
-            ticket = ".".join(parts[0:i]).strip()
-            # If the ticket is an integer, remove any leading zeros (to resolve
+            issue = ".".join(parts[0:i]).strip()
+            # If the issue is an integer, remove any leading zeros (to resolve
             # issue #126).
-            if ticket.isdigit():
-                ticket = str(int(ticket))
+            if issue.isdigit():
+                issue = str(int(issue))
             counter = 0
             # Use the following part as the counter if it exists and is a valid
             # digit.
             if len(parts) > (i + 1) and parts[i + 1].isdigit():
                 counter = int(parts[i + 1])
-            return ticket, category, counter
+            return issue, category, counter
     else:
         # No valid category found.
         return invalid
@@ -97,15 +98,15 @@ def find_fragments(
         file_content = {}
 
         for basename in files:
-            ticket, category, counter = parse_newfragment_basename(
+            issue, category, counter = parse_newfragment_basename(
                 basename, frag_type_names
             )
             if category is None:
                 continue
-            assert ticket is not None
+            assert issue is not None
             assert counter is not None
-            if orphan_prefix and ticket.startswith(orphan_prefix):
-                ticket = ""
+            if orphan_prefix and issue.startswith(orphan_prefix):
+                issue = ""
                 # Use and increment the orphan news fragment counter.
                 counter = orphan_fragment_counter[category]
                 orphan_fragment_counter[category] += 1
@@ -114,13 +115,13 @@ def find_fragments(
             fragment_filenames.append(full_filename)
             data = Path(full_filename).read_text(encoding="utf-8", errors="replace")
 
-            if (ticket, category, counter) in file_content:
+            if (issue, category, counter) in file_content:
                 raise ValueError(
                     "multiple files for {}.{} in {}".format(
-                        ticket, category, section_dir
+                        issue, category, section_dir
                     )
                 )
-            file_content[ticket, category, counter] = data
+            file_content[issue, category, counter] = data
 
         content[key] = file_content
 
@@ -153,7 +154,7 @@ def split_fragments(
     for section_name, section_fragments in fragments.items():
         section: dict[str, dict[str, list[str]]] = {}
 
-        for (ticket, category, counter), content in section_fragments.items():
+        for (issue, category, counter), content in section_fragments.items():
             if all_bullets:
                 # By default all fragmetns are append by "-" automatically,
                 # and need to be indented because of that.
@@ -168,30 +169,57 @@ def split_fragments(
 
             texts = section.setdefault(category, {})
 
-            tickets = texts.setdefault(content, [])
-            if ticket:
-                # Only add the ticket if we have one (it can be blank for orphan news
+            issues = texts.setdefault(content, [])
+            if issue:
+                # Only add the issue if we have one (it can be blank for orphan news
                 # fragments).
-                tickets.append(ticket)
-                tickets.sort()
+                issues.append(issue)
+                issues.sort()
 
         output[section_name] = section
 
     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 the issue ID inside a news fragment in a human-friendly way.
+
+    Issue IDs are grouped by 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]

diff --git a/src/towncrier/create.py b/src/towncrier/create.py
@@ -70,7 +70,7 @@ def _main(
     * .bugfix - a bug fix
     * .doc - a documentation improvement,
     * .removal - a deprecation or removal of public API,
-    * .misc - a ticket has been closed, but it is not of interest to users.
+    * .misc - an issue has been closed, but it is not of interest to users.
 
     If the FILENAME base is just '+' (to create a fragment not tied to an
     issue), it will be appended with a random hex string.

diff --git a/src/towncrier/newsfragments/608.feature.rst b/src/towncrier/newsfragments/608.feature.rst
@@ -0,0 +1,8 @@
+News fragments are now sorted by issue number even if they have non-digit characters.
+For example::
+
+    - some issue (gh-3, gh-10)
+    - another issue (gh-4)
+    - yet another issue (gh-11)
+
+The sorting algorithm groups the issues first by non-text characters and then by number.
diff --git a/src/towncrier/test/test_build.py b/src/towncrier/test/test_build.py
@@ -914,7 +914,7 @@ def test_bullet_points_false(self, runner):
         """
         When all_bullets is false, subsequent lines are not indented.
 
-        The automatic ticket number inserted by towncrier will align with the
+        The automatic issue number inserted by towncrier will align with the
         manual bullet.
         """
         os.mkdir("newsfragments")