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

Add a post-generate extension point for bootstraps. #2119

Merged
merged 4 commits into from
Jan 19, 2025

Conversation

freakboy3742
Copy link
Member

@freakboy3742 freakboy3742 commented Jan 18, 2025

In the process of working on #1288, I discovered another extension point that is required on bootstraps - a "post generation" step that is invoked after the template has been generated. This is required because the Briefcase template has a fixed set of files; if you need to write an additional file (for example, to write an index.html for an embedded web server), there's not currently any way to do that with the bootstrap.

This requires a slight reorganisation to how the bootstrap is created, so that the bootstrap instance can be retained until after the template is generated. However, by doing this, it becomes helpful for there to always be a bootstrap... which also provides an opportunity to address #2006.

Makes 2 other small changes to the Console bootstrap:

  1. Adds some additional documentation to the code generated by the Console bootstrap to match that generated by new Empty bootstrap
  2. Renames the override variable for specifying a console app in the convert command. This hasn't been in an official release, so there's no backwards compatibility concerns; but specifying "-Q console_app=GUI` doesn't make any sense from a UX point of view.

Refs #1288
Fixes #1899
Fixes #2006

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@freakboy3742 freakboy3742 requested a review from mhsmith January 18, 2025 16:11
Copy link
Member

@rmartin16 rmartin16 left a comment

Choose a reason for hiding this comment

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

lgtm w/ some minor comments.

This also addresses #1899. And it may make #1898 even more difficult to address....although, as you said, a more general approach with PEP-621 will likely reshapre that whole conversation.

@@ -120,3 +121,12 @@ def pyproject_table_web(self) -> str | None:

def pyproject_extra_content(self) -> str | None:
"""Additional TOML to add to the bottom of pyproject.toml."""

def post_generate(self, base_path: Path) -> None:
"""Code to run after the template has been generated.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""Code to run after the template has been generated.
"""Runs after the template has been generated.

@@ -326,10 +327,10 @@ def input_url(self, app_name, override_value: str | None) -> str:

return url

def input_console_app(self, override_value: str | None) -> bool:
def input_app_type(self, override_value: str | None) -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

I was a bit caught off-guard when this didn't work:

briefcase convert -Q app_type=console
-- Interface Style -----------------------------------------------------------

Using override value 'console'

Invalid value; console
Invalid override value 'console' for Interface Style, using user-provided value.

Is this a GUI application or a console application?

  1) GUI
  2) Console

Interface Style [1]:

Maybe better to make these comparisons case-insensitive....

@freakboy3742
Copy link
Member Author

This also addresses #1899.

Good catch - I've added a changenote and a "Fixes" annotation in the PR body.

And it may make #1898 even more difficult to address....although, as you said, a more general approach with PEP-621 will likely reshapre that whole conversation.

Yeah - I think the real fix for #1898 is to stop using keys that are already covered by PEP 621, which is a much bigger project. I'm not sure this makes #1898 substantially worse though - the only project-wide keys it's added are ones that are cumulative; and the platform-specific keys are all either cumulative, or not covered by PEP 621.

@freakboy3742 freakboy3742 merged commit 731f213 into beeware:main Jan 19, 2025
49 checks passed
@freakboy3742 freakboy3742 deleted the post-generate branch January 19, 2025 01:31
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.

Provide a more extensive default bootstrap Include all Briefcase tables in pyproject.toml during conversion
2 participants