add utility function to calculate underline size for category names

[jacobgulan]

Description

Fixes #626

Added a helper function to calculate the underline size for the jinja templates. Specifically addresses not enough underline characters being produced for category names that include wide characters like emojis.

Checklist

• Make sure changes are covered by existing or new tests.
• For at least one Python version, make sure test pass on your local environment.
• Create a file in src/towncrier/newsfragments/. Briefly describe your
  changes, with information useful to end users. Your change will be included in the public release notes.
• Make sure all GitHub Actions checks are green (they are automatically checking all of the above).
• Ensure docs/tutorial.rst is still up-to-date.
• If you add new CLI arguments (or change the meaning of existing ones), make sure docs/cli.rst reflects those changes.
• If you add new configuration options (or change the meaning of existing ones), make sure docs/configuration.rst reflects those changes.

[adiroiban]

Change looks good.

To merge this we need mypy fixed and at least one integration/end to end test.

Many thanks for the PR

[adiroiban]

Note that the underline is also computed here ... line 237

towncrier/src/towncrier/build.py

Lines 230 to 239 in afa6cc0

	if config.title_format:
	top_line = config.title_format.format(
	name=project_name, version=project_version, project_date=project_date
	)
	if is_markdown:
	parts = [f"# {top_line}"]
	else:
	parts = [top_line, config.underlines[0] * len(top_line)]
	parts.append(rendered)
	content = "\n".join(parts)

[jacobgulan]

Wanted to discuss the mypy change before touching it. It seems to be complaining that I'm trying to assign "underline_size" to a Mapping type. Mapping types are read-only. Therefore, mypy fails. I can update the type to a dict and mypy will likely be happy again, though I'm not sure that'd be the right approach.

[adiroiban]

Thanks for the update. It's a good start.

Please consider using render_fragments to create the data for the jinja template.

Inside render_fragments we can create a copy of definitions and update that copy and then pass it to jinja template rendering

so we would have something like definitions=updated_definitions in the call below

towncrier/src/towncrier/_builder.py

Lines 405 to 409 in 4aa8174

	res = jinja_template.render(
	render_title=render_title,
	sections=data,
	definitions=definitions,
	underlines=underlines,

this should clean the code and should also make mypy happy

it's best not to randomly update the configuration object or to update the configuration object after it was loaded from the config file

[jacobgulan]

Appreciate the feedback. I switched directions on the implementation after your last comment and seeing a better way that should prove to be more reliable and maintainable.

Instead of creating a copy of definitions and then calculating the underline size for that modified dictionary, I'm instead just passing the jinja template the get_underline_length() function itself now and calculating the length with it inside the .rst files.

Added some helper functions to recursively handle other types like dicts & lists which proved necessary for some of the other integration tests, but the behavior is working as expected now without modifying the original definitions mapping.

[adiroiban]

Thanks for the update.

The data exposed to the jinja template is part of our public API.
We need to double check that it make sense, as deprecating this type of code is very hard.

---

For a template design , I like the first way of exposing the underline lenght better.

As a general rule for template design, I try to avoid calling methods/functions from the template... but I can't tel if this is good or bad.
I feel that doing function calls in the templates, makes the template harder to read.

[jacobgulan]

I personally think that reading a function is easier to follow - if the name is intuitive - than indexing off variables. I would say it also makes sense to consolidate calculating length to one function rather than mixing get_underline_length() & the jinja pipe operation.

That said, I can also see the argument to maintain what's already present and just add another field to it. That second approach would look something like this:

  updated_definitions = {
      category: {**value, "underline_length": get_underline_length(value["name"])}
      for category, value in definitions.items()
  }

Couldn't use copy.deepcopy() as mypy screams about passing the wrong argument.

Let me know what you think

[adiroiban]

We can go with a separate function... and later we can look at making it available as a filter.

Right now, we use the jinja Template() API instead of the Environment() API and I don't know how to add an environment to an existing template... so that we can add a custom filter to the template.

I know that you can add custom filters to environments, but not to templates.

---

The thing is that jinja2 is a beast of its own... with inheritence, conditional statements, variable assignations ... and I have seen a lot jinja2 templates that are very hard to understand and update.

This is why I prefer to go with the pre-computed view-model approach, so that in the template, I only have loops and simple variables insertion.

[adiroiban]

One idea is a function like this

-{{ versiondata.name }} {{ versiondata.version }} ({{ versiondata.date }})
-{{ top_underline * (get_underline_length(versiondata.name + versiondata.version + versiondata.date) + 4)}}
+{{ top_section(versiondata.name, versiondata.version) }}

this function will join the arguments with a single space, and will generate 2 lines.

This looks to me less error prone

just an idea