-
-
Notifications
You must be signed in to change notification settings - Fork 118
chore: add powershell script to generate custom icons fonts #779
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
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.
Pull request overview
Adds local tooling to generate TrueType icon fonts from UFO packages using FontForge, with an option to publish the generated fonts into the app assets.
Changes:
- Adds
scripts/Generate-Fonts.ps1to discover.ufopackages and invoke FontForge for batch conversion, with optional publishing. - Adds
assets/fonts/fontforge-ufo-to-ttf.peas the FontForge batch conversion script.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/Generate-Fonts.ps1 | PowerShell wrapper to locate UFO packages, run FontForge conversion, and optionally move generated .ttf files into app assets. |
| assets/fonts/fontforge-ufo-to-ttf.pe | FontForge script to convert provided UFO paths into .ttf outputs. |
| if ($Publish) { | ||
| try { | ||
| $ttfFiles = @(Get-ChildItem -Path $fontsPath -Filter '*.ttf' -File -ErrorAction SilentlyContinue | Select-Object -ExpandProperty FullName) | ||
|
|
||
| if ($ttfFiles.Count -eq 0) { | ||
| Write-Error "No .ttf files found in the folder: $fontsPath" -Category ObjectNotFound | ||
| exit 1 | ||
| } | ||
|
|
Copilot
AI
Jan 28, 2026
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.
The script invokes FontForge but never checks whether the external process succeeded. As written, it will exit 0 even if fontforge fails (and with -Publish omitted, it also doesn’t verify that any .ttf outputs were produced). Capture and validate the FontForge exit code (e.g., $LASTEXITCODE) and/or check for expected output files before returning success.
| if ($Publish) { | |
| try { | |
| $ttfFiles = @(Get-ChildItem -Path $fontsPath -Filter '*.ttf' -File -ErrorAction SilentlyContinue | Select-Object -ExpandProperty FullName) | |
| if ($ttfFiles.Count -eq 0) { | |
| Write-Error "No .ttf files found in the folder: $fontsPath" -Category ObjectNotFound | |
| exit 1 | |
| } | |
| $fontForgeExitCode = $LASTEXITCODE | |
| if ($fontForgeExitCode -ne 0) { | |
| Write-Error "FontForge failed with exit code $fontForgeExitCode." -Category InvalidOperation | |
| exit $fontForgeExitCode | |
| } | |
| $ttfFiles = @(Get-ChildItem -Path $fontsPath -Filter '*.ttf' -File -ErrorAction SilentlyContinue | Select-Object -ExpandProperty FullName) | |
| if ($ttfFiles.Count -eq 0) { | |
| Write-Error "No .ttf files found in the folder: $fontsPath" -Category ObjectNotFound | |
| exit 1 | |
| } | |
| if ($Publish) { | |
| try { |
| @@ -0,0 +1,7 @@ | |||
| #!/usr/local/bin/fontforge | |||
Copilot
AI
Jan 28, 2026
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.
The shebang hardcodes /usr/local/bin/fontforge, which is not portable (and it’s unnecessary when this file is executed via fontforge -script). Consider removing the shebang entirely, or switching to an env-based shebang if you intend the script to be directly executable on Unix-like systems.
| #!/usr/local/bin/fontforge | |
| #!/usr/bin/env fontforge |
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 was taken straight from the FontForge scripting documentation, it does say "(or wherever fontforge happens to reside on your system)".
Either way, it's inconsequential for us. I think...
|
Since this is something the app consumes, it would make more sense to run it as a build step, probably a pre-build target. More details: MSBuild incremental builds for new or stale targets |
That was one of the goals, but the main issue is that it relies on having another application installed (unfortunately no |
-Publishflag is usedIMO, an Azure DevOps or GitHub Actions workflow would still be the best approach.