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 all 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
8 changes: 4 additions & 4 deletions NEWS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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>`_!

Expand Down
2 changes: 1 addition & 1 deletion docs/customization/newsfile.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions docs/tutorial.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
92 changes: 60 additions & 32 deletions src/towncrier/_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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.
Expand All @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion src/towncrier/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
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 @@
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.
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion src/towncrier/test/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading
Loading