Skip to content

feat: Add context-aware functions - #31

Open
jkroepke wants to merge 1 commit into
cli:mainfrom
jkroepke:ctx
Open

feat: Add context-aware functions#31
jkroepke wants to merge 1 commit into
cli:mainfrom
jkroepke:ctx

Conversation

@jkroepke

Copy link
Copy Markdown

In rare cases, where xdg-open hangs, kill it if context is canceled.

CopilotAI review requested due to automatic review settings June 21, 2026 10:33
@jkroepke
jkroepke requested a review from a team as a code ownerJune 21, 2026 10:33
@jkroepke
jkroepke requested a review from BagToadJune 21, 2026 10:34

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds context-aware variants of the public browser-opening helpers so that callers can cancel the underlying OS command (notably to stop hung xdg-open processes).

Changes:

  • Introduce OpenFileContext, OpenReaderContext, and OpenURLContext, and re-route existing APIs through context.Background().
  • Plumb context.Context through openBrowser and runCmd via exec.CommandContext.
  • Update examples to use the new *Context APIs.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
example_test.goUpdates examples to call the new context-aware APIs.
browser.goAdds exported *Context functions and updates runCmd to use exec.CommandContext.
browser_linux.goThreads context through Linux browser provider execution.
browser_freebsd.goThreads context into xdg-open invocation.
browser_openbsd.goThreads context into xdg-open invocation.
browser_netbsd.goThreads context into xdg-open invocation.
browser_darwin.goThreads context into open invocation on macOS.
browser_windows.goUpdates openBrowser signature to accept context (unused on Windows).
browser_unsupported.goUpdates openBrowser signature to accept context and adds //go:build line.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadbrowser.go
Comment threadbrowser_darwin.go Outdated
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

browser.go:49

  • OpenReaderContext currently only uses ctx when launching the browser (OpenFileContext), but it will still create a temp file and fully io.Copy() the reader even if ctx is already canceled. This makes the new context-aware API not actually cancellable for potentially expensive reads, and it can also leave behind temp files on cancellation/open errors. Consider failing fast on ctx.Err() and cleaning up the temp file on any error (including ctx cancellation and OpenFileContext failure).
func OpenReaderContext(ctx context.Context, r io.Reader) error {
f, err := os.CreateTemp("", "browser.*.html")
if err != nil {
return fmt.Errorf("browser: could not create temporary file: %w", err)
}

@williammartin
williammartin self-requested a review July 21, 2026 14:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jkroepke