Skip to content
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

[WIP] file watching #588

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
105 changes: 105 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 15 additions & 1 deletion helix-term/src/application.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use helix_core::syntax;
use helix_lsp::{lsp, util::lsp_pos_to_pos, LspProgressMap};
use helix_view::{theme, Editor};
use helix_view::{file_watcher, theme, Editor};

use crate::{args::Args, compositor::Compositor, config::Config, job::Jobs, ui};

Expand Down Expand Up @@ -192,6 +192,9 @@ impl Application {
self.jobs.handle_callback(&mut self.editor, &mut self.compositor, callback);
self.render();
}
Some(msg) = self.editor.watcher.receiver.recv() => {
self.handle_watcher_message(msg)
}
}
}
}
Expand Down Expand Up @@ -490,6 +493,17 @@ impl Application {
}
}

fn handle_watcher_message(&mut self, msg: file_watcher::Message) {
match msg {
file_watcher::Message::NotifyEvent(event) => {
log::info!("handling file watcher event: {:?}", event);
}
// match event.unwrap() {
// _ => unimplemented!(),
// },
}
}

async fn claim_term(&mut self) -> Result<(), Error> {
terminal::enable_raw_mode()?;
let mut stdout = stdout();
Expand Down
5 changes: 5 additions & 0 deletions helix-view/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ tokio = { version = "1", features = ["rt", "rt-multi-thread", "io-util", "io-std
futures-util = { version = "0.3", features = ["std", "async-await"], default-features = false }

slotmap = "1"
rustc-hash = "1.1.0"
oberblastmeister marked this conversation as resolved.
Show resolved Hide resolved

encoding_rs = "0.8"
chardetng = "0.1"
Expand All @@ -38,6 +39,10 @@ log = "~0.4"

which = "4.2"

crossbeam-channel = "0.5.1"
notify = "=5.0.0-pre.11" # check that it builds on NetBSD before upgrading
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we need to use a pre-release version instead of current stable? (Which I believe is 4.x)

Copy link
Member

Choose a reason for hiding this comment

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

Last time I looked at it, it seemed like they were supporting both versions and 5 seemed stable enough maybe

jod-thread = "0.1.2"

[target.'cfg(windows)'.dependencies]
clipboard-win = { version = "4.2", features = ["std"] }

Expand Down
16 changes: 12 additions & 4 deletions helix-view/src/editor.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{
clipboard::{get_clipboard_provider, ClipboardProvider},
file_watcher::{self, NotifyActor, NotifyHandle},
graphics::{CursorKind, Rect},
theme::{self, Theme},
tree::Tree,
Expand All @@ -13,6 +14,7 @@ use std::{
time::Duration,
};

use rustc_hash::FxHashMap;
use slotmap::SlotMap;

use anyhow::Error;
Expand Down Expand Up @@ -52,12 +54,14 @@ impl Default for Config {
pub struct Editor {
pub tree: Tree,
pub documents: SlotMap<DocumentId, Document>,
pub path_to_doc: FxHashMap<PathBuf, DocumentId>,
Copy link
Member

Choose a reason for hiding this comment

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

I'd call this documents_by_path and while I think it's useful, you should probably keep the original code which loops through the open documents. This new hashmap doesn't handle possible renames via set_path or set_path + write

pub count: Option<std::num::NonZeroUsize>,
pub selected_register: RegisterSelection,
pub registers: Registers,
pub theme: Theme,
pub language_servers: helix_lsp::Registry,
pub clipboard_provider: Box<dyn ClipboardProvider>,
pub watcher: NotifyHandle,

pub syn_loader: Arc<syntax::Loader>,
pub theme_loader: Arc<theme::Loader>,
Expand Down Expand Up @@ -90,6 +94,7 @@ impl Editor {
Self {
tree: Tree::new(area),
documents: SlotMap::with_key(),
path_to_doc: FxHashMap::default(),
count: None,
selected_register: RegisterSelection::default(),
theme: themes.default(),
Expand All @@ -98,6 +103,7 @@ impl Editor {
theme_loader: themes,
registers: Registers::default(),
clipboard_provider: get_clipboard_provider(),
watcher: NotifyActor::spawn(),
status_msg: None,
config,
}
Expand Down Expand Up @@ -218,10 +224,7 @@ impl Editor {
pub fn open(&mut self, path: PathBuf, action: Action) -> Result<DocumentId, Error> {
let path = crate::document::canonicalize_path(&path)?;

let id = self
.documents()
.find(|doc| doc.path() == Some(&path))
.map(|doc| doc.id);
let id = self.path_to_doc.get(&path).map(|it| *it);

let id = if let Some(id) = id {
id
Expand Down Expand Up @@ -253,6 +256,11 @@ impl Editor {

let id = self.documents.insert(doc);
self.documents[id].id = id;
self.watcher
Copy link
Member

Choose a reason for hiding this comment

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

Now that I think about it, this needs to deal with renaming too, :w somename.txt could set a name for an unnamed file or write an existing file to a new location

.sender
.send(file_watcher::ActorMessage::Watch(path.clone()))
.unwrap();
self.path_to_doc.insert(path, id);
id
};

Expand Down
97 changes: 97 additions & 0 deletions helix-view/src/file_watcher.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
use crossbeam_channel::unbounded;
use crossbeam_channel::{select, Receiver};
use notify::{recommended_watcher, RecommendedWatcher, RecursiveMode, Watcher};
use std::path::PathBuf;
use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver};

pub struct NotifyActor {
sender: Sender,
watcher: (RecommendedWatcher, Receiver<NotifyEvent>),
}

impl NotifyActor {
pub fn spawn() -> NotifyHandle {
let (actor_tx, actor_rx) = crossbeam_channel::unbounded();
let (tx, rx) = unbounded_channel();
let actor = NotifyActor::new(Box::new(move |msg| tx.send(msg).unwrap()));
let thread = jod_thread::Builder::new()
oberblastmeister marked this conversation as resolved.
Show resolved Hide resolved
.name("FileWatcher".to_string())
.spawn(move || actor.run(actor_rx))
.expect("Failed to spawn file watcher");
NotifyHandle {
sender: actor_tx,
receiver: rx,
thread,
}
}

fn next_event(
&mut self,
receiver: &crossbeam_channel::Receiver<ActorMessage>,
) -> Option<ActorEvent> {
let watcher_receiver = &self.watcher.1;
select! {
oberblastmeister marked this conversation as resolved.
Show resolved Hide resolved
recv(receiver) -> it => it.ok().map(ActorEvent::Message),
recv(watcher_receiver) -> it => Some(ActorEvent::NotifyEvent(it.unwrap())),
}
}

fn new(sender: Sender) -> NotifyActor {
let (tx, rx) = unbounded();
let watcher: RecommendedWatcher = recommended_watcher(move |e| tx.send(e).unwrap()).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Return a result and propagate the error here.

Copy link
Member

Choose a reason for hiding this comment

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

I also noticed that v4 supported debounce, but seems like that's removed in v5?

NotifyActor {
sender,
watcher: (watcher, rx),
}
}

fn run(mut self, inbox: crossbeam_channel::Receiver<ActorMessage>) {
Copy link
Member

Choose a reason for hiding this comment

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

I think you could merge next_event and run, you construct an enum then immediately deconstruct it.

while let Some(event) = self.next_event(&inbox) {
match event {
ActorEvent::Message(msg) => self.handle_message(msg),
ActorEvent::NotifyEvent(event) => self.send(Message::NotifyEvent(event)),
}
}
}

fn handle_message(&mut self, msg: ActorMessage) {
use ActorMessage::*;

// probably log errors
match msg {
Watch(path) => self.watcher.0.watch(&path, RecursiveMode::NonRecursive),
Unwatch(path) => self.watcher.0.unwatch(&path),
};
}

fn send(&mut self, msg: Message) {
(self.sender)(msg)
}
}

pub enum ActorMessage {
Watch(PathBuf),
Unwatch(PathBuf),
}

pub enum ActorEvent {
Message(ActorMessage),
NotifyEvent(NotifyEvent),
}

#[derive(Debug)]
pub enum Message {
NotifyEvent(NotifyEvent),
}

type NotifyEvent = notify::Result<notify::Event>;

type Sender = Box<dyn Fn(Message) + Send>;

#[derive(Debug)]
pub struct NotifyHandle {
// Relative order of fields below is significant.
pub sender: crossbeam_channel::Sender<ActorMessage>,
thread: jod_thread::JoinHandle,
pub receiver: UnboundedReceiver<Message>,
}