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

Users can now write the content in the "create" method. #275

Merged
merged 12 commits into from
Dec 2, 2020
31 changes: 27 additions & 4 deletions src/towncrier/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@


@click.command(name="create")
@click.pass_context
@click.option("--dir", "directory", default=None)
@click.option("--config", "config", default=None)
@click.option("--edit/--no-edit", default=False) # TODO: default should be true
saroad2 marked this conversation as resolved.
Show resolved Hide resolved
@click.argument("filename")
def _main(directory, config, filename):
return __main(directory, config, filename)
def _main(ctx, directory, config, filename, edit):
return __main(ctx, directory, config, filename, edit)


def __main(directory, config, filename):
def __main(ctx, directory, config, filename, edit):
"""
The main entry point.
"""
Expand Down Expand Up @@ -59,11 +61,32 @@ def __main(directory, config, filename):
if os.path.exists(segment_file):
raise click.ClickException("{} already exists".format(segment_file))

content = _get_news_content(edit)

if content is None:
click.echo("Abort creating news fragment.")
ctx.exit(1)
Copy link
Member

Choose a reason for hiding this comment

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

Do you know the details of the various ways to exit a click program and which is best? (and why :]) I wasn't even aware that ctx.exit() existed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ctx.exit means exit the program with code.
You can always use the python return directive if you want, but it means that the exit code would be 0.

Here, I chose to exit with code 1 because if the user did not write content, it means the operation has "failed", and the exit code should reflect that.

As for the use of ctx.exit, click has this real bad habit of wrapping built-in python methods with click functions (for example, wrapping the print method in click.echo). I don't like it at all, but I take it as a conversion when working with click (this is related to our mocking discussion). In our case, we could simply use sys.exit instead and it would do the same.

To sum up, ctx.exit is the best choice, considering that it is a convention.

Copy link
Member

Choose a reason for hiding this comment

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

I agree a non-zero exit code is sensible, my only question was how to do it. I asked about this in the Click discord and the only answer I got was sys.exit(), but I think they hadn't heard of ctx.exit() either. :] Anyways, click.echo() does do more than print especially when dealing across Python versions and platforms. There's lots of room for major hassle when trying to handle that at the lower levels.


with open(segment_file, "w") as f:
f.writelines(["Add your info here"])
f.write(content)

click.echo("Created news fragment at {}".format(segment_file))


def _get_news_content(edit, default_content="Add your info here"):
if not edit:
return default_content
saroad2 marked this conversation as resolved.
Show resolved Hide resolved
content = click.edit(
"# Please write your news content. When finished, save the file.\n"
"# In order to abort, exit without saving.\n"
"# Lines starting with \"#\" are ignored.\n"
)
if content is None:
return None
all_lines = content.split("\n")
lines = [line.rstrip() for line in all_lines if not line.lstrip().startswith("#")]
return "\n".join(lines)


if __name__ == "__main__": # pragma: no cover
_main()
1 change: 1 addition & 0 deletions src/towncrier/newsfragments/275.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Users can now write the content in the ``create`` subcommand.
saroad2 marked this conversation as resolved.
Show resolved Hide resolved
43 changes: 40 additions & 3 deletions src/towncrier/test/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@

import os
from textwrap import dedent

from twisted.trial.unittest import TestCase
import mock
altendky marked this conversation as resolved.
Show resolved Hide resolved

from click.testing import CliRunner

Expand Down Expand Up @@ -33,18 +35,25 @@ def setup_simple_project(config=None, mkdir=True):
class TestCli(TestCase):
maxDiff = None

def _test_success(self, config=None, mkdir=True):
def _test_success(
self, content=None, config=None, mkdir=True, additional_args=None
):
runner = CliRunner()

with runner.isolated_filesystem():
setup_simple_project(config, mkdir)

result = runner.invoke(_main, ["123.feature.rst"])
args = ["123.feature.rst"]
if content is None:
content = ["Add your info here"]
if additional_args is not None:
args.extend(additional_args)
result = runner.invoke(_main, args)

self.assertEqual(["123.feature.rst"], os.listdir("foo/newsfragments"))

with open("foo/newsfragments/123.feature.rst") as fh:
self.assertEqual("Add your info here", fh.readlines()[0])
self.assertEqual(content, fh.readlines())

self.assertEqual(0, result.exit_code)

Expand All @@ -56,6 +65,34 @@ def test_directory_created(self):
"""Ensure both file and output directory created if necessary."""
self._test_success(mkdir=False)

def test_edit_without_comments(self):
"""Create file with dynamic content."""
content = ["This is line 1\n", "This is line 2"]
with mock.patch("click.edit") as mock_edit:
mock_edit.return_value = "".join(content)
self._test_success(content=content, additional_args=["--edit"])

def test_edit_with_comment(self):
"""Create file editly with ignored line."""
content = ["This is line 1\n", "This is line 2"]
comment = "# I am ignored\n"
with mock.patch("click.edit") as mock_edit:
mock_edit.return_value = "".join(content[:1] + [comment] + content[1:])
self._test_success(content=content, additional_args=["--edit"])

def test_edit_abort(self):
"""Create file editly and abort."""
with mock.patch("click.edit") as mock_edit:
mock_edit.return_value = None

runner = CliRunner()

with runner.isolated_filesystem():
setup_simple_project(config=None, mkdir=True)
result = runner.invoke(_main, ["123.feature.rst", "--edit"])
self.assertEqual([], os.listdir("foo/newsfragments"))
self.assertEqual(1, result.exit_code)

def test_different_directory(self):
"""Ensure non-standard directories are used."""
runner = CliRunner()
Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ deps =
Twisted
coverage
incremental
mock

commands =
python -V
Expand Down