Windows follow-ups: app icon, working update checks, settings fixes, docs#358
Windows follow-ups: app icon, working update checks, settings fixes, docs#358davidbudnick wants to merge 9 commits into
Conversation
The Windows exes shipped with no resource section at all: Explorer, the taskbar, and Task Manager showed the generic blank-binary icon, and the VersionInfo pane was empty. Generate a .rc at build time (icon + VERSIONINFO tracking CARGO_PKG_VERSION) and compile it with embed-resource — pinned to the exact version already in Cargo.lock as gpui's own build-dependency, so the resolve adds an edge, not a crate, and the pinned gpui rev cannot move. The new design/icon/openlogi.ico (16-256px, PNG-compressed entries) is rendered from design/icon/openlogi.png. The MSI gets an Add/Remove Programs icon via ARPPRODUCTICON; the Start Menu shortcut needs nothing — it picks up the exe's embedded icon automatically. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cks on Windows Windows checks failed with "no release asset matched the current platform (windows/x86_64)" — for two stacked reasons (AprilNEA#347 PR 4): latest.json only ever listed the DMGs, and the client asked for format "exe", which no release ships. Now: - xtask release latest-json classifies the per-arch Windows .msi/.zip (minisign-required, like the DMGs) into the manifest alongside them; the Linux packages stay out on purpose — those update through the distro package manager. - The R2 publish step stops excluding the zip/msi + minisigs from the immutable releases/ prefix the manifest URLs point into. - The client requests format "msi" so a check can resolve on both x86_64 and arm64. Windows stays notify-only for now: gpui-updater's install strategy there is rename-in-place for a bare exe, and it would clobber OpenLogi.exe with the MSI's bytes if handed one. INSTALL_SUPPORTED gates auto-install off, hides the auto-install switch, and turns a found update into a "Download from GitHub" action until an msiexec flow lands upstream. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sibly work Three intertwined Settings problems, worst on Windows: - Custom rows (Updates hero, update source, About config path, the Appearance filter row) built h_flex().justify_between() with a non-shrinking left block, so long content — or just Segoe UI being wider than the macOS system font — shoved the trailing button past the window edge with no horizontal scroll to save it. Give the left blocks flex_1().min_w_0() with truncate() on the long line and pin the controls with flex_shrink_0(), mirroring what the library's own SettingItem rows already do; widen Settings to 920x640 and give aux windows a window_min_size floor like the main window's. - Settings → Assets "Open" was silently dead off macOS: open_in_file_manager was #[cfg(target_os = "macos")] with an empty stub elsewhere, even though the macOS arm already used the cross-platform opener crate. Drop the split. - "Clear" and "Refresh" worked but looked inert: the cache-size blurb is a construction-time snapshot, so the row kept quoting the pre-Clear number until the window reopened. Recompute it shortly after each manual action (the wipe/fetch runs on the main loop's channel arm, not synchronously in the click handler). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ndows out of preview The Windows section listed both artifacts without saying which to pick or why; the intro and roadmap still called the port an untested preview even though the README itself records end-to-end validation on Windows 11 (and the roadmap row still advertised a bare .exe that no release ships). Recommend the per-user MSI, frame the zip as the portable/no-install path, and align the status lines with the recorded validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…missions page Two parity gaps from a macOS-to-Windows feature audit: the notification-area icon honors show_in_menu_bar on Windows (tray_windows.rs) but the switch was compiled out as macOS-only, leaving config.toml surgery as the only way to hide the icon; and the Permissions sidebar entry rendered an empty body on Windows because both of its groups are cfg-gated to macOS/Linux - Windows needs no privacy grants for the WH_MOUSE_LL hook or raw HID access, so the page now only registers where it has content (SettingsPage::index tracks the shift). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR improves the Windows release and settings experience. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "style: rustfmt" | Re-trigger Greptile |
The unsupported-target cfg predates the Windows port (AprilNEA#167), so cargo test failed on every real Windows box: Hook::start succeeds there now. Windows joins the supported-platform test (which also exercises real WH_MOUSE_LL install/stop), and the Unsupported expectation is left for platforms that genuinely have no hook. CI never caught this because Windows runs clippy only (AprilNEA#347 PR 3). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The notification-area toggle, its description, and the notify-only "Download from GitHub" action were en-only literals; the locale key-consistency test requires every file to carry them, and Crowdin only surfaces keys that exist. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review feedback (Greptile P1): the release workflow R2 upload step still excludes the zip/msi from the releases/ prefix, and this PR cannot modify workflow files - so unconditional Windows entries would publish URLs the bucket does not serve on the next tag. Off by default, the manifest stays byte-identical dmg-only until the same release.yml change that drops the exclusions also passes --include-windows: the two halves can never disagree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
b2f8047 to
58555d2
Compare
|
Confirmed and addressed in 58555d2: the Windows entries are now gated off by default behind |
|
Intentional, and the two P1s on this file are each other's mirror: unconditional entries ⇒ the first P1 (manifest URLs the R2 step never uploads); gated entries ⇒ this one (checks stay macOS-only until enabled). Since workflow files can't be pushed from this fork's token, the gate is the only shape where the manifest and the bucket cannot disagree — and Windows clients meanwhile get the correct, honest behavior from this PR anyway: Enabling end-to-end Windows checks is the single two-hunk |
|
@AprilNEA A few more follow up's to the Windows PR from the other day. Should be able to move it from un-tested to fully supported. Needs an action change, and a PR merged in |
Summary
Windows follow-ups from daily-driving the v0.6.19 MSI on Windows 11: the missing app icon, the always-failing update check, clipped Settings rows, a dead button, and two parity gaps found in a full macOS→Windows feature audit. Five commits, reviewable independently:
1.
feat(windows): embed the app icon + version info in both exesThe shipped exes have no resource section at all — Explorer/taskbar/Task Manager show the generic blank-binary icon and the file-properties pane is empty. A build-time-generated
.rc(icon +VERSIONINFOtrackingCARGO_PKG_VERSION) is compiled withembed-resource, pinned to the exact version already inCargo.lockas gpui's own build-dep — the resolve adds an edge, not a crate, and the pinned gpui rev cannot move (verified: the lock diff is +2 dep-edge lines). Newdesign/icon/openlogi.ico(16–256 px, PNG-compressed entries, rendered fromdesign/icon/openlogi.png); the MSI getsARPPRODUCTICONvia a<?ifndef?>preprocessor default resolved relative to the.wxs($(sys.SOURCEFILEDIR)), so the release workflow needs no new define.2.
feat(updater): publish Windows assets inlatest.json; notify-only checks on WindowsThis is #347 "PR 4", scoped to what's safe today. On Windows every check fails with "no release asset matched the current platform (windows/x86_64)" because
latest.jsonis dmg-only and the client asks for format"exe", which no release ships.xtask release latest-jsonnow classifies the per-arch Windows.msi/.zip(minisign-required, like the DMGs) into the manifest;.deb/.rpmstay out on purpose (distro package managers own those updates)."msi", so checks resolve on both x86_64 and arm64.Why notify-only: gpui-updater v0.0.5's Windows install strategy is rename-in-place for a bare
.exeand only validates the target extension — handed the MSI the manifest serves, it would renameOpenLogi.exeaside and copy the MSI's bytes over it.INSTALL_SUPPORTEDgates auto-install off on Windows, hides the auto-install switch, and turns a found update into a "Download from GitHub" action.➡️ Companion PR for true one-click updates: AprilNEA/gpui-updater#6 adds the staged msiexec install flow (and fixes the payload-over-exe hazard above). The full macOS-style "Download & Install → Restart to Update" flow on Windows needs that PR merged and tagged first; then OpenLogi bumps the dep and flips
INSTALL_SUPPORTED— a two-line follow-up. Until then this PR delivers working checks + notify.release.yml. GitHub rejects pushes touching workflow files from my CLI token (noworkflowscope). To keep the manifest and the R2 bucket from ever disagreeing in the meantime, the Windows manifest entries are gated off by default behind a new--include-windowsflag onxtask release latest-json— merging this PR changes the published manifest by exactly nothing. Enabling Windows update checks is then one workflow edit that flips both halves together ("Allow edits by maintainers" is enabled; or apply on merge):- name: Generate static updater manifest ... cargo run -p xtask -- release latest-json \ --dist dist \ --tag "$GITHUB_REF_NAME" \ --base-url "$OPENLOGI_UPDATE_BASE_URL" \ + --include-windows \ --output dist/latest.json - name: Upload static updater assets to R2 ... - # The Windows binaries' distribution channel is the GitHub Release; - # latest.json is still dmg-only, so nothing in the updater bucket - # references the zip/msi or their minisigs — keep them all out of - # the immutable releases/ prefix until the Windows auto-updater - # lands (#347 PR 4). SHA256SUMS still lists them — it describes the - # GitHub Release's asset set. *.exe stays excluded defensively even - # though no bare exe ships anymore. + # latest.json now lists the Windows zip/msi alongside the DMGs + # (#347 PR 4), so they and their minisigs belong in the immutable + # releases/ prefix the manifest URLs point into. *.exe stays + # excluded defensively even though no bare exe ships anymore. aws s3 cp dist/ "$release_prefix/" \ --recursive \ --exclude latest.json \ --exclude "*.exe" \ --exclude "*.exe.minisig" \ - --exclude "*.zip" \ - --exclude "*.zip.minisig" \ - --exclude "*.msi" \ - --exclude "*.msi.minisig" \ --cache-control "public, max-age=31536000, immutable" \ --endpoint-url "$endpoint"The updater bucket then gains Windows entries on the next tagged release automatically.
3.
fix(settings): stop clipping row controls; make the Assets actions visibly workh_flex().justify_between()with a non-shrinking left block — long content, or just Segoe UI being wider than the macOS system font, shoved the trailing button past the window edge (no horizontal scroll exists). Left blocks nowflex_1().min_w_0()+truncate(), controlsflex_shrink_0()— mirroring what the library's ownSettingItemrows already do. Settings widens to 920×640 and aux windows get awindow_min_sizefloor like the main window's.open_in_file_managerwas#[cfg(target_os = "macos")]with an empty stub elsewhere — even though the macOS arm already used the cross-platformopenercrate. The split is gone.4.
docs(readme): recommend the MSI, explain the portable zip, promote Windows out of previewThe intro/roadmap still said "untested preview" (and advertised a bare
.exethat no release ships) even though the README itself records end-to-end validation on Windows 11. Aligned, plus explicit guidance: MSI for most users, zip for portable/no-install.5.
fix(settings): surface the tray toggle on Windows; drop the empty Permissions pageFrom the parity audit: the Windows notification-area icon honors
show_in_menu_bar(tray_windows.rs) but the switch was compiled out as macOS-only —config.tomlsurgery was the only way to hide the icon. And the Permissions sidebar entry rendered an empty body on Windows (both groups cfg-gated to macOS/Linux; Windows needs no privacy grants for theWH_MOUSE_LLhook or raw HID). The page now registers only where it has content.Validation checklist
Icon & version info (needs a local/CI Windows build):
OpenLogi.exeandopenlogi-agent.exeshow the OpenLogi icon in Explorer, the taskbar, and Task ManagerUpdater (client behavior; server side needs the next tagged release):
cargo test -p xtaskpasses (manifest generator: dmg + msi + zip classification, per-arch, minisign-required)Settings UI:
Assets page:
Parity:
Follow-ups (out of scope here)
INSTALL_SUPPORTEDopenlogi://, which Windows doesn't register)os_version()returnsNoneon Windows — diagnostics reports lack the OS version there🤖 Generated with Claude Code