Created attachment 24922 [details] [review]
vfsmm-async-newclass.patch

1. VolumeMonitor::get_volume_for_path(), get_volume_by_id(),
get_drive_by_id() and Volume::get_drive() should have both a non-const
and const version.
2. VolumeMonitor::get_*_by_id() should probably be get_*() anyway. We
don't do the get_*_by_* thing in C++, because we don't need to.
3. VolumeMonitor::volume_mounted(), volume_pre_unmount(),
volume_unmounted(), drive_connected(), and drive_disconnected() should
take const Glib::RefPtr<>& instead of const Glib::RefPtr<>.
4. In your enums.ccg, you have a lot of Value template
specialisations, which I would expect gmmproc to generate for you.
They seem to have nothing to do with the async changes. Is there
something similar in gtkmm?
5. Handle::set_file_info() takes a Glib::RefPtr<Uri> instead of a
Glib::RefPtr<Uri>&.
6, Handle::create(), create_symbolic_link(), create_as_channel() and
load_directory(), open() and open_as_channel() take a const
Glib::RefPtr<Uri> instead of a const Glib::RefPtr<Uri>&.
7. The Drive::mounted() signal and friends should take references to
the Glib::RefPtr, not copy-by-value, as above.
8. Drive::compare should maybe be an operator==(). We might have a
gmmproc macro for that, but we could hand-code it easily.
9. Drive::get_mounted_volume() should have const and non-const
versions. A const by-value return type is meaningless.
10. The use of _CC_INCLUDE() seems unnecessary - why not just put the
#includes in the .ccg file?
11. In Volume::unmount(), you don't create a SignalProxy - you copy
the slot instead. SignalProxy_VolumeOp::c_callback() then casts that
to a SignalProxy_VolumeOp which it is not. Ditto eject().
12. FileInfoResult: Why have you used a void* in the constructor? Why
isn't the implementation in the .ccg file?
13. Are FileInfoResult and FindDirectoryResult meant to own the
underlying C instance? If not, then that should be documented. If so,
then they should probably free it sometime.

Thats enough for now. Thanks for the great work even if I seem overly
critical. I'd like to see the new patch before it's committed.

1) done.
2) done.
3) done.
4) this was to wrap the new GType stuff in gnome-vfs.  I'll resubmit
this in a new patch.
5) done.
6) done.
7) done.
8) done, also for Volume and VolumeMonitor.
9) done.
10) done.
11) done.
12) In the ListHandle of FileInfoResult and FindDirectoryResult, it
wants a void*, so I put one in.  We may need TypeTraits for these
classes.  Not sure.
13) I'm not sure about this.  How do I tell if it's supposed to be owned?

All of the "done"'s are fixed in this new patch.

Created attachment 25020 [details] [review]
Fixed patch

I made the following changes:

A. You added const versions of VolumeMonitor::get_volume_for_path()
and similar, but the const ones did not return a const. Please look at
similar things in gtkmm if you are not sure what to do. You might want
to read the RefPtr appendix in the gtkmm book to understand what the
different consts actually mean.

B. I don't think you did 8. (tell me if I'm wrong), but it might not
be useful anyway, because it would need RefPtr::operator*(), so that
people could do *refThing == *refThing.

C. Added the optional _WRAP_METHOD() retreturn parameter to
Drive::get_mounted_volume() and similar.

D. Let's leave 12 and 13 for now. But we have to deal with them before
API freeez, so I'm leaving this open.

I have attached my patch (revision.patch) so you can see what I changed.

Created attachment 25027 [details] [review]
revision.patch