Warn when an app description exceeds the recommended length#2869
Warn when an app description exceeds the recommended length#2869Labib-Bin-Salam wants to merge 1 commit into
Conversation
freakboy3742
left a comment
freakboy3742
left a comment
There was a problem hiding this comment.
Thanks for the contribution. A couple of suggestions inline, mostly related to keeping comments terse. There's no need to provide multiple copies of a long description in the code.
The CI failure on documentation will be resolved after merging with main - we landed a change today that made a change to how documentation is structured. Don't worry about the Pyside6 failures - those tests have a tendency to be unstable for a reason that we haven't quite determined yet.
| # the shortcut icon path in Windows MSI installers. | ||
| description = config.get("description") | ||
| if isinstance(description, str) and len(description) > MAX_DESCRIPTION_LENGTH: | ||
| console.warning( |
There was a problem hiding this comment.
When a warning is long, we can now use the console.warning_banner() helper. That helper automatically wraps the the content, as well as standardising the presentation of config issues.
| # The `description` is a short, single-line summary of the app. Longer values | ||
| # belong in `long_description`. Some packaging formats embed the description in | ||
| # length-limited fields (e.g. Windows MSI shortcuts truncate at 256 characters, | ||
| # corrupting the icon path), so Briefcase warns when it exceeds this length. | ||
| MAX_DESCRIPTION_LENGTH = 80 |
There was a problem hiding this comment.
This doesn't need to be factored out like this. It's used in exactly one place.
| # The description should be a short, single-line summary. Warn (but don't | ||
| # fail) if it's too long, as some packaging formats embed it in | ||
| # length-limited fields; for example, an over-long description corrupts | ||
| # the shortcut icon path in Windows MSI installers. |
There was a problem hiding this comment.
We don't need excessively long comments explaining code, especially when the warning message explains the underlying problem.
| # The description should be a short, single-line summary. Warn (but don't | |
| # fail) if it's too long, as some packaging formats embed it in | |
| # length-limited fields; for example, an over-long description corrupts | |
| # the shortcut icon path in Windows MSI installers. |
A `description` longer than 256 characters silently corrupts the shortcut icon path in Windows MSI installers (the path prefix is truncated by the overflow amount). Descriptions are meant to be short, single-line summaries; longer text belongs in `long_description`. Briefcase now emits a warning during config parsing when an app's `description` is longer than 80 characters, pointing the user at `long_description`. The value is still preserved as-is. Refs beeware#2864.
Labib-Bin-Salam
force-pushed
the
2864-warn-long-description
branch
from
c2850fc to
1d7a87e
Compare
2 participants
Changes
Partially addresses #2864.
That issue notes that a
descriptionlonger than 256 characters silently corrupts the shortcut icon path in Windows MSI installers, and lists three required fixes. This PR implements the Briefcase-side fix:Specifically:
MAX_DESCRIPTION_LENGTH(80) constant tobriefcase.config.parse_config()now emits aconsole.warning(...)when an app'sdescriptionexceeds that length, pointing the user atlong_description. The description value itself is preserved as-is, so this is purely advisory and non-breaking.The two remaining fixes from the issue — capping the description at 256 characters in the briefcase-windows-app-template and briefcase-windows-visualstudio-template — live in separate repositories and are out of scope for this PR.
PR Checklist:
Assisted-by: Claude Opus 4.8