Repository: twisted/towncrier
Pull request #603 (2024-05-23 23:31:39)

Section aware create
# Description
`towncrier create` should work better with sections if defined in the configuration file.

This adds a new `towncrier create --section <section name>` option, or if you're using the new interactive create then you'll be asked for the section if there are multiple ones defined in your config:

```console
$ towncrier create --content "Sections!"
Pick a section
 1. Backend
 2. Frontend
Section (1, 2) [1]: 2
Issue number (`+` for none): 603
Fragment type (feature, bugfix, doc, removal, misc): feature
Created news fragment at /home/chris/demo/changes/frontend/603.feature.rst
```

If you use sections and none have an empty path, you must now specify the section when creating a new news fragment (if one _does_ have an empty path, that section will be used by default).

This PR also deduplicates the code that determines the base fragment path which was done in three places!

# Checklist
<!-- add a "X" inside the brackets to confirm -->
* [x] Make sure changes are covered by existing or new tests.
* [x] For at least one Python version, make sure local test run is green.
* [x] Create a file in `src/towncrier/newsfragments/`. Describe your
  change and include important information. Your change will be included in the public release notes.
* [x] Make sure all GitHub Actions checks are green (they are automatically checking all of the above).
* [x] Ensure `docs/tutorial.rst` is still up-to-date.
* [x] If you add new **CLI arguments** (or change the meaning of existing ones), make sure `docs/cli.rst` reflects those changes.
* [x] If you add new **configuration options** (or change the meaning of existing ones), make sure `docs/configuration.rst` reflects those changes.


adiroiban: Thanks for the PR.
I am not using the `create` command, so there is not much UI/UX feedback I can provide.

As long as you are happy with the behaviour, we can merge it.

----------

I am tring to understand what "sections" is in the context of towncrier.

I am reading something about it here https://towncrier.readthedocs.io/en/latest/configuration.html#sections

Especially the part from `Section Path Behaviour` is confusing.

I think that it needs better example.

For example

> If `directory` is not set, “newsfragments” is added to the end of each path. For example, with the above sections, the paths would be:

Which `directory` , I guess that this is `[tool.towncrier] -> directory`

Are people using this behaviour ?

To me, this all seems very confusing.

Patch:
diff --git a/docs/cli.rst b/docs/cli.rst
index 478a88b..6adbb94 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 751d4b1..bfb0522 100644
--- a/src/towncrier/_builder.py
+++ b/src/towncrier/_builder.py
@@ -14,6 +14,8 @@ from typing import Any, DefaultDict, Iterable, Iterator, Mapping, NamedTuple, Se
 
 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 e0add37..2d77bb0 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 ee9b612..f0d4567 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 39fddec..e78fb65 100644
--- a/src/towncrier/create.py
+++ b/src/towncrier/create.py
@@ -10,9 +10,11 @@ from __future__ import annotations
 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_CONTENT = "Add your info here"
     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 0000000..afe4816
--- /dev/null
+++ b/src/towncrier/newsfragments/603.feature.rst
@@ -0,0 +1,1 @@
+The ``towncrier create`` action now uses sections defined in your config (either interactively, or via the new ``--section`` option).


Test patch:
diff --git a/src/towncrier/test/test_create.py b/src/towncrier/test/test_create.py
index 2ba74af..dc6f6b9 100644
--- a/src/towncrier/test/test_create.py
+++ b/src/towncrier/test/test_create.py
@@ -415,6 +415,142 @@ Created news fragment at {expected}
         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):
         """