Add configurable host binding for external access#13
Conversation
selimacerbas
left a comment
There was a problem hiding this comment.
Hey @crankycoder, thanks for this — the host config itself is clean and well-tested, and we just shipped the live-server.nvim side in v1.3.0.
Before merging this PR, could you split it into two? The reason: it bundles two independent feature sets, and they should be reviewable / revertable separately.
PR A: host config (this PR, scoped down)
hostconfig fieldlock.is_server_alive(host, port)signature changeremote.send_event(host, port, ...)signature change- Browser URL uses configured host
- The test harness (
tests/host_config_test.lua)
PR B: browser opening improvements (new PR)
The changes in util.open_in_browser() are useful but unrelated to host config:
vim.notifyURL alwaysvim.ui.openfallbackvim.fn.executable()checkson_exiterror handling
These are 4 separate improvements and they touch the same function that PR #14 (just merged) modified for WSL support — so you'll need to rebase regardless.
I'll happily merge both PRs. Splitting just keeps the history clean and makes it easier to land each piece without holding the other hostage.
Let me know if you'd prefer me to do the split for you (cherry-picking your commits into two branches) — happy to take that on if it's easier.
|
sorry for the long delay. i think this should be ok now - i've rebased and thinned out the PR. |
|
hrm. couple minor issues i found that i'm working through rn |
- lock.is_server_alive now verifies the lock owner PID is still alive before trusting the port check. Prevents false secondary detection when a different process reuses the port after the original owner dies. - Add lock_test.lua with 10 tests covering PID liveness, write/read/remove roundtrip, corrupt files, port+PID semantics, and the stale-lock scenario. - Add GitHub Actions workflow that auto-discovers tests/*_test.lua. - Add CI badge and document host/port network options in README.
|
alright. fixed a stale lock bug and added a couple tests around that. |
|
Thanks @crankycoder, sorry for the delay on my side. The split looks great, the stale-lock fix is a good catch, and the tests are exactly what we needed (especially the port-reuse scenario in section 4 of Before merge1. README security warning. When a user sets
Could you add a > [!WARNING]
> Binding to `0.0.0.0` exposes the preview server (and the current
> markdown buffer) to anyone on the same network. The live-reload
> control endpoints are currently unauthenticated. Only use this on
> trusted networks. Prefer a one-shot SSH tunnel otherwise.2. Windows verification. The PID-aliveness check uses Follow-up I'll handle separatelyI'm going to add token auth in live-server.nvim, Jupyter style. Random 128-bit token at server start, embedded in Once the warning is in and Windows is confirmed, this is good to go. Thanks again for the patience and the tests. |
Uh oh!
There was an error while loading. Please reload this page.