-
-
Notifications
You must be signed in to change notification settings - Fork 785
Ensure app name is normalised, and comes from an explicit source #3931
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
base: main
Are you sure you want to change the base?
Conversation
mhsmith
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep discussion about the substance of the change in one place in #3926.
core/src/toga/app.py
Outdated
| #. If the `app_id` argument was provided, its last segment will be used. For | ||
| example, an `app_id` of `com.example.my-app` would yield a distribution | ||
| name of `my-app`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't rendering as a list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be a bug/limitation of mkdocstrings when used with Sphinx-style docstrings. For some reason, the parser isn't reading any content that includes newlines.
I've logged this as mkdocstrings/mkdocstrings#808; in the meantime, I've reworked the text to not require bullet points.
Makes 2 small changes to the determination of app name:
This fixes #3926, which identified that running
python -m toga_demoresulted in the wrong app icon (because the app icon was evaluated asresources/toga_demo, whereas if you ran withbriefcase, you would get the icon (because Briefcase would embed the icon into the app binary)It also fixes the fact that
togawasn't listed as a dependency (because of the use of setuptools-dynamic-dependencies), and that some of the other definitions needed for successful Briefcase operation weren't listed.The app won't run on iOS, Android or Web, so those configurations are now disabled.
PR Checklist: