Fix cross-platform URL opening on Windows and Linux - #36
Merged
Conversation
Open story URLs with execFile on darwin, win32, and linux so Windows uses cmd /c start and URLs are passed as arguments, not interpolated into a shell string. Also point the OpenTUI credit at anomalyco/opentui. Co-authored-by: Brian Lovin <brianlovin@users.noreply.github.com>
scrollToStory ran after await getPostById, so a slow or hung HNPWA request left selectedIndex updated and scrollTop at 0. Scroll immediately from the last laid-out frame, ignore zero-size layout metrics, and lock the regression with a hanging-fetch test. Co-authored-by: Brian Lovin <brianlovin@users.noreply.github.com>
Attach preconnect so the hanging/404 fetch stubs satisfy typeof fetch. Co-authored-by: Brian Lovin <brianlovin@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #35 (thanks @jensrot for the original idea and the README OpenTUI credit fix). That PR correctly added a platform-aware open helper and pointed the OpenTUI credit at
anomalyco/opentui, but its code path had blockers this change avoids.Why
onOpenUrlhardcoded macOSopen, so opening articles failed on Windows and Linux even though optionalDependencies already ship binaries for those platforms.What changed
openUrlhelper insrc/index.tsusingexecFile(no shell interpolation of the URL):open <url>cmd /c start "" <url>(startis a cmd builtin, not an executable)xdg-open <url>Why not merge #35 as-is
import { open } from "fs"exec('start "" "…")fails on Windows becausestartis not an executableexec(\… "${url}"`)` breaks on quotes/spacesNo new dependency. Helper stays in
src/index.tsto match the existing clipboard platform split insrc/app.ts.Follow-up: flaky story-list scroll test
CI on this PR (and the last two docs-only pushes to
main) failedshould scroll down to show off-screen selected story. Unrelated to URL opening.Root cause:
selectStorycalledscrollToStoryonly afterawait getPostById().selectedIndexupdates synchronously, but list scroll waited on the HNPWA network.renderer.idle()does not wait forfetch. When the request is slow or hung,scrollTopstays 0 — matching CI (currentSelectedIndex === 14,scrollTop === 0). The sibling “scroll up” test does more navigation and often wins the race.Fix: scroll immediately from the last laid-out frame, before the detail fetch. Ignore zero-size layout metrics so a stale 0×0 frame cannot reset
scrollTop. The down-scroll test now hangsfetchso this cannot regress on network timing.Do not merge.