Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 39 additions & 11 deletions src/towncrier/_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved

Issues 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.
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved

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]
Expand Down
8 changes: 8 additions & 0 deletions src/towncrier/newsfragments/608.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Issues are now sorted by issue number even if they have non-digit characters.
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
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.
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
74 changes: 73 additions & 1 deletion src/towncrier/test/test_builder.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -132,3 +134,73 @@ def test_orphan_all_digits(self):
parse_newfragment_basename("+123.feature", ["feature"]),
("+123", "feature", 0),
)


class TestIssueOrdering(TestCase):
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
"""
Tests to ensure that issues are ordered correctly in the output.

This tests both ordering of issues within a fragment and ordering of
fragments within a section.
"""

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 first by the non-text component, then by their number.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess here "issues" means "news fragments"

Suggested change
Issues are ordered first by the non-text component, then by their number.
News fragments are ordered first by the non-text component, then by their number.


For backwards compatibility, issues with no number are grouped first and issues
which are only a number are grouped last.

Orhpan issues are always last, sorted by their fragment text.
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
"""
output = self.render(
{
"": {
"feature": {
"Added Cheese": ["10", "gh-25", "gh-3", "4"],
"Added Fish": [],
"Added Bread": [],
"Added Milk": ["gh-1"],
"Added Eggs": ["gh-2", "random"],
}
}
},
)
# "Eggs" are first because they have an issue with no number, and the first
# issue for each fragment is what is used for sorting the overall list.
assert output == dedent(
"""
## feature
- Added Eggs (random, gh-2)
- Added Milk (gh-1)
- Added Cheese (gh-3, gh-25, #4, #10)
- Added Bread
- Added Fish
"""
)
Loading