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

Fix pyproject.toml formatting error #18

Merged

Conversation

GenevieveBuckley
Copy link
Contributor

The cookicutter template is gnerating a pyproject.toml file with two missing \n characters. There should be a line break before [tool.briefcase.app.helloworld.windows] in the generated file.

We can fix this by moving the cookiecutter tags inside the square brackets of our list.
(The cookiecutter tags seem "greedy" and appear to suck up extra whitespace around them.)

Closes beeware/briefcase#405

Before the fix:

{%- if cookiecutter.gui_framework == 'Toga' %}
system_requires = [
    'libgirepository1.0-dev',
    'libcairo2-dev',
    'libpango1.0-dev',
    'libwebkitgtk-3.0-0',
    'gir1.2-webkit-3.0',
]
{% endif -%}

Generates this in pyproject.toml

...
[tool.briefcase.app.helloworld.macOS]
requires = []

[tool.briefcase.app.helloworld.linux]
requires = [][tool.briefcase.app.helloworld.windows]
requires = []

# Mobile deployments
...

After the fix:

The cookicutter tags have been moved inside the square brackets of the list.

system_requires = [
{%- if cookiecutter.gui_framework == 'Toga' %}
    'libgirepository1.0-dev',
    'libcairo2-dev',
    'libpango1.0-dev',
    'libwebkitgtk-3.0-0',
    'gir1.2-webkit-3.0',
{% endif -%}
]

Generates this correct pyproject.toml

[tool.briefcase.app.helloworld.macOS]
requires = []

[tool.briefcase.app.helloworld.linux]
requires = []
system_requires = []

[tool.briefcase.app.helloworld.windows]
requires = []

# Mobile deployments

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the contribution!

For the record - the "greedy" behavior you've observed is a feature of Jinja; {% if ... %} is an if tag, but the - on the tag [controls whether whitespace around the tag are consumed(https://jinja.palletsprojects.com/en/2.11.x/templates/?highlight=newlines#whitespace-control). In this case, it looks like the template was consuming a little too enthusiastically. The update you've provided tackles the problem in a slightly different, but probably better way - ensuring that there is always a system_requires directive, with only the contents being framework-specific.

If you're interested in a stretch goal related to this fix, I'd love to add a test suite to this repository so that bugs of this nature don't occur again in future. I've written up some requirements on #19; if you're interested, and/or if you have any questions, let me know!

Also - next time there's an in-person PyCon AU, don't forget to remind me that you're owed a Challenge Coin :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants