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

fbrouille · 2025-03-04T12:46:54Z

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

sdroege · 2025-03-04T12:54:43Z

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

sdroege · 2025-03-04T12:56:44Z

gio/src/subclass/file_enumerator.rs

+    unsafe impl Send for MyFileEnumerator {}
+    unsafe impl Sync for MyFileEnumerator {}


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

Removed. It was old code while implementing the tests.

sdroege · 2025-03-04T12:58:36Z

gio/src/subclass/file_monitor.rs

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


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

sdroege · 2025-03-04T12:59:44Z

gio/src/subclass/vfs.rs

+        self.parent_is_active()
+    }
+
+    fn get_file_for_path(&self, path: &GString) -> File {


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

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 · 2025-03-04T13:00:28Z

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

bilelmoussaoui · 2025-03-04T13:10:23Z

gio/src/subclass/vfs.rs

+        self.parent_is_active()
+    }
+
+    fn get_file_for_path(&self, path: &GString) -> File {


should it take a GString or a PathBuf?

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 ?

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

fbrouille · 2025-03-05T19:12:22Z

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

bilelmoussaoui · 2025-03-05T20:50:35Z

gio/src/subclass/vfs.rs

+            }
+        }
+
+        unsafe impl IsImplementable<MyFile> for File {


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.

bilelmoussaoui · 2025-04-19T06:52:37Z

would you mind rebasing & finishing the small remaining bits here?

fbrouille · 2025-04-19T09:06:45Z

Yes, I'm adding subclass/file.rs (with almost all vfuncs), tests and a complete example to show how to implement a custom Vfs. I will share it during next week.

bilelmoussaoui · 2025-04-26T20:16:37Z

gio/src/subclass/file_monitor.rs

+
+// Support parent implementation of virtual functions defined in `gio::ffi::GFileMonitorClass`.
+pub trait FileMonitorImplExt: FileMonitorImpl {
+    fn parent_cancel(&self) -> bool {


the return of this vfunc doesn't seem to be used https://gitlab.gnome.org/GNOME/glib/-/blob/main/gio/gfilemonitor.c#L254, weird but maybe we can make it return Option and fallback to TRUE if the user has no clue what to return (which is expected, i guess?)

the documentation states that this cancel must always return true.(https://docs.gtk.org/gio/vfunc.FileMonitor.cancel.html)
so I suggest to simply debug_assert! the value returned by parent.
And maybe document the method to indicate that the return value must be true.

The docs will get embedded automatically, so maybe a debug_assert with an in-code comment is good enough

bilelmoussaoui · 2025-04-26T20:17:23Z

gio/src/subclass/file_enumerator.rs

+        // default implementation provided by GIO
+        // klass.next_files_async = Some(next_files_async::<T>);
+        // klass.next_files_finish = Some(next_files_finish::<T>);
+        // klass.close_async = Some(close_async::<T>);
+        // klass.close_finish = Some(close_finish::<T>);


Maybe add a TODO comment so they can easily be grep-ed for ?

or at least document the why it is like this

done (also in subclass/file.rs)

bilelmoussaoui · 2025-04-26T20:19:08Z

gio/src/file.rs

@@ -1111,6 +1113,98 @@ pub trait FileExtManual: IsA<File> + Sized {

        (fut, Box::pin(receiver))
    }
+
+    #[doc(alias = "g_file_set_attribute")]


this could be split and we can land it already

in a separate Pull Request ?

yes, to avoid having a very big PR here that will take forever to review

done. See #1713

bilelmoussaoui · 2025-04-30T14:58:32Z

gio/tests/file_monitor.rs

+                use gio::{Cancellable, File, FileMonitorFlags};
+                let _ = File::for_uri("resource:/").monitor_file(FileMonitorFlags::NONE, Cancellable::NONE);


can't you create a GType from the names and then calling T::ensure_type on it?

unfortunately it does not work and parent type is not initialized, that is why it was indirectly initialized by calling File::monitor_file, as a workaround.

bilelmoussaoui · 2025-04-30T14:59:37Z

gio/tests/file_monitor.rs

+glib::wrapper! {
+    #[doc(alias = "GResourceFileMonitor")]
+    pub struct ResourceFileMonitor(Object<ffi::GResourceFileMonitor, ffi::GResourceFileMonitorClass>) @extends FileMonitor;


ResourceFileMonitor why is this here? i don't feel like it should be part of the test...

Yes it seems strange, but ResourceFileMonitor (which is not a public GType) is the easier existing type to subclass for this test.
Anyway, I reviewed the code to not rely on existing type. Therefore, this test defines 2 custom types: MyCustomFileMonitor which subclasses MyFileMonitor.
Maybe I will have to review in the same way tests for FileEnumerator, File and Vfs.

bilelmoussaoui · 2025-05-21T09:29:02Z

gio/src/subclass/file_monitor.rs

@@ -0,0 +1,56 @@
+// Take a look at the license at the top of the repository in the LICENSE file.


Given that this one is very simple, we can review/merge it in a separate PR already.

bilelmoussaoui · 2025-05-21T09:29:25Z

gio/src/subclass/vfs.rs

@@ -0,0 +1,184 @@
+// Take a look at the license at the top of the repository in the LICENSE file.
+
+use glib::{prelude::*, subclass::prelude::*, translate::*, GString};


Maybe split this one as well

Signed-off-by: fbrouille <[email protected]>

bilelmoussaoui · 2025-06-17T09:38:36Z

gio/src/subclass/file_enumerator.rs

@@ -0,0 +1,154 @@
+// Take a look at the license at the top of the repository in the LICENSE file.


Can you pull this into a separate PR as well? so you only keep GFile for this one.

sdroege reviewed Mar 4, 2025

View reviewed changes

fbrouille force-pushed the gio_file_subclass branch from 94aacc1 to bbfa6db Compare March 4, 2025 13:03

bilelmoussaoui reviewed Mar 4, 2025

View reviewed changes

fbrouille force-pushed the gio_file_subclass branch from bbfa6db to ce47c8b Compare March 5, 2025 18:52

bilelmoussaoui reviewed Mar 5, 2025

View reviewed changes

fbrouille force-pushed the gio_file_subclass branch from ce47c8b to b4e9183 Compare April 26, 2025 09:53

fbrouille changed the title ~~Add gio::FileEnumerator, gio::FileMonitor, gio::Vfs subclasses~~ Add gio::File, gio::FileEnumerator, gio::FileMonitor, gio::Vfs subclasses Apr 26, 2025

fbrouille force-pushed the gio_file_subclass branch 9 times, most recently from e9842cc to 10bb6d0 Compare April 30, 2025 11:12

bilelmoussaoui reviewed Apr 30, 2025

View reviewed changes

fbrouille force-pushed the gio_file_subclass branch 2 times, most recently from 260cf0e to 0cb4a44 Compare May 3, 2025 07:38

fbrouille force-pushed the gio_file_subclass branch from 0cb4a44 to a0345a9 Compare May 20, 2025 15:54

bilelmoussaoui reviewed May 21, 2025

View reviewed changes

fbrouille mentioned this pull request May 23, 2025

Add gio::FileMonitor subclass #1725

Merged

fbrouille mentioned this pull request May 23, 2025

Add gio::Vfs subclass #1726

Merged

fbrouille added 4 commits May 29, 2025 15:38

Add gio::FileEnumerator subclass

3ce3935

Signed-off-by: fbrouille <[email protected]>

Add gio::File subclass

303648a

Signed-off-by: fbrouille <[email protected]>

Add gio::Vfs subclass

faa3f81

Signed-off-by: fbrouille <[email protected]>

Add an example for gio::Vfs implementation

72eca83

Signed-off-by: fbrouille <[email protected]>

fbrouille force-pushed the gio_file_subclass branch from a0345a9 to 72eca83 Compare June 4, 2025 16:11

bilelmoussaoui reviewed Jun 17, 2025

View reviewed changes

		unsafe impl Send for MyFileEnumerator {}
		unsafe impl Sync for MyFileEnumerator {}

		use gio::{Cancellable, File, FileMonitorFlags};
		let _ = File::for_uri("resource:/").monitor_file(FileMonitorFlags::NONE, Cancellable::NONE);

		@@ -0,0 +1,56 @@
		// Take a look at the license at the top of the repository in the LICENSE file.

		@@ -0,0 +1,184 @@
		// Take a look at the license at the top of the repository in the LICENSE file.

		use glib::{prelude::, subclass::prelude::, translate::*, GString};

		@@ -0,0 +1,154 @@
		// Take a look at the license at the top of the repository in the LICENSE file.

Uh oh!

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

fbrouille commented Mar 4, 2025

Uh oh!

sdroege commented Mar 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fbrouille Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fbrouille Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sdroege commented Mar 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fbrouille commented Mar 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bilelmoussaoui commented Apr 19, 2025

Uh oh!

fbrouille commented Apr 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fbrouille May 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fbrouille Mar 5, 2025 •

edited

Loading

fbrouille Mar 5, 2025 •

edited

Loading

fbrouille May 3, 2025 •

edited

Loading