Add --keep option to allow to generate newsfile, but keep newsfragmen… (#453)

* Add --keep option to allow to generate newsfile, but keep newsfragments - closes #129

* Apply suggestions from code review

Co-authored-by: Adi Roiban <adiroiban@gmail.com>

* Additional post-PR enhancements

* Update build --keep comment to be more relevant

Co-authored-by: Adi Roiban <adiroiban@gmail.com>

* More compact remove_files and use with_isolated_runner for new tests

* Refactored _git.remove_files

    Code making decision to remove prompt or keep news fragments
    is being kept in new intermediate function _remover.remove_news_fragment_files

* Update src/towncrier/newsfragments/129.feature

Co-authored-by: Hynek Schlawack <hs@ox.cx>

* Make a decision and return it instead of calling _git.remove underneath

* Move should_remove_fragment_files into build module

Co-authored-by: Adi Roiban <adiroiban@gmail.com>
Co-authored-by: Hynek Schlawack <hs@ox.cx>

Author: 3 people

docs/cli.rst

@@ -50,6 +50,10 @@ Build the combined news file from news fragments.
    Do not ask for confirmations.
    Useful for automated tasks.
 
+.. option:: --keep
+
+   Don't delete news fragments after the build and don't ask for confirmation whether to delete or keep the fragments.
+
 
 ``towncrier create``
 --------------------

src/towncrier/_git.py

@@ -7,22 +7,9 @@
 
 from subprocess import STDOUT, call, check_output
 
-import click
 
-
-def remove_files(fragment_filenames: list[str], answer_yes: bool) -> None:
-    if not fragment_filenames:
-        return
-
-    if answer_yes:
-        click.echo("Removing the following files:")
-    else:
-        click.echo("I want to remove the following files:")
-
-    for filename in fragment_filenames:
-        click.echo(filename)
-
-    if answer_yes or click.confirm("Is it okay if I remove those files?", default=True):
+def remove_files(fragment_filenames: list[str]) -> None:
+    if fragment_filenames:
         call(["git", "rm", "--quiet"] + fragment_filenames)
 
 

src/towncrier/build.py

@@ -15,8 +15,11 @@
 
 import click
 
+from click import Context, Option
+
+from towncrier import _git
+
 from ._builder import find_fragments, render_fragments, split_fragments
-from ._git import remove_files, stage_newsfile
 from ._project import get_project_name, get_version
 from ._settings import ConfigError, config_option_help, load_config_from_options
 from ._writer import append_to_newsfile
@@ -26,6 +29,18 @@ def _get_date() -> str:
     return date.today().isoformat()
 
 
+def _validate_answer(ctx: Context, param: Option, value: bool) -> bool:
+    value_check = (
+        ctx.params.get("answer_yes")
+        if param.name == "answer_keep"
+        else ctx.params.get("answer_keep")
+    )
+    if value_check and value:
+        click.echo("You can not choose both --yes and --keep at the same time")
+        ctx.abort()
+    return value
+
+
 @click.command(name="build")
 @click.option(
     "--draft",
@@ -67,9 +82,18 @@ def _get_date() -> str:
 @click.option(
     "--yes",
     "answer_yes",
-    default=False,
+    default=None,
     flag_value=True,
     help="Do not ask for confirmation to remove news fragments.",
+    callback=_validate_answer,
+)
+@click.option(
+    "--keep",
+    "answer_keep",
+    default=None,
+    flag_value=True,
+    help="Do not ask for confirmations. But keep news fragments.",
+    callback=_validate_answer,
 )
 def _main(
     draft: bool,
@@ -79,6 +103,7 @@ def _main(
     project_version: str | None,
     project_date: str | None,
     answer_yes: bool,
+    answer_keep: bool,
 ) -> None:
     """
     Build a combined news file from news fragment.
@@ -92,6 +117,7 @@ def _main(
             project_version,
             project_date,
             answer_yes,
+            answer_keep,
         )
     except ConfigError as e:
         print(e, file=sys.stderr)
@@ -106,6 +132,7 @@ def __main(
     project_version: str | None,
     project_date: str | None,
     answer_yes: bool,
+    answer_keep: bool,
 ) -> None:
     """
     The main entry point.
@@ -234,13 +261,43 @@ def __main(
         )
 
         click.echo("Staging newsfile...", err=to_err)
-        stage_newsfile(base_directory, news_file)
+        _git.stage_newsfile(base_directory, news_file)
 
         click.echo("Removing news fragments...", err=to_err)
-        remove_files(fragment_filenames, answer_yes)
+        if should_remove_fragment_files(
+            fragment_filenames,
+            answer_yes,
+            answer_keep,
+        ):
+            _git.remove_files(fragment_filenames)
 
         click.echo("Done!", err=to_err)
 
 
+def should_remove_fragment_files(
+    fragment_filenames: list[str],
+    answer_yes: bool,
+    answer_keep: bool,
+) -> bool:
+    try:
+        if answer_keep:
+            click.echo("Keeping the following files:")
+            # Not proceeding with the removal of the files.
+            return False
+
+        if answer_yes:
+            click.echo("Removing the following files:")
+        else:
+            click.echo("I want to remove the following files:")
+    finally:
+        # Will always be printed, even for answer_keep to help with possible troubleshooting
+        for filename in fragment_filenames:
+            click.echo(filename)
+
+    if answer_yes or click.confirm("Is it okay if I remove those files?", default=True):
+        return True
+    return False
+
+
 if __name__ == "__main__":  # pragma: no cover
     _main()

src/towncrier/newsfragments/129.feature

@@ -0,0 +1,2 @@
+Added ``--keep`` option to the ``build`` command that allows to generate a newsfile, but keeps the newsfragments in place.
+This option can not be used together with ``--yes``.

src/towncrier/test/test_build.py

@@ -407,6 +407,66 @@ def test_no_confirmation(self):
             self.assertFalse(os.path.isfile(fragment_path1))
             self.assertFalse(os.path.isfile(fragment_path2))
 
+    @with_isolated_runner
+    def test_keep_fragments(self, runner):
+        """
+        The `--keep` option will build the full final news file
+        without deleting the fragment files and without
+        any extra CLI interaction or confirmation.
+        """
+        setup_simple_project()
+        fragment_path1 = "foo/newsfragments/123.feature"
+        fragment_path2 = "foo/newsfragments/124.feature.rst"
+        with open(fragment_path1, "w") as f:
+            f.write("Adds levitation")
+        with open(fragment_path2, "w") as f:
+            f.write("Extends levitation")
+
+        call(["git", "init"])
+        call(["git", "config", "user.name", "user"])
+        call(["git", "config", "user.email", "user@example.com"])
+        call(["git", "add", "."])
+        call(["git", "commit", "-m", "Initial Commit"])
+
+        result = runner.invoke(_main, ["--date", "01-01-2001", "--keep"])
+
+        self.assertEqual(0, result.exit_code)
+        # The NEWS file is created.
+        # So this is not just `--draft`.
+        self.assertTrue(os.path.isfile("NEWS.rst"))
+        self.assertTrue(os.path.isfile(fragment_path1))
+        self.assertTrue(os.path.isfile(fragment_path2))
+
+    @with_isolated_runner
+    def test_yes_keep_error(self, runner):
+        """
+        It will fail to perform any action when the
+        conflicting --keep and --yes options are provided.
+
+        Called twice with the different order of --keep and --yes options
+        to make sure both orders are validated since click triggers the validator
+        in the order it parses the command line.
+        """
+        setup_simple_project()
+        fragment_path1 = "foo/newsfragments/123.feature"
+        fragment_path2 = "foo/newsfragments/124.feature.rst"
+        with open(fragment_path1, "w") as f:
+            f.write("Adds levitation")
+        with open(fragment_path2, "w") as f:
+            f.write("Extends levitation")
+
+        call(["git", "init"])
+        call(["git", "config", "user.name", "user"])
+        call(["git", "config", "user.email", "user@example.com"])
+        call(["git", "add", "."])
+        call(["git", "commit", "-m", "Initial Commit"])
+
+        result = runner.invoke(_main, ["--date", "01-01-2001", "--yes", "--keep"])
+        self.assertEqual(1, result.exit_code)
+
+        result = runner.invoke(_main, ["--date", "01-01-2001", "--keep", "--yes"])
+        self.assertEqual(1, result.exit_code)
+
     def test_confirmation_says_no(self):
         """
         If the user says "no" to removing the newsfragements, we end up with
@@ -429,7 +489,7 @@ def test_confirmation_says_no(self):
             call(["git", "add", "."])
             call(["git", "commit", "-m", "Initial Commit"])
 
-            with patch("towncrier._git.click.confirm") as m:
+            with patch("towncrier.build.click.confirm") as m:
                 m.return_value = False
                 result = runner.invoke(_main, [])