-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
automatic reload when file changes externally #1125
Comments
See #588 |
Humbly, I prefer helix always alert me before it reloads an open file even if there are no changes to the opened file.
Long story short there are always good reasons for me to have helix hold on to a version of the file that's not the current version in the file system. |
Just some quick feedback: I'm currently trying Helix as an alternative to Kakoune and the lack of file watching really hurts. I frequently have several windows open with Helix where I work on the same files. Since there is no server-client architecture, the buffers get out-of-sync and make this workflow virtually unusable. I suppose the most viable workaround is to use splits in a single Helix instance, instead of letting my window manager handle the windows. |
👋 Hi @pascalkuthe! I see you self-assigned this issue earlier in the year, but I would love to help move this feature along if possible. I've taken a look at #2653, but it seems like that branch has stalled (and doesn't include reloading open buffers). I'd like to propose some functionality that could be broken up into multiple steps/PRs: File watching and reloading open buffers Watching individual files that are opened, but not in the CWD Support LSP didChangeWatchedFiles Optimizations for file watching If there is already work underway on this topic, then feel free to dismiss the above. But if not, I'd be interested in people's thoughts and happy to start with the work. Thanks! |
I agree that depending in an external binary like watchmen is not a good idea. However the notify crate is not just non-optimal its implementation is exactly what the kernel documentation tells you not to do. The API is also pretty limited and doesn't play that well with tokio. So to solve this I wanted to implement my own file watcher (that would work more like watchmen, si maimtainting an in memory tree of the the fs that could be quieried, we also have other uses for that). Haven't gotten around to that yet tough |
Ah okay, that's fair! I can create a separate issue for this, but until a file watcher is in place, would you be opposed to adding support for the LSP didChangeWatchedFiles and send events when relevant documents are written to, from within Helix? This would resolve a common pain-point I have of modifying metadata files (e.g. |
That could be a reasonable interim solution to simply send events into a channel in our save implementation and then handle those events in a watcher tokio task. That task can then handle the LSP related stuff (and will already be nontrivial) and keeps the whole thing decoupled from the actual watcher/out of the main code. That does seem like a reasonable first step to me. You can ignore registering "interest" for now I think (so passing the glob pattern to the actual watcher in the future) and simply filter the glob patterns as required. That's pretty complex and I will probably initially startout watching the entire workspace anyway (I would ignore gitignored file tough to avoid crawling build artifacts). That's what watchmen does too (when you subscribe to sepific diectoies/files it only sends you what you are interested in, but it always track everything internally). Btw for a really nice experience, I would also send a filewatcher event when we reload inside helix so that if the file changes externally you can still let the LSP know with
|
For what it's worth, I'm running into this a lot where I save my files, run Would it be okay to just get an initial draft of this functionality that uses |
This is one of the last few things I really feel I'm missing in Helix — would I be right in thinking the LSP side of this was sorted in #7665 ? Does that mean what's missing now is just buffer reloading when files change on the disk? Let me know if that's the case and if there is anything I can do to help out! |
One nice addition to the notification and option to auto-reload would also be to have the notification a bit more "in your face".. currently if you try to save a buffer that was externally modified you get a one-line notice in the footer that is extremely easy to miss and disappears on the next cursor move. I'm constantly running into this issue if I do a |
I find that I mostly run into this because I suspend helix, run some CLI operations, then resume helix. Reloading changes upon resuming from suspension would fix 90% of this for me. Would that be simpler than implementing a file watcher? |
Just one person here, but I should probably mention that wouldn't satisfy my use case. I almost always am working simultaneously in an editor and in a terminal. The terminal side of things is often making changes to files (via formatting tools, fixers, sops editing, etc), after which point, I have to manually restart Helix every time, or call |
As a temporary workaround, we could separately watch for some file changes in a given folder, and send some keystrokes to a byobu/tmux session. Of course, Using inotifywait: In this simplest example, that command is running into a
Using
It works, but of course, this feels very hacky, because it is. It could be nice that Links to help building such watcher script: |
another reason this is required: the colors to the left marking git status don't update if I do git commit in another terminal |
Checking in with @pascalkuthe who's down as an assignee — do you have a minimum set of requirements / functionality that you'd like to see for a first draft of this functionality? Are there any partial solutions that would be acceptable? From looking through things, it seems the obvious place is something that loops into inotify and plays nice with Tokio (would a Linux-only first pass at things be acceptable, or would you like to support all OS's from the get-go?). For that in-memory filesystem tree you mentioned, what sort of "queries" are you interested in there? Is this another thing that could be added later down the line? If you have the time, I'd be lovely to get a few indicators as to what you'd see as an acceptable contribution, then some of us with spare time can gradually work our way towards that vision! Working with several Helix instances open in Zellij means that I often run into this limitation and I'm more than happy to try to make some progress on it :) Thanks a ton! |
The file watcher needs to be a seperate library that should be working well before any PR to helix is amde. It should essentially be a rust version of watchman (as a library instead of as an external process). I got started with ut but it's pretty complex to implement. |
just run into this issue while building a lsp, and testing using Helix, kind of annoying to I am late to the party, so sorry if this is also covered earlier or elsewhere. but, is there a way to just add a this solves issue of and problems with a watch every file solution, and just doing a the tmux / external tools ideas are not user friendly, and doing watchman like tooling and forcing watching across all buffers will not be gerat for those that do not want or need to see an active buffer update. anyway just my 2c, after hitting |
I had very much the same issue, could simply not use Helix without auto-reloading, so I combined this, it does include a file watcher: |
@webdev23 will take a look later tonight, thanks for the link -> I went oldschool for time being and am now watching local stdout log using |
Please, don't give up trying to implement this! Auto-reloading the file, which was changed externally is a must-have feature for a modern (and even post-modern) editor. ;) |
Just one small correction: offering an (optional) prompt to reload would be modern. Sometimes your don't want it to autoreload at all costs or at least want to be able to press U and undo the external changes ;) Wouldn't be the first time helix saved a file that became a victim of an accidental delete or overwrite - both scenarios where auto reload without prompt would have been fatal. |
I would prefer a prompt if and only if the file was modified both inside and outside the editor. If I haven't modified the file in helix, and it changed on disk, this likely means a VCS checkout, and I don't want a prompt. |
Besides, you don't need a prompt to be able to undo across reloads. |
What about implementation like this: let's introduce an option
If the file is "dirty" (has unsaved changes) the prompt to save, reload or ignore should always be fired. |
@VKondakoff Sounds great! It could even be configured based on regex matches to the file name or somesuch. :) |
I have no experience here but, naively, https://github.com/watchexec/watchexec/tree/main/crates/lib might work. Update: Watchexec seems to be a wrapper around the |
Just so there’s a trace of the reasoning here, what are the issues that make you ignore the notify crate? When I started looking into wrapping inotify sys crate I saw this, and I’ve been wondering what the problem is ever since. The unreliability of |
See #8073 (comment) regarding that And more directly, https://matrix.to/#/!zMuVRxoqjyxyjSEBXc:matrix.org/$j0U306IWclmC5iO_kgCvn286QGruwM71iqFXem2-xBc?via=matrix.org&via=mozilla.org&via=tchncs.de
|
If you are using WezTerm, with Helix at the top and a terminal at the bottom, please check out my workaround. The idea is to emit an event when switching back from terminal (bottom) to the Helix editor (top): wezterm.on('reload-helix', function(window, pane)
local top_process = basename(pane:get_foreground_process_name())
if top_process == 'hx' then
local bottom_pane = pane:tab():get_pane_direction('Down')
if bottom_pane ~= nil then
local bottom_process = basename(bottom_pane:get_foreground_process_name())
wezterm.log_info('bottom process: ' .. bottom_process)
if bottom_process == 'lazygit' or bottom_process == 'fish' then
local action = wezterm.action.SendString(':reload-all\r')
window:perform_action(action, pane);
end
end
end
end) {
key = '[',
mods = 'CMD',
action = act.Multiple {
act.ActivatePaneDirection 'Up',
act.EmitEvent 'reload-helix',
}
}, |
Is this being worked on? Seems like there's implementation discussion going on but no linked PR. However, I just want to state that I agree with both sides (auto-reload and no auto-reload), hence I would prefer if there was a config option for this and I'm not forced into losing my unsaved changes just because something ran in the background. |
Would really benefit from this feature in my workflow. Often working in a codebase where I need to rebase and files may become stale (then I might accidentally overwrite something without realising). Kakoune prompts me to reload which I think is the best of both worlds. |
Modified @quantonganh's wezterm solution a bit to reload any time window focus is changed -- https://wezfurlong.org/wezterm/config/lua/pane/get_foreground_process_name.html
-- Equivalent to POSIX basename(3)
-- Given "/foo/bar" returns "bar"
-- Given "c:\\foo\\bar" returns "bar"
function basename(s)
return string.gsub(s, '(.*[/\\])(.*)', '%2')
end
wezterm.on('window-focus-changed', function(window, pane)
if window:is_focused() then
local top_process = basename(pane:get_foreground_process_name())
print(top_process)
if top_process == 'hx' then
local action = wezterm.action.SendString(':rla\r')
window:perform_action(action, pane);
end
end
end) |
Actually love current behavior of helix allowing me to keep stale versions of my file, as others mentioned, very useful for comparison in between branches. But also need this feature as an alternative to "tail -f" for watching log files. |
Oh, I just noticed a potential pitfall with this: |
VSCode seems to use https://github.com/parcel-bundler/watcher I think we can just implement something like this? I'm not sure what @pascalkuthe had in mind for a fleshed out watcher but I think if it works for VSCode than it'll likely be enough for us. A POC w/ bindings to Parcel's implementation might be worth exploring. |
I think a much simpler and specific solution could be implemented. Namely, call Push notifications via a file watcher could later be integrated as an optimization; either using the notify crate, whatever Pascal is working on (if it's ever finished), or something else. The stat-based solution might not be fit for wider purposes like having a full FS cache for the LSP, but it seems to me like #1125 deserves a short-term solution given the number of whacky solutions people are implementing already (sending signals to helix or even piping keystrokes to their terminal emulator 🤢). I'm happy to implement the stat-based approach for myself. I tried to touch base with Pascal on Matrix a couple of weeks ago to discuss a PR but got no response.
An external process like that is disagreeable for the same reasons as watchman - perhaps more so since it's Node bloatware vs a C++ binary. While I'm here, it's worth discussing Pascal's reasons for objecting to the notify crate:
This is an intrinsic problem with using a best-effort system API like inotify - the notify crate does at least expose
Just use From Pascal's message on matrix:
I agree what the notify crate is doing is inferior to integrating with a tokio reactor, but the thread starvation concerns would be pretty minor for #1125 (i.e. there only needs to be a single inotify fd for all open Documents; you can have multiple watches on the one queue/thread). Also, FWIW, a relatively small change to the notify crate would let it use tokio to wait on the inotify FD, rather than it spawning new threads. Another issue that Pascal pointed out from reddit:
Using a watchman-style serial clockid is only really relevant if you want to sync cached metadata, which I think is massively over-engineering the solution to #1125 compared to the |
What I proposed is to rewrite it in Rust if a POC using bindings to the C++ library (the JavaScript is only for the WASM wrapper) pans out. |
I have a proof-of-concept for the polling approach in this branch: master...p00ya:helix:file-changed I haven't implemented a menu/picker yet; it just indicates an external modification via
Ah, I see. That doesn't sound too bad. |
It would be great if it were detected when the file you are editing has been changed externally, and if possible, automatically reload the file.
I imagine it could work similarly to vim and kakoune, i.e. if there are no modifications that have yet to be written, just reload the file, and treat it as a single modification that can be undone. If there are modifications that haven't been written yet, prompt the user about what they would like to do:
The text was updated successfully, but these errors were encountered: