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

Interactive create #482

Merged
merged 22 commits into from
Apr 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
8647ee0
Interactive create when no filename is passed
SmileyChris Feb 21, 2023
d90def4
Configurable eof newline
SmileyChris Feb 21, 2023
c594476
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 21, 2023
fda486e
Add changes
SmileyChris Feb 21, 2023
96bb093
More tests for file extensions
SmileyChris Feb 21, 2023
13ac5b3
mypy fix
SmileyChris Feb 22, 2023
e97b3e2
Test interactive create for orphan news fragments
SmileyChris Feb 28, 2023
819638e
Interactive create when no filename is passed
SmileyChris May 7, 2023
49fc0b4
Configurable eof newline
SmileyChris May 7, 2023
b86c6a0
Add changes
SmileyChris May 7, 2023
24d7cb3
More tests for file extensions
SmileyChris May 7, 2023
c6b5f77
mypy fix
SmileyChris May 7, 2023
c742497
Test interactive create for orphan news fragments
SmileyChris May 7, 2023
1718ac1
Merge branch 'trunk' into feature/interactive-create
glyph Aug 21, 2023
f071ef8
Merge branch 'trunk' into feature/interactive-create
adiroiban Oct 23, 2023
fe10bfd
Merge branch 'trunk' into feature/interactive-create
SmileyChris Apr 24, 2024
e87fcb2
Document two other new features in the newsfragment
SmileyChris Apr 24, 2024
f02e8f6
Merge remote-tracking branch 'origin/feature/interactive-create' into…
SmileyChris Apr 24, 2024
9211524
Test improvements
SmileyChris Apr 24, 2024
f672dbc
Fix test to use default extension
SmileyChris Apr 24, 2024
9fe507e
remove obscure --eof-newline/--no-eof-newline cli option. Have a conf…
SmileyChris Apr 25, 2024
49145c7
Merge branch 'trunk' into feature/interactive-create
adiroiban Apr 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions docs/cli.rst
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ Create a news fragment in the directory that ``towncrier`` is configured to look

$ towncrier create 123.bugfix.rst

If you don't provide a file name, ``towncrier`` will prompt you for one.

``towncrier create`` will enforce that the passed type (e.g. ``bugfix``) is valid.

If the fragments directory does not exist, it will be created.
Expand All @@ -91,9 +93,10 @@ If that is the entire fragment name, a random hash will be added for you::
A string to use for content.
Default: an instructive placeholder.

.. option:: --edit
.. option:: --edit / --no-edit

Create file and start `$EDITOR` to edit it right away.
Whether to start ``$EDITOR`` to edit the news fragment right away.
Default: ``$EDITOR`` will be started unless you also provided content.


``towncrier check``
Expand Down
10 changes: 10 additions & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,16 @@ Top level keys

``"+"`` by default.

``create_eof_newline``
Ensure the content of a news fragment file created with ``towncrier create`` ends with an empty line.

``true`` by default.

``create_add_extension``
Add the ``filename`` option extension to news fragment files created with ``towncrier create`` if an extension is not explicitly provided.

``true`` by default.

Extra top level keys for Python projects
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
2 changes: 2 additions & 0 deletions src/towncrier/_settings/load.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ class Config:
wrap: bool = False
all_bullets: bool = True
orphan_prefix: str = "+"
create_eof_newline: bool = True
create_add_extension: bool = True
Copy link
Member

Choose a reason for hiding this comment

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

Should these 2 new config options have the default set to False, for backward compatibility ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends if backwards compatibility is more important than (imo) more sensible defaults. I'll leave that decision to the maintainers to make a call on.

Copy link
Member

Choose a reason for hiding this comment

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

I am not using the create command... so I don't know what to say.

In general, the rule is to keep backward compatibiliy. Any exception should be explicitly raised and accepted on a case by case basis.

We don't have a separate backward compatiblity policy for towncrier ... so I go with the general Twisted org policy https://docs.twisted.org/en/stable/development/compatibility-policy.html

I am happy to have a separate policy for towncrier.
Since towncrie is a dev tool, I think we can be more releaxed.

At the same time, I don't like when a CI pipeline breaks after an update

I guess that towncrier create is not used as part of any automation... so we should be ok.

If nobody else is -1 on this default, I will merge is at it is.

I am happy with the new defaults.

Thanks!



class ConfigError(ClickException):
Expand Down
79 changes: 55 additions & 24 deletions src/towncrier/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
from ._settings import config_option_help, load_config_from_options


DEFAULT_CONTENT = "Add your info here"


@click.command(name="create")
@click.pass_context
@click.option(
Expand All @@ -32,30 +35,32 @@
)
@click.option(
"--edit/--no-edit",
default=False,
default=None,
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
help="Open an editor for writing the newsfragment content.",
) # TODO: default should be true
)
@click.option(
"-c",
"--content",
type=str,
default="Add your info here",
default=DEFAULT_CONTENT,
help="Sets the content of the new fragment.",
)
@click.argument("filename")
@click.argument("filename", default="")
def _main(
ctx: click.Context,
directory: str | None,
config: str | None,
filename: str,
edit: bool,
edit: bool | None,
Copy link
Member

Choose a reason for hiding this comment

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

Why not leave edit as always being boolean ?

It looks like we are using None here as a placeholder for the default value.

But maybe it's better to have the explicit default value defined in this way.

Suggested change
edit: bool | None,
edit: bool = True,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only set to True only if the content is still the default content. If a user specifies their own content then, we don't want to default to triggering editing mode. But by leaving it None, it makes it possible to provide custom content and still explicitly choose to enable editing mode with this flag.

Copy link
Member

Choose a reason for hiding this comment

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

We can have it like this... it looks like we are hijacking this variable :)

We should have a separate flag to signal the other use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the same behaviour as any of the other cli settings that also have a configuration setting. If None, fall back to the configuration, otherwise use the value as provided.

content: str,
) -> None:
"""
Create a new news fragment.

Create a new news fragment called FILENAME or pass the full path for a file.
Towncrier has a few standard types of news fragments, signified by the file extension.
If FILENAME is not provided, you'll be prompted to create it.

Towncrier has a few standard types of news fragments, signified by the file
extension.

\b
These are:
Expand All @@ -76,14 +81,34 @@ def __main(
directory: str | None,
config_path: str | None,
filename: str,
edit: bool,
edit: bool | None,
content: str,
) -> None:
"""
The main entry point.
"""
base_directory, config = load_config_from_options(directory, config_path)

filename_ext = ""
if config.create_add_extension:
ext = os.path.splitext(config.filename)[1]
if ext.lower() in (".rst", ".md"):
filename_ext = ext

if not filename:
prompt = "Issue number"
# Add info about adding orphan if config is set.
if config.orphan_prefix:
prompt += f" (`{config.orphan_prefix}` if none)"
issue = click.prompt(prompt)
fragment_type = click.prompt(
"Fragment type",
type=click.Choice(list(config.types)),
)
filename = f"{issue}.{fragment_type}"
if edit is None and content == DEFAULT_CONTENT:
edit = True

file_dir, file_basename = os.path.split(filename)
if config.orphan_prefix and file_basename.startswith(f"{config.orphan_prefix}."):
# Append a random hex string to the orphan news fragment base name.
Expand All @@ -94,15 +119,18 @@ def __main(
f"{file_basename[len(config.orphan_prefix):]}"
),
)
if len(filename.split(".")) < 2 or (
filename.split(".")[-1] not in config.types
and filename.split(".")[-2] not in config.types
filename_parts = filename.split(".")
if len(filename_parts) < 2 or (
filename_parts[-1] not in config.types
and filename_parts[-2] not in config.types
):
raise click.BadParameter(
"Expected filename '{}' to be of format '{{name}}.{{type}}', "
"where '{{name}}' is an arbitrary slug and '{{type}}' is "
"one of: {}".format(filename, ", ".join(config.types))
)
if filename_parts[-1] in config.types and filename_ext:
filename += filename_ext

if config.directory:
fragments_directory = os.path.abspath(
Expand Down Expand Up @@ -135,31 +163,34 @@ def __main(
)

if edit:
edited_content = _get_news_content_from_user(content)
if edited_content is None:
click.echo("Abort creating news fragment.")
if content == DEFAULT_CONTENT:
content = ""
content = _get_news_content_from_user(content)
if not content:
click.echo("Aborted creating news fragment due to empty message.")
ctx.exit(1)
content = edited_content

with open(segment_file, "w") as f:
f.write(content)
if config.create_eof_newline and content and not content.endswith("\n"):
f.write("\n")

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


def _get_news_content_from_user(message: str) -> str | None:
initial_content = (
"# 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'
)
initial_content += f"\n{message}\n"
def _get_news_content_from_user(message: str) -> str:
initial_content = """
# Please write your news content. Lines starting with '#' will be ignored, and
# an empty message aborts.
"""
if message:
initial_content = f"{message}\n{initial_content}"
content = click.edit(initial_content)
if content is None:
return None
return message
all_lines = content.split("\n")
lines = [line.rstrip() for line in all_lines if not line.lstrip().startswith("#")]
return "\n".join(lines)
return "\n".join(lines).strip()


if __name__ == "__main__": # pragma: no cover
Expand Down
5 changes: 5 additions & 0 deletions src/towncrier/newsfragments/482.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
If no filename is given when doing ``towncrier`` create, interactively ask for the issue number and fragment type (and then launch an interactive editor for the fragment content).
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved

Now by default, when creating a fragment it will be appended with the ``filename`` option's extension (unless an extension is explicitly provided). For example, ``towncrier create 123.feature`` will create ``news/123.feature.rst``. This can be changed in configuration file by setting `add_extension = false`.
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this is feature, or a backward compatiblity break :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a change in the filename but since the file is parsed by towncrier in exactly the same way, I don't think it's really backwards incompatible

Copy link
Member

Choose a reason for hiding this comment

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

OK. Thanks. Fine by me.

I am not sure that adding release notes in this way, in a single fragment, does what we want.

I remember that towncrier will create one list item per file

On my project, as a hack I have 482-1.feature.rst as a way to add another item in the notes...but I think that we need better support here

I remember there were other disussions about this


A new line is now added by default to the end of the fragment contents. This can be reverted in the configuration file by setting `add_newline = false`.
Loading
Loading