From e3e73bd037ee214678d7cf7da4e7def40ac69d82 Mon Sep 17 00:00:00 2001 From: Chris Beaven Date: Fri, 14 Jun 2024 10:57:18 +1200 Subject: [PATCH] Section aware create (#603) * Abstract the fragments path generation * Update the create option to work with sections * Add fragment * Update newsfraghment * No need to mention new behaviour in news fragment, since that case wouldn't have worked previously anyway * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add some test docstrings * Default section * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Typing improvement * Skip an invalid branch to cover * Add test for multiple sections all with paths --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- docs/cli.rst | 5 + src/towncrier/_builder.py | 49 +++++-- src/towncrier/build.py | 19 +-- src/towncrier/check.py | 19 +-- src/towncrier/create.py | 73 +++++++++-- src/towncrier/newsfragments/603.feature.rst | 1 + src/towncrier/test/test_create.py | 136 ++++++++++++++++++++ 7 files changed, 241 insertions(+), 61 deletions(-) create mode 100644 src/towncrier/newsfragments/603.feature.rst diff --git a/docs/cli.rst b/docs/cli.rst index 478a88b9..6adbb94d 100644 --- a/docs/cli.rst +++ b/docs/cli.rst @@ -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 section with no path, or if all sections have a path then the first defined section. + ``towncrier check`` ------------------- diff --git a/src/towncrier/_builder.py b/src/towncrier/_builder.py index 751d4b14..bfb05227 100644 --- a/src/towncrier/_builder.py +++ b/src/towncrier/_builder.py @@ -14,6 +14,8 @@ from jinja2 import Template +from towncrier._settings.load import Config + # Returns issue, category and counter or (None, None, None) if the basename # could not be parsed or doesn't contain a valid category. @@ -54,6 +56,35 @@ def parse_newfragment_basename( return invalid +class 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: # # { @@ -70,25 +101,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, ) -> 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) @@ -99,13 +126,13 @@ def find_fragments( for basename in files: issue, category, counter = parse_newfragment_basename( - basename, frag_type_names + basename, config.types ) if category is None: continue assert issue is not None assert counter is not None - if orphan_prefix and issue.startswith(orphan_prefix): + if config.orphan_prefix and issue.startswith(config.orphan_prefix): issue = "" # Use and increment the orphan news fragment counter. counter = orphan_fragment_counter[category] diff --git a/src/towncrier/build.py b/src/towncrier/build.py index e0add371..2d77bb09 100644 --- a/src/towncrier/build.py +++ b/src/towncrier/build.py @@ -178,24 +178,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( diff --git a/src/towncrier/check.py b/src/towncrier/check.py index ee9b612e..f0d45677 100644 --- a/src/towncrier/check.py +++ b/src/towncrier/check.py @@ -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 diff --git a/src/towncrier/create.py b/src/towncrier/create.py index 39fddec0..e78fb658 100644 --- a/src/towncrier/create.py +++ b/src/towncrier/create.py @@ -10,9 +10,11 @@ import os from pathlib import Path +from typing import cast import click +from ._builder import FragmentsPath from ._settings import config_option_help, load_config_from_options @@ -47,6 +49,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, @@ -55,6 +62,7 @@ def _main( filename: str, edit: bool | None, content: str, + section: str | None, ) -> None: """ Create a new news fragment. @@ -75,7 +83,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( @@ -85,6 +93,7 @@ def __main( filename: str, edit: bool | None, content: str, + section: str | None, ) -> None: """ The main entry point. @@ -97,7 +106,54 @@ def __main( if ext.lower() in (".rst", ".md"): filename_ext = ext + section_provided = section is not None + if not section_provided: + # Get the default section. + if len(config.sections) == 1: + section = next(iter(config.sections)) + else: + # If there are multiple sections then the first without a path is the default + # section, otherwise it's the first defined section. + for ( + section_name, + section_dir, + ) in config.sections.items(): # pragma: no branch + if not section_dir: + section = section_name + break + if section is None: + section = list(config.sections.keys())[0] + + if section not in config.sections: + # Raise a click exception with the correct parameter. + 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, + ) + section = cast(str, section) + if not filename: + if not section_provided: + sections = list(config.sections) + if len(sections) > 1: + click.echo("Pick a section:") + default_section_index = None + for i, s in enumerate(sections): + click.echo(f" {i+1}: {s or '(primary)'}") + if not default_section_index and s == 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: @@ -134,19 +190,8 @@ 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", - ) - ) + 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) diff --git a/src/towncrier/newsfragments/603.feature.rst b/src/towncrier/newsfragments/603.feature.rst new file mode 100644 index 00000000..afe48164 --- /dev/null +++ b/src/towncrier/newsfragments/603.feature.rst @@ -0,0 +1 @@ +The ``towncrier create`` action now uses sections defined in your config (either interactively, or via the new ``--section`` option). diff --git a/src/towncrier/test/test_create.py b/src/towncrier/test/test_create.py index 2ba74af9..dc6f6b9d 100644 --- a/src/towncrier/test/test_create.py +++ b/src/towncrier/test/test_create.py @@ -415,6 +415,142 @@ 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): + """ + When creating a new fragment, the user can specify the section from the command + line (and if none is provided, the default section will be used). + + The default section is either the section with a blank path, or else the first + section defined in the configuration file. + """ + setup_simple_project( + extra_config=""" +[[tool.towncrier.section]] +name = "Backend" +path = "backend" +[[tool.towncrier.section]] +name = "Frontend" +path = "" +""" + ) + result = runner.invoke(_main, ["123.feature.rst"]) + self.assertFalse(result.exception, result.output) + frag_path = Path("foo", "newsfragments") + fragments = [f.name for f in frag_path.iterdir()] + self.assertEqual(fragments, ["123.feature.rst"]) + + 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", "Backend"]) + self.assertFalse(result.exception, result.output) + frag_path = Path("foo", "backend", "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): + """ + When multiple sections exist when the interactive prompt is used, the user is + prompted to select a section. + """ + 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): + """ + When multiple sections exist and the section is provided via the command line, + the user isn't prompted to select a section. + """ + 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_sections_all_with_paths(self, runner: CliRunner): + """ + When all sections have paths, the first is the default. + """ + setup_simple_project( + extra_config=""" +[[tool.towncrier.section]] +name = "Frontend" +path = "frontend" + +[[tool.towncrier.section]] +name = "Backend" +path = "backend" +""" + ) + result = runner.invoke(_main, ["123.feature.rst"]) + 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_without_filename_with_message(self, runner: CliRunner): """