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

Add gio::FileEnumerator, gio::FileMonitor, gio::Vfs subclasses #1676

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

fbrouille
Copy link
Contributor

Allow custom implementation of gio::Vfs, gio::File, gio::FileEnumerator and gio::FileMonitor

@sdroege
Copy link
Member

sdroege commented Mar 4, 2025

Very interesting, thanks! OOC, what are you planning to implement with this?

Comment on lines 471 to 472
unsafe impl Send for MyFileEnumerator {}
unsafe impl Sync for MyFileEnumerator {}
Copy link
Member

Choose a reason for hiding this comment

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

These are implemented automatically if possible. Are they not, and why are they required here?

Copy link
Contributor Author

@fbrouille fbrouille Mar 5, 2025

Choose a reason for hiding this comment

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

Removed. It was old code while implementing the tests.


impl MyFileMonitor {
pub async fn tick(&self) {
std::thread::sleep(std::time::Duration::from_millis(10));
Copy link
Member

Choose a reason for hiding this comment

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

As this is an async function it should probably use an async sleep mechanism. glib::timeout_future() for example

Copy link
Contributor Author

@fbrouille fbrouille Mar 5, 2025

Choose a reason for hiding this comment

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

Done

self.parent_is_active()
}

fn get_file_for_path(&self, path: &GString) -> File {
Copy link
Member

Choose a reason for hiding this comment

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

You probably also need to implement the required pieces to implement gio::File for this to be useful, or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.
Initially i through I would have to implement 70+ methods of gio::File, but finally only few ones are required for tests of gio::Vfs subclass, so it is ok.

@sdroege
Copy link
Member

sdroege commented Mar 4, 2025

Just shortly looked over it, will do a proper review one of these days but generally it looks correct.

@fbrouille fbrouille force-pushed the gio_file_subclass branch from 94aacc1 to bbfa6db Compare March 4, 2025 13:03
self.parent_is_active()
}

fn get_file_for_path(&self, path: &GString) -> File {
Copy link
Member

Choose a reason for hiding this comment

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

should it take a GString or a PathBuf?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

path parameter in VfsExt::file_for_path is a &str, so I think it should be a &str.
Another alternative is to change path parameter in VfsExt::file_for_path and in gio::VfsImpl::get_file_for_path to impl AsRef<std::path::Path> as in gio::File::for_path .
What do you think ?

Copy link
Member

Choose a reason for hiding this comment

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

We should fix the annotation upstream for VfsExt::file_for_path then

@fbrouille fbrouille force-pushed the gio_file_subclass branch from bbfa6db to ce47c8b Compare March 5, 2025 18:52
@fbrouille
Copy link
Contributor Author

OOC I'm planning to implement some nautilus extension in Rust (very challenging 😅)

}
}

unsafe impl IsImplementable<MyFile> for File {
Copy link
Member

Choose a reason for hiding this comment

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

Can you split this into subclass/file.rs even if it only implements a small subset of the vfuncs, the rest can be added later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants