Skip to content

Conversation

@johannst
Copy link
Contributor

@johannst johannst commented Sep 1, 2021

When running 'serve --open' / 'watch --open', start the browser in a
separate OS thread to start watching for changes immediately in case the
watch feature is enabled.

See issue (#1639).

When running 'serve --open' / 'watch --open', start the browser in a
separate OS thread to start watching for changes immediately in case the
watch feature is enabled.
@ehuss
Copy link
Contributor

ehuss commented Sep 8, 2021

Hm, so I haven't noticed this problem, and I'm wondering if it is specific to some configurations of xdg-open. I'm wondering if it would make sense to switch to the opener crate which just spawns the xdg-open in the background as opposed to open which for some reason waits for the process to finish.

@johannst
Copy link
Contributor Author

johannst commented Sep 8, 2021

I looked into it again and observed the following (linux, firefox):

  1. If the browser process is started due to --open then xdg-open will block waiting for the browser process to terminate (as you linked in the open crate). I mean that makes sense it waits for the child process to terminate.
  2. If a browser process is already running then xdg-open will return after opening a new tab in the running browser instance (the new firefox child process terminates after opening the tab).

So to support 1) we should either use a crate to just spawn the process (as you mentioned opener) or handle it by ourself.
But if we always want non-blocking behavior I don't see much value in doing ourself. In that case we could just use a crate that already does the thing we want :)

@johannst
Copy link
Contributor Author

@ehuss, please feel free to close this PR in case you agree that using a crate which allows non-blocking xdg-open is preferred over doing it ourselves here.

@ehuss
Copy link
Contributor

ehuss commented Sep 22, 2021

Sure, yea. I'd be happy to merge a PR that switched to opener.

@ehuss ehuss closed this Sep 22, 2021
Sign up for free to 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