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

Section aware create #603

Merged
merged 14 commits into from
Jun 13, 2024
Merged
5 changes: 5 additions & 0 deletions docs/cli.rst
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ If that is the entire fragment name, a random hash will be added for you::
Whether to start ``$EDITOR`` to edit the news fragment right away.
Default: ``$EDITOR`` will be started unless you also provided content.

.. option:: --section SECTION

The section to use for the news fragment.
Default: the first section with no path.
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved


``towncrier check``
-------------------
Expand Down
49 changes: 38 additions & 11 deletions src/towncrier/_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

from jinja2 import Template

from towncrier._settings.load import Config


# Returns ticket, category and counter or (None, None, None) if the basename
# could not be parsed or doesn't contain a valid category.
Expand Down Expand Up @@ -53,6 +55,35 @@ def parse_newfragment_basename(
return invalid


class FragmentsPath:
Copy link
Member

Choose a reason for hiding this comment

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

Is this expected to be public API?

I can do something like this: from towncrier.create import FragmentsPath

"""
A helper to get the full path to a fragments directory.

This is a callable that optionally takes a section directory and returns the full
path to the fragments directory for that section (or the default if no section is
provided).
"""

def __init__(self, base_directory: str, config: Config):
self.base_directory = base_directory
self.config = config
if config.directory is not None:
self.base_directory = os.path.abspath(
os.path.join(base_directory, config.directory)
)
self.append_directory = ""
else:
self.base_directory = os.path.abspath(
os.path.join(base_directory, config.package_dir, config.package)
)
self.append_directory = "newsfragments"

def __call__(self, section_directory: str = "") -> str:
return os.path.join(
self.base_directory, section_directory, self.append_directory
)


# Returns a structure like:
#
# {
Expand All @@ -69,25 +100,21 @@ def parse_newfragment_basename(
# Also returns a list of the paths that the fragments were taken from.
def find_fragments(
base_directory: str,
sections: Mapping[str, str],
fragment_directory: str | None,
frag_type_names: Iterable[str],
orphan_prefix: str | None = None,
config: Config,
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
) -> tuple[Mapping[str, Mapping[tuple[str, str, int], str]], list[str]]:
"""
Sections are a dictonary of section names to paths.
"""
get_section_path = FragmentsPath(base_directory, config)

content = {}
fragment_filenames = []
# Multiple orphan news fragments are allowed per section, so initialize a counter
# that can be incremented automatically.
orphan_fragment_counter: DefaultDict[str | None, int] = defaultdict(int)

for key, val in sections.items():
if fragment_directory is not None:
section_dir = os.path.join(base_directory, val, fragment_directory)
else:
section_dir = os.path.join(base_directory, val)
for key, section_dir in config.sections.items():
section_dir = get_section_path(section_dir)

try:
files = os.listdir(section_dir)
Expand All @@ -98,13 +125,13 @@ def find_fragments(

for basename in files:
ticket, category, counter = parse_newfragment_basename(
basename, frag_type_names
basename, config.types
)
if category is None:
continue
assert ticket is not None
assert counter is not None
if orphan_prefix and ticket.startswith(orphan_prefix):
if config.orphan_prefix and ticket.startswith(config.orphan_prefix):
ticket = ""
# Use and increment the orphan news fragment counter.
counter = orphan_fragment_counter[category]
Expand Down
19 changes: 1 addition & 18 deletions src/towncrier/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,24 +175,7 @@ def __main(

click.echo("Finding news fragments...", err=to_err)

if config.directory is not None:
fragment_base_directory = os.path.abspath(
os.path.join(base_directory, config.directory)
)
fragment_directory = None
else:
fragment_base_directory = os.path.abspath(
os.path.join(base_directory, config.package_dir, config.package)
)
fragment_directory = "newsfragments"

fragment_contents, fragment_filenames = find_fragments(
fragment_base_directory,
config.sections,
fragment_directory,
config.types,
config.orphan_prefix,
)
fragment_contents, fragment_filenames = find_fragments(base_directory, config)

click.echo("Rendering news fragments...", err=to_err)
fragments = split_fragments(
Expand Down
19 changes: 1 addition & 18 deletions src/towncrier/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,25 +106,8 @@ def __main(
click.echo("Checks SKIPPED: news file changes detected.")
sys.exit(0)

if config.directory:
fragment_base_directory = os.path.abspath(
os.path.join(base_directory, config.directory)
)
fragment_directory = None
else:
fragment_base_directory = os.path.abspath(
os.path.join(base_directory, config.package_dir, config.package)
)
fragment_directory = "newsfragments"

fragments = {
os.path.abspath(path)
for path in find_fragments(
fragment_base_directory,
config.sections,
fragment_directory,
config.types.keys(),
)[1]
os.path.abspath(path) for path in find_fragments(base_directory, config)[1]
}
fragments_in_branch = fragments & files

Expand Down
71 changes: 58 additions & 13 deletions src/towncrier/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import click

from ._builder import FragmentsPath
from ._settings import config_option_help, load_config_from_options


Expand Down Expand Up @@ -47,6 +48,11 @@
default=DEFAULT_CONTENT,
help="Sets the content of the new fragment.",
)
@click.option(
"--section",
type=str,
help="The section to create the fragment for.",
)
@click.argument("filename", default="")
def _main(
ctx: click.Context,
Expand All @@ -55,6 +61,7 @@ def _main(
filename: str,
edit: bool | None,
content: str,
section: str | None,
) -> None:
"""
Create a new news fragment.
Expand All @@ -75,7 +82,7 @@ def _main(
If the FILENAME base is just '+' (to create a fragment not tied to an
issue), it will be appended with a random hex string.
"""
__main(ctx, directory, config, filename, edit, content)
__main(ctx, directory, config, filename, edit, content, section)


def __main(
Expand All @@ -85,6 +92,7 @@ def __main(
filename: str,
edit: bool | None,
content: str,
section: str | None,
) -> None:
"""
The main entry point.
Expand All @@ -97,7 +105,47 @@ def __main(
if ext.lower() in (".rst", ".md"):
filename_ext = ext

# Get the default section.
default_section = None
if len(config.sections) == 1:
default_section = next(iter(config.sections))
else:
# If there are mulitple sections then the first without a path is the default
# section, otherwise there's no default.
for section_name, section_dir in config.sections.items():
if not section_dir:
default_section = section_name
break

if section is not None:
if section not in config.sections:
section_param = None
for p in ctx.command.params: # pragma: no branch
if p.name == "section":
section_param = p
break
expected_sections = ", ".join(f"'{s}'" for s in config.sections)
raise click.BadParameter(
f"expected one of {expected_sections}",
param=section_param,
)

if not filename:
if section is None:
sections = list(config.sections)
if len(sections) > 1:
click.echo("Pick a section:")
default_section_index = None
for i, section in enumerate(sections):
click.echo(f" {i+1}: {section or '(primary)'}")
if not default_section_index and section == default_section:
default_section_index = str(i + 1)
section_index = click.prompt(
"Section",
type=click.Choice([str(i + 1) for i in range(len(sections))]),
default=default_section_index,
)
section = sections[int(section_index) - 1]
prompt = "Issue number"
# Add info about adding orphan if config is set.
if config.orphan_prefix:
Expand Down Expand Up @@ -134,19 +182,16 @@ def __main(
if filename_parts[-1] in config.types and filename_ext:
filename += filename_ext

if config.directory:
fragments_directory = os.path.abspath(
os.path.join(base_directory, config.directory)
)
else:
fragments_directory = os.path.abspath(
os.path.join(
base_directory,
config.package_dir,
config.package,
"newsfragments",
if not section:
if default_section is None:
raise click.UsageError(
"Multiple sections defined in configuration file, all with paths."
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
" Please define a section with `--section`."
)
)
section = default_section

get_fragments_path = FragmentsPath(base_directory, config)
fragments_directory = get_fragments_path(section_directory=config.sections[section])

if not os.path.exists(fragments_directory):
os.makedirs(fragments_directory)
Expand Down
4 changes: 4 additions & 0 deletions src/towncrier/newsfragments/603.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
The ``towncrier create`` action now uses sections defined in your config (either interactively, or via the new ``--section`` option).

If you use sections and none have an empty path, you must now specify the section when creating a new news fragment.
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
If one does have an empty path, that section will be used by default.
107 changes: 107 additions & 0 deletions src/towncrier/test/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,113 @@ def test_without_filename_no_orphan_config(self, runner: CliRunner):
with open(expected) as f:
self.assertEqual(f.read(), "Edited content\n")

@with_isolated_runner
def test_sections(self, runner: CliRunner):
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
setup_simple_project(
extra_config="""
[[tool.towncrier.section]]
name = "Backend"
path = "backend"
[[tool.towncrier.section]]
name = "Frontend"
path = "frontend"
"""
)
result = runner.invoke(_main, ["123.feature.rst"])
self.assertTrue(result.exception, result.output)
self.assertEqual(
result.output,
"""\
Usage: create [OPTIONS] [FILENAME]
Try 'create --help' for help.

Error: Multiple sections defined in configuration file, all with paths.\
Please define a section with `--section`.
""",
)

result = runner.invoke(_main, ["123.feature.rst", "--section", "invalid"])
self.assertTrue(result.exception, result.output)
self.assertIn(
"Invalid value for '--section': expected one of 'Backend', 'Frontend'",
result.output,
)

result = runner.invoke(_main, ["123.feature.rst", "--section", "Frontend"])
self.assertFalse(result.exception, result.output)
frag_path = Path("foo", "frontend", "newsfragments")

fragments = [f.name for f in frag_path.iterdir()]
self.assertEqual(fragments, ["123.feature.rst"])

@with_isolated_runner
def test_sections_without_filename(self, runner: CliRunner):
setup_simple_project(
extra_config="""
[[tool.towncrier.section]]
name = "Backend"
path = ""

[[tool.towncrier.section]]
name = "Frontend"
path = "frontend"
"""
)
with mock.patch("click.edit") as mock_edit:
mock_edit.return_value = "Edited content"
result = runner.invoke(_main, input="2\n123\nfeature\n")
self.assertFalse(result.exception, result.output)
mock_edit.assert_called_once()
expected = os.path.join(
os.getcwd(), "foo", "frontend", "newsfragments", "123.feature.rst"
)

self.assertEqual(
result.output,
f"""\
Pick a section:
1: Backend
2: Frontend
Section (1, 2) [1]: 2
Issue number (`+` if none): 123
Fragment type (feature, bugfix, doc, removal, misc): feature
Created news fragment at {expected}
""",
)

@with_isolated_runner
def test_sections_without_filename_with_section_option(self, runner: CliRunner):
setup_simple_project(
extra_config="""
[[tool.towncrier.section]]
name = "Backend"
path = ""

[[tool.towncrier.section]]
name = "Frontend"
path = "frontend"
"""
)
with mock.patch("click.edit") as mock_edit:
mock_edit.return_value = "Edited content"
result = runner.invoke(
_main, ["--section", "Frontend"], input="123\nfeature\n"
)
self.assertFalse(result.exception, result.output)
mock_edit.assert_called_once()
expected = os.path.join(
os.getcwd(), "foo", "frontend", "newsfragments", "123.feature.rst"
)

self.assertEqual(
result.output,
f"""\
Issue number (`+` if none): 123
Fragment type (feature, bugfix, doc, removal, misc): feature
Created news fragment at {expected}
""",
)

@with_isolated_runner
def test_without_filename_with_message(self, runner: CliRunner):
"""
Expand Down
Loading