Add file system watcher to FileSelector and FileBrowser widgets #76
Comments
|
I took a look at the code for the "file_browser" widget and it seems like there is going to need to be some integration of the "notify-rs" crate into the overall event loop and then send messages to the widget. Does that sound about right? Where should I be looking to do the event loop integration? |
|
I checked This is just modified example from use notify::{Watcher, RecommendedWatcher, RecursiveMode, Result};
fn setup_watcher<M: MessageData, C: Control<M, C>>(sender: Sender<UiMessage<M, C>>, path: &Path) -> Result<()> {
let mut watcher = notify::recommended_watcher(|res| {
match res {
Ok(event) = { sender.send( specific_file_browser_message )... },
Err(e) => println!("watch error: {:?}", e),
}
})?;
// Add a path to be watched. All files and directories at that path and
// below will be monitored for changes.
watcher.watch(path, RecursiveMode::Recursive)?;
Ok(())
}It is very simple - rg3d-ui uses channels for communication, it allows us to send message from any thread (file system watcher works in separate thread). To get |
|
OK, I'm going to try implementing this and then creating a PR. I'm fairly beginner with Rust and no experience with RG3D so it might take me some time, but I'm pretty sure I can implement this. |
|
Thanks! Just tell me if you need help, I'll do my best to help you. |
|
OK, I've made some progress, but I'm running into issues because "M" is not "Send" in the generics. I've added the following code to FileBrowserBuilder.build: let widget = self.widget_builder.with_child(grid).build();
let ui_sender = ctx.ui.sender();
let ui_to_send_to = widget.handle();
let (tx, rx) = std::sync::mpsc::channel();
let watcher = match notify::watcher(tx, time::Duration::from_secs(1)) {
Ok(watcher) => Some(watcher),
Err(_) => None,
};
let watcher_conversion_thread = std::thread::spawn(move || loop { // HERE GETTING ERROR THAT 'M' IS NOT Send - I'm kinda stuck here
match rx.recv() {
Ok(event) => match event {
notify::DebouncedEvent::NoticeRemove(path)
| notify::DebouncedEvent::Remove(path) => {
ui_sender.send(FileBrowserMessage::remove(
ui_to_send_to,
MessageDirection::ToWidget,
path,
));
}
notify::DebouncedEvent::Create(path) => {
ui_sender.send(FileBrowserMessage::add(
ui_to_send_to,
MessageDirection::ToWidget,
path,
));
}
notify::DebouncedEvent::Rescan | notify::DebouncedEvent::Error(_, _) => {
ui_sender.send(FileBrowserMessage::rescan(
ui_to_send_to,
MessageDirection::ToWidget,
));
}
_ => (),
},
Err(_) => break,
}
});
let browser = FileBrowser {
widget,
tree_root,
path_text,
path: match self.mode {
FileBrowserMode::Open => self.path,
FileBrowserMode::Save {
ref default_file_name,
} => self.path.join(default_file_name),
},
file_name_value: match self.mode {
FileBrowserMode::Open => Default::default(),
FileBrowserMode::Save {
ref default_file_name,
} => default_file_name.clone(),
},
filter: self.filter,
mode: self.mode,
scroll_viewer,
root: self.root,
file_name,
watcher: Rc::new(watcher),
watcher_conversion_thread,
};
ctx.add_node(UINode::FileBrowser(browser))I'm not sure this approach is going to work based on the constraints of the types "M" involved. Any thoughts? |
|
You could just add pub trait MessageData: 'static + Debug + Clone + PartialEq + Send {}I just checked and it compiles without any issues. |
|
Yeah, I tried that but then I get a host of other errors at the definition of the closure for the conversion thread. If I do that, I get the following: std::rc::Rc<std::cell::RefCell<(dyn for<'r> std::ops::FnMut(&'r std::path::Path) -> bool + 'static)>> It seems like file_browser::Filter might be the issue now. I'll check if there is a way to make that Send as well. I'm not sure why that is an issue though. It doesn't seem like something that the closure is capturing. |
|
I believe the problem is here: #[derive(Clone)]
pub struct Filter(pub Rc<RefCell<dyn FnMut(&Path) -> bool>>);from file_browser.rs. This seems like a problem in the Rc is not Send. What about making this an Arc? That would cascade into other things, but maybe it won't be too bad. What do you think? |
|
Ok, I think it is the time for |
|
Do you mean to change Rc in Filter definition to an Arc<...> and also do this unsafe thing? Filter is currently not Send because of the Rc not due to C (Control). Are you seeing another compile issue that I haven't run into yet surrounding the "C"? |
|
No, just add unsafe thing. I'm pretty sure that without manual |
|
Leaving Filter be a wrapper around Rc will not be sound to Send. Even if it were to compile using unsafe, it would be unsound (that is, Undefined Behavior). I really think it needs to be an Arc, like: #[derive(Clone)]
pub struct Filter(pub Arc<Mutex<dyn FnMut(&Path) -> bool>>);or somesuch. Otherwise, the "FileBrowserMessage" will not be "Send". If this is not "Send" then you can't send FileBrowserMessage(s) from the conversion thread to the UI thread. Do you not agree? |
|
UiMessage does not contain any instance of any widget, so manual Send implementation is safe in this case. The only reason why it has |
|
As near as I can tell, the issue is not "C". It is the Rc in Filter as well as teh FnMut not having the Send bound. file_browser.rs pub struct Filter(pub Arc<Mutex<dyn FnMut(&Path) -> bool + Send>>);and message.rs pub trait MessageData: 'static + Debug + Clone + PartialEq + Send {}This seems sufficient to resolve the issues with sending UI Messages across thread boundaries. EDIT: Of course, I updated the rest of the code to handle the change to Filter (which wasn't that many changes). If you're OK with it, I'll continue down this path. |
|
Aha, that's good! Sorry, I was replying in the middle of the night and I was "a bit" tired 😅 |
|
Hello again. I'm running into an issue that the messages are not getting through to the "handle_routed_message" method of the "FileBrowser". Here is the code that builds the FS watcher and conversion thread to send messages to the "FileBrowser": In FileBrowserBuilder.build: let widget = self.widget_builder.with_child(grid).build();
let widget_handle = widget.handle();
let the_path = match &self.root {
Some(path) => path.clone(),
_ => self.path.clone(),
};
let browser = FileBrowser {
widget,
tree_root,
path_text,
path: match self.mode {
FileBrowserMode::Open => self.path,
FileBrowserMode::Save {
ref default_file_name,
} => self.path.join(default_file_name),
},
file_name_value: match self.mode {
FileBrowserMode::Open => Default::default(),
FileBrowserMode::Save {
ref default_file_name,
} => default_file_name.clone(),
},
filter: self.filter,
mode: self.mode,
scroll_viewer,
root: self.root,
file_name,
watcher: Rc::new(cell::Cell::new(None)),
};
let watcher = browser.watcher.clone();
let filebrowser_node = UINode::FileBrowser(browser);
let node = ctx.add_node(filebrowser_node);
watcher.replace(setup_filebrowser_fs_watcher(
ctx.ui.sender(),
widget_handle,
the_path,
));
nodeHere is the code that sets the FS watcher up (setup_filebrowser_fs_watcher): fn setup_filebrowser_fs_watcher<M: MessageData, C: Control<M, C>>(
ui_sender: mpsc::Sender<UiMessage<M, C>>,
filebrowser_widget_handle: Handle<UINode<M, C>>,
the_path: PathBuf,
) -> Option<(notify::RecommendedWatcher, thread::JoinHandle<()>)> {
let (tx, rx) = mpsc::channel();
match notify::watcher(tx, time::Duration::from_secs(1)) {
Ok(mut watcher) => {
#[allow(clippy::while_let_loop)]
let watcher_conversion_thread = std::thread::spawn(move || loop {
println!("Waiting for FS Watcher Event....");
match rx.recv() {
Ok(event) => match event {
notify::DebouncedEvent::NoticeRemove(path)
| notify::DebouncedEvent::Remove(path) => {
println!("Sent Remove Message");
match ui_sender.send(FileBrowserMessage::remove(
filebrowser_widget_handle,
MessageDirection::ToWidget,
path,
)) {
Ok(_) => println!("Successfully sent Remove message"),
Err(_) => println!("Failed to Send Remove Message"),
}
}
notify::DebouncedEvent::Create(path) => {
println!("Sent Create Message");
let _ = ui_sender.send(FileBrowserMessage::add(
filebrowser_widget_handle,
MessageDirection::ToWidget,
path,
));
}
notify::DebouncedEvent::Rescan | notify::DebouncedEvent::Error(_, _) => {
println!("Sent Rescan Message");
let _ = ui_sender.send(FileBrowserMessage::rescan(
filebrowser_widget_handle,
MessageDirection::ToWidget,
));
}
_ => {
println!("Ignored FS Watcher Event");
()
}
},
Err(_) => {
println!("Breaking out of FS Watcher Event Loop");
break;
}
};
});
println!(
"Starting FS Event watch on Path: {}",
the_path.to_str().unwrap()
);
let _ = watcher.watch(the_path, notify::RecursiveMode::Recursive);
Some((watcher, watcher_conversion_thread))
}
Err(_) => None,
}
}And then here is the code to handle the messages that are received (in "handle_routed_message"): match &message.data() {
UiMessageData::FileBrowser(msg) => {
if message.destination() == self.handle() {
match msg {
FileBrowserMessage::Path(path) => {...}
FileBrowserMessage::Root(root) => {...}
FileBrowserMessage::Filter(filter) => {...}
FileBrowserMessage::Add(path) => {
// Check if leading path is expanded, if so, add item to expanded tree
println!("Add message received");
todo!("FileBrowserMessage::Add Not Implemented");
}
FileBrowserMessage::Remove(path) => {
// Check if leading path is expanded, if so, remove item from expanded tree
println!("Remove message received");
todo!("FileBrowserMessage::Remove Not Implemented");
}
FileBrowserMessage::Rescan => {
//
println!("Rescan message received");
todo!("FileBrowserMessage::Rescan Not Implemented");
}
}
}
}
UiMessageData::TextBox(TextBoxMessage::Text(txt)) => {...}
UiMessageData::Tree(TreeMessage::Expand { expand, .. }) => {...}
UiMessageData::TreeRoot(msg) => {...}
_ => {}
}I'm seeing that that the "Add", "Remove" messages are being sent (due to the println!), but the messages are never arriving at the handler. Do I have the right ui sender (ui.ctx.sender())? Do I have the right handle to send to? Am I using the correct API to send the messages? Any thoughts on what could be amiss? |
|
This let node = ctx.add_node(filebrowser_node);
watcher.replace(setup_filebrowser_fs_watcher(
ctx.ui.sender(),
node,
the_path,
));
node |
|
Aaarrrrgh! That did it. Now, there is another issue I'm not sure how best to resolve given the API. Perhaps you can give me some insight into something I'm not seeing. When the FileBrowser is built, the working directory is the start-up directory of the rusty-editor. After the widget is built, and as a result, the watcher is setup to watch the rust-editor working directory, then the user selects the working directory and the working directory of the process is changed. Somehow, I need an event that the working directory was changed to get to the FileBrowser so that I can re-setup the watcher to the new directory. Any thoughts on what would be best given the architecture? |
|
I think this could be done in handler of |
|
Hello again. I'm running into a problem understanding the API for interacting with the Browser Tree. I have added the following code to handle the "Add" event for the FS Notifier: In match msg {
FileBrowserMessage::Path(path) => {...}
FileBrowserMessage::Root(root) => {...}
FileBrowserMessage::Filter(filter) => {...}
FileBrowserMessage::Add(path) => {
if let Some(filter) = self.filter.as_mut() {
if !filter.0.borrow_mut().deref_mut().lock().unwrap()(path) {
return;
}
}
let mut parent_path = path.clone();
parent_path.pop();
let existing_parent_node = find_tree(self.tree_root, &parent_path, ui);
if existing_parent_node.is_some() {
build_tree(
existing_parent_node,
existing_parent_node == self.tree_root,
path,
&parent_path,
ui,
);
}
}
FileBrowserMessage::Remove(_path) => {
println!("FileBrowserMessage::Remove Received and Ignored");
}
FileBrowserMessage::Rescan => {
println!("FileBrowserMessage::Rescan Received and Ignored");
}
}When I call the fn find_tree<M: MessageData, C: Control<M, C>, P: AsRef<Path>>(
node: Handle<UINode<M, C>>,
path: &P,
ui: &UserInterface<M, C>,
) -> Handle<UINode<M, C>> {
let mut tree_handle = Handle::NONE;
match ui.node(node) {
UINode::Tree(tree) => {
let tree_path = tree.user_data_ref::<PathBuf>().unwrap();
if tree_path == path.as_ref() {
tree_handle = node;
}
for &item in tree.items() {
let tree = find_tree(item, path, ui);
if tree.is_some() {
tree_handle = tree;
break;
}
}
}
UINode::TreeRoot(root) => {
for &item in root.items() {
let tree = find_tree(item, path, ui);
if tree.is_some() {
tree_handle = tree;
break;
}
}
}
_ => unreachable!(),
}
tree_handle
}When I step through with the debugger it goes into "find_tree", then to the "TreeRoot" of find_tree which then performs "find_tree" on the "Items" where the only item is the "." node. In the recursive call to find_tree it goes to the "Tree" route sees that the Here is what the debugger shows when it stops at the
Meanwhile, here is what the UI shows:
|
|
Hi! At first, I wrote a test to ensure that The Your code seems to be correct, it should add items only to expanded tree items, it makes no sense to add to collapsed because a user won't see them. |
It doesn't seem like it. Notice that the "tree_path" local variable is "." not "./src" or "src". Also, it got here by a single recursive call from the "TreeRoot" route which, from my reading of the code, never has an associated "pathBuf". So, the first and only node of the "TreeRoot" is the "." node. As can be seen from the debug output, "items" is empty on the "." node even though the UI clearly shows children. It should be iterating over "src", "assets", "git", etc. Instead, it iterates over nothing. It almost seems like there are two ui components, the one in the actual UI, and the one in the "FileBrowser" struct and they are out-of-sync. As if it were cloned, but, I can't see where that would be occurring. |
|
It is hard to tell what is wrong here, I checked the code and it seems to work fine. I mean I added a simple resursive print func that prints paths of trees (basically just stripped version of find_tree) and it correctly prints all the paths in the entire hierarchy. I need to see all your changes, if you have a repo, I could take a look. |
|
Sure thing. Here is a link to my GitHUB repo/fork of rg3d (https://github.com/gbutler69/rg3d) where I'm working on this. My intent was to sync up once I had it working and create a PR. |
|
Ok, I'm confused about
|
Yes, I know I'll need to strip off the root path when attempting to match the path; however, that doesn't change the fact that "items()" is empty in the "." node of the UI instead of having sub-nodes "src", "git", "assets", etc. Until that is resolved, it doesn't matter whether "path" is relative or absolute because there is no comparison happening. When you run the code from my repo, what shows in the UI? Does it appear differently than it does for me? The problem right now is not that the comparison fails but that the comparison of "path" to items.paths never occurs because "items()" is empty on the "." node. How can that be? What do you see when you run it? This is the procedure I'm using:
You'll notice that at this point What are you seeing when you run it? I'm befuddled at the behavior and can't make sense of how |
|
I managed to figure out what happens and why it "not working". But let's start from the beginning:
|
|
@mrDIMAS : OK, I have the add functionality mostly working. It correctly adds a new node to an open folder and does not add an item to a non-open folder; however, if a folder is initially empty, it is displayed in the widget without an expander. If you add an item to that folder, even though you don't want to display the new item, you want to add an expander to the existing folder that is displayed so that the user now knows that it can be expanded (there is something underneath it now). I tried adding the following code to the "tree" widget implementation for message handling: In &TreeMessage::SetExpanderShown(show) => {
self.always_show_expander = show;
}I send this message from the FileBrowser widget when an item is added to a folder that is displayed, but, was empty and so has no expander and is not currently expanded. This should then display the expander. The expander does not display. Is there something I need to call so that it knows to refresh the rendering? I've committed all these changes to my repo that you looked at before (https://github.com/gbutler69/rg3d). |
|
Cool, great job! To make The reason why it needs manual triggering, is because layout is very heavy and it is cached as much as possible. |
|
@mrDIMAS : OK, but, I found where the expander button is built and it is the following code: from ButtonBuilder::new(
WidgetBuilder::new()
.with_width(20.0)
.with_visibility(always_show_expander || items_populated)
.on_row(0)
.on_column(0),
)
.with_text(if is_expanded { "-" } else { "+" })
.build(ctx)This seems to indicate that I'll need to somehow set the button's visibility property before doing the arrange_override, or will the arrange_override cause that to recalculated somewhere already? |
|
You just need to call fn arrange_override(&self, ui: &UserInterface<M, C>, final_size: Vector2<f32>) -> Vector2<f32> {
let size = self.widget.arrange_override(ui, final_size);
if !self.always_show_expander {
let expander_visibility = !self.items.is_empty();
ui.send_message(WidgetMessage::visibility(
self.expander,
MessageDirection::ToWidget,
expander_visibility,
));
}
size
}Is you can see, expander will be shown if the tree has some items. |
|
@mrDIMAS : I should've looked for arrange_override before I questioned what you were saying. I have it working now, however, I needed to make the following changes to arrange_override: fn arrange_override(&self, ui: &UserInterface<M, C>, final_size: Vector2<f32>) -> Vector2<f32> {
let size = self.widget.arrange_override(ui, final_size);
let expander_visibility = !self.items.is_empty() || self.always_show_expander;
ui.send_message(WidgetMessage::visibility(
self.expander,
MessageDirection::ToWidget,
expander_visibility,
));
size
}Do you consider this to be a problem at all? |
|
It is fine. There is small drawback of this - it will send a message on each arrange call of tree, but I think it shouldn't be an issue, because by default |
|
OK, sounds good. That aligns with what I was thinking as well. I think I'm ready to create a PR for the "Add" portion or would you prefer we wait until Add & Remove is fully functional to have a PR? |
|
I'm not in a hurry, I think adding a Remove functionaly woudn't be too hard so take your time. |
|
Resolved in #176 |
Currently, if you open a FileSelector or FileBrowser and navigate to a folder and then add some content to the folder, it won't show up until you collapse\expand the folder. It can be fixed by adding file system watcher which will notify about changes in file system.
The text was updated successfully, but these errors were encountered: