Open dead10ck opened 2 years ago
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 https://github.com/helix-editor/helix/pull/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
This first step (and likely also the largest) would involve adding "file watching" functionality to the editor. I've looked into both the notify
crate, in addition to using an external tool like watchman
. I think that using notify
is the better approach that doesn't make Helix reliant on external tooling. I think a reasonable first implementation would watch the files in the current working directory, and take a list of excludes (probably globs?) to cut down on noise. VSCode uses some default excludes ("files.watcherExclude"
) that seem sensible. Once file watching is in place, open buffers that haven't been modified can be reloaded on change.
Watching individual files that are opened, but not in the CWD
For files that are opened but not watched because they are outside of the CWD, they can be separately watched via individual requests to the Watcher
.
Support LSP didChangeWatchedFiles
Add support for dynamic_registration
with LSPs, and forward requested file changes as necessary. I think this should be quite straightforward once file watching is already in place. Changed files can be compared against the patterns that LSPs request, and sent if there are any matches.
Optimizations for file watching Specifically for inotify on linux, the exclude globs should be taken into account when traversing the file system to cut down on the number of paths that need to be watched. However, this is likely only a problem on a small number of very large repositories, or on less powerful hardware.
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!
👋 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 This first step (and likely also the largest) would involve adding "file watching" functionality to the editor. I've looked into both the
notify
crate, in addition to using an external tool likewatchman
. I think that usingnotify
is the better approach that doesn't make Helix reliant on external tooling. I think a reasonable first implementation would watch the files in the current working directory, and take a list of excludes (probably globs?) to cut down on noise. VSCode uses some default excludes ("files.watcherExclude"
) that seem sensible. Once file watching is in place, open buffers that haven't been modified can be reloaded on change.Watching individual files that are opened, but not in the CWD For files that are opened but not watched because they are outside of the CWD, they can be separately watched via individual requests to the
Watcher
.Support LSP didChangeWatchedFiles Add support for
dynamic_registration
with LSPs, and forward requested file changes as necessary. I think this should be quite straightforward once file watching is already in place. Changed files can be compared against the patterns that LSPs request, and sent if there are any matches.Optimizations for file watching Specifically for inotify on linux, the exclude globs should be taken into account when traversing the file system to cut down on the number of paths that need to be watched. However, this is likely only a problem on a small number of very large repositories, or on less powerful hardware.
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. Cargo.toml
) in Helix and having to restart the LSP to pull in the changes. That work could then be replaced when full file watching functionality is added sometime in the future.
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 :reload
. I think this also nicely illustrates that we really have two problems:
For what it's worth, I'm running into this a lot where I save my files, run cargo clippy --fix --allow-staged
, and then I have to remember to :rla
before I continue to work on them. I definitely agree with @pascalkuthe that "reload open files" and "tell LSP when a file has changed" are separate issues.
Would it be okay to just get an initial draft of this functionality that uses notify
and only implements the 'reload currently-open files if they changed on disk and are not dirty' part? I'd be up for implementing that.
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 git pull --rebase --autostash
wich apparently makes helix think any and all open files were modified externally and it usually takes me a few frustrated debug prints() in my code to notice that it's not a problem with the code but with helix refusing to save the file without :w!
and showing only the briefest of mentions that that's what's happening.
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?
@rcorre
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 :reload-all
. This would keep happening regardless of any 'save on suspend' feature.
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, helix
has to run into a tmux
session for this to work.
Using inotifywait: sudo apt install inotify-tools
In this simplest example, that command is running into a byobu
split, the Helix editor is in the first split. It does reload the file effectively, when helix
is in normal mode.
while inotifywait -e modify ~/Desktop; do byobu send-keys -t 0 ":rla" Enter; done;
Using tmux
only, we have to pass in the session name and the split number. It allows to be ran in the background. The -r
flag is for recursive.
while inotifywait -r -e modify ~/Desktop; do tmux send-keys -t mytmuxsession:0 ":rla" Enter; done;
It works, but of course, this feels very hacky, because it is. Looking forward for such watcher capabilities built-in.
It could be nice that helix
accepts a given POSIX signal to trigger a reload.
Links to help building such watcher script: https://superuser.com/a/181543 https://unix.stackexchange.com/a/409863
It could be nice that helix accepts a given POSIX signal to trigger a reload.
Yeah, I was thinking the same thing. How does everyone feel about this?
It could be nice that helix accepts a given POSIX signal to trigger a reload.
Yeah, I was thinking the same thing. How does everyone feel about this?
At that stage you could simply check every X number of key presses for the active buffer if the source file has changed and if so notify the user about it. There is no need to have a full-on file monitor or external service, possibly monitoring and signalling for multiple files for something that can be done on actual user-interaction to an open buffer.
A POSIX signal is a nice addition but the practical use feels extremely limited and it also would not work on non-posix platforms.
At that stage you could simply check every X number of key presses for the active buffer if the source file has changed and if so notify the user about it
It's not the best experience to find out a few keypresses in that the file changed. I'd rather know right away.
A POSIX signal is a nice addition but the practical use feels extremely limited and it also would not work on non-posix platforms.
Agreed, but it sounds like there's a lot of pushback against a file monitor, and this feels like a pretty lightweight solution that would satisfy a lot of folks. If we could detect SIGCONT, that would make me happy.
Is there pushback against a file monitor? The proposed solutions such as "check every X keystrokes" or "just reload on SIGCONT/some other signal" seem more like workarounds to the lack of a file watcher rather than the ideal solution, and have flaws of their own (the former leads to lost of pointless disk IO, the latter has no way to indicate which file to reload and also requires a very specific workflow).
Is there pushback against a file monitor? The proposed solutions such as "check every X keystrokes" or "just reload on SIGCONT/some other signal" seem more like workarounds to the lack of a file watcher rather than the ideal solution, and have flaws of their own (the former leads to lost of pointless disk IO, the latter has no way to indicate which file to reload and also requires a very specific workflow).
If it's a monitor that runs inside helix that would be fine as it could limit the checks to when helix is active / the visible buffers. In that regard it would be a clean replacement to my 'x' keystrokes (which btw is next to no io overhead as all you do is query the metadata not the file contents).
An external monitor is shooting canons at birds as it would likely monitor and signal helix for multiple files without the ability to detect if helix is presently active (vs. suspended with ctrl-z or simply tabbed out). Nor would it have any ability to know which buffer is active in helix, which then would cause unnecessary signalling for buffers the users doesn't have visible right now and which would need to be discarded. So for that one: yes, clear pus back from at least one person 🤚
(an integrated watcher in helix would also guarantee it is available on all platforms helix is on vs. the inotify suggested further up)
(an integrated watcher in helix would also guarantee it is available on all platforms helix is on vs. the inotify suggested further up)
Well, you still need to implement the watcher, at which point you use inotify or equivalent unless you want to poll on disk.
Overall I do think using the OS's file-watcher system is the best approach here, since it's literally what they're there for and all the major operating systems (Linux, OSX, Windows, BSDs) have one.
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 :rl
(reload) all of the time.
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 :watch
command that works on active buffer to watrch to see if that file changes, this way you can quick build (yeah, a little hacky) a buffer based watch routine in Helix. (or even simpler, on selecting a buffer check file chanegs, if active buffer just do on a simple timer that starts from a keypress in the buffer and tracks time since last keypress, kinda thang)
this solves issue of and problems with a watch every file solution, and just doing a :watch
:watch-off
is a lot less hassle.
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 rl
too manyt times, and simply now using nvim in seperate session to watch the file for updates live... :(
just run into this issue while building a lsp, and testing using Helix, kind of annoying to
:rl
(reload) all of the time.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
:watch
command that works on active buffer to watrch to see if that file changes, this way you can quick build (yeah, a little hacky) a buffer based watch routine in Helix. (or even simpler, on selecting a buffer check file chanegs, if active buffer just do on a simple timer that starts from a keypress in the buffer and tracks time since last keypress, kinda thang)this solves issue of and problems with a watch every file solution, and just doing a
:watch
:watch-off
is a lot less hassle.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
rl
too manyt times, and simply now using nvim in seperate session to watch the file for updates live... :(
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 tail -f ...
works good enough for LSP debugging, must admit Helix is the easiest editor by far for adding a new LSP dont get me started on vscode lsp dev.
I got started with ut but it's pretty complex to implement.
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.
Just one small correction: offering an (optional) prompt to reload would be modern.
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 on-external-change
(what to do when file changes externally) with following parameters:
ignore
(do nothing, if the file is not "dirty") reload
(default)tail
(reload and scroll to the end of the file)prompt
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. :)
Here is how it is done in Kakoune.
There is an option:
autoreload enum(yes|no|ask)
default ask
auto reload the buffers when an external modification is detected
Here is a pop-up which is shown when autoreload
is set to ask
:
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 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 notify
crate, so the following comments apply to it.
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 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 inotify
on a big number of watchers?
See https://github.com/helix-editor/helix/issues/8073#issuecomment-1902666294 regarding that
And more directly, https://matrix.to/#/!zMuVRxoqjyxyjSEBXc:matrix.org/$j0U306IWclmC5iO_kgCvn286QGruwM71iqFXem2-xBc?via=matrix.org&via=mozilla.org&via=tchncs.de
The notify crate is the current goto solution in rust but it's too unreliable in my opinion. There is no automatic way to recover from dropped events (which can occur frequently, articels on the linux kernel mailing list explaining how to use inotify specically recommend not to use the architecture notify is using). Instead a client would have to restart notify in those situations which is very slow and makes downstream logic complicated. There is also no way to query the current fs state (which a filewatcher needs to keep anyway to work properly so maintaining that downstream is both inefficient and quite cumbersome. There are a bunch more technical shortcomings but those are details that are hard to understand.
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
:reload-all
. This would keep happening regardless of any 'save on suspend' feature.another reason this is required: the colors to the left marking git status don't update if I do git commit in another terminal
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.
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)
Oh, I just noticed a potential pitfall with this: reload
discard changes instead of requiring reload!
for changed files, so this is not a great solution :/
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: