GIO API review

[Michael Natterer <mitch imendio com>]

Hey everybody,

We've been doing a GIO API review in the last couple of days and
here is the list of comments and issues we've come up with:


General:
========

It seems GIO allows individual files to be included, this should be
avoided like gobject does it:

#if !defined (__GLIB_GIO_H_INSIDE__) && !defined (GIO_COMPILATION)
#error "Only <gio.h> can be included directly."
#endif


A big issue is that GIO wastes namespaces massively:
  g_app, GAsync, g_buffered, g_cancellable, g_content, g_data,
  g_desktop, g_directory, g_drive, g_file, g_filename, g_filter,
  g_icon, g_input, g_io, g_schedule, g_cancel, g_loadable,
  g_local, g_memory, g_mount, g_native, g_seekable, g_simple,
  g_themed, g_unix, g_vfs, g_volume, g_win32, g_push, g_pop

That's clearly too much and can be reduced. While these are not
strictly "namespaces" (the namespace is g_), they partly clash with
g_foos and g_bars we already have, or might need in the future. We
stronlgy suggest to use a common g_io/GIO prefix for all
functions/types in GIO.

GAsnc*        -> GIOAsync*
G*Stream      -> GIOStream*
GIcon         -> GIOIcon
G*Icon        -> GIOIcon*
GCancellable  -> GIOCancellable
...

Also, subclasses should probably append their name, not prepend it:

GFilterOutputStream -> GIOOutputStreamFilter
GUnixOutputStream   -> GIOOutputStreamUnix
...

This makes the file and inheritence structure much clearer and would
be consistent with stuff we already have (e.g. GtkTreeModelFilter,
GtkTreeModelSort).

All progress callbacks should have a cancellable argument, otherwise
you always need to pass the cancellable as user_data if your progress
dialog has a cancel button.

All parent instance members should be called "parent_instance" for
consistency, "parent" is a far too common member name to be used
like that.

Nitpick: The number of padding elements in class structures varies, why?


GFile:
======

g_mount_for_location should probably be g_file_mount_for_location

Some async operations are not implemented:
  _query_settable_attributes_async
  _query_writable_namespaces_async
  _delete_file_async
  _trash_async
  _make_directory_async
  _make_symbolic_link_async
  _copy_async
  _move_async

Is there an ETA for these? Especially copy_async() and move_async()
seem very important missing pieces.

A General issue with GFile is that the name suggests that you are holding
some kind of file handle, where you actually have nothing but a filename
(or uri). This could be problematic since the API does not make clear
which functions simply work on the filename representation in memory and
which do actual I/O (read: which function is cheap and which is expensive).

E.g. g_file_contains_file gives no hint about whether it's just doing
string operations on the filename, or actual I/O.

Since GFile is merely an interface, it can probably not be stated generally
which of the functions does I/O and which doesn't. It might however make
sense to mention that in the docs for the built-in default implementation
of local files.


GFileEnumerator:
================

GFileEnumerator -> GIOFileEnumerator


GMountOperation:
================

GMountOperation -> GIOMountOperation

G_PASSWORD_FLAGS_ANON_SUPPORTED -> G_IO_PASSWORD_FLAGS_ANONYMOUS_SUPPORTED

Most of the API seems to deal with setting user/pass/choice in response to
signals. Shouldn't there be one function to pass the entire set of response
fields related to one signal instead of a combo of setters plus reply() ?


G*Monitor:
==========

GFileMonitor      -> GIOMonitorFile
GDirectoryMonitor -> GIOMonitorDirectory

Wouldn't it make sense to have a common (perhaps abstract) base class here,
or derive one from the other? The APIs are very similar at least.


GFileAttribute:
===============

GFileAttribute* -> GIOFileAttribute*

GFileAttributeValue: why is the union not a GValue?

Our biggest issue is probably that the attributes are semantically
very similar to object properties and should therefore follow the
conventions introduced there:

Namespace should be separated from attribute name by '::', not ':', gobject
does this for both properties and detailled signals, so GIO should name
file attributes consistently.

The use of '_' should be avoided, please use '-' instead. We put a
great deal of effort into canonicalizing identifiers elsewhere (object
property names and signal names), so GIO should follow that habit here
for the sake of consistency.

Also, there is no good reason to use abbreviations like "std" and "fs",
we are not in the 60ies any more ;)

std -> standard
fs  -> filesystem

std:type      -> standard::type
std:is_hidden -> standard::is-hidden


There also seems to be quite some duplication between GFileInfo and
GFileAttributeValue for no good reason, such as:

g_file_info_get_attribute_string vs. g_file_attribute_value_get_string
g_file_info_set_attribute_int32  vs. g_file_attribute_value_set_int32
...

Where these functions in GFileInfo just seem to create/get a
GFileAttributeValue and call the corresponding API, in fact these seem
to be the only calls to g_file_attribute_value_* inside GIO.

It also seems like a good idea to provide a more generic API similar to
g_object_get in order to get multiple values at the same time:

g_io_file_info_get_attributes (GIOFileInfo *info,
                               const gchar *first_attribute_name,
                               ...)


G*Stream:
=========

GInputStream     -> GIOInputStream     (or even GIOStreamInput)
GFileInputStream -> GIOInputStreamFile (or even GIOStreamInputFile)
...


GCancellable:
=============

GCancellable -> GIOCancellable

There is no need to break the common prefix for global things:

g_push_current_cancellable -> g_cancellable_push_current
g_pop_current_cancellable  -> g_cancellable_pop_current


G*AsyncResult:
=============

GAsyncResult       -> GIOAsyncResult
GSimpleAsyncResult -> GIOAsyncResultSimple


GIOScheduler:
=============

Each function has a different namespace here, I suggest:

GIOSchedulerJobFunc
GIOSchedulerDataFund
g_io_scheduler_schedule_job
g_io_scheduler_cancel_all_jobs
g_io_scheduler_send_to_mainloop

Especially the "schedule" and "send_to_mainloop" functions are
quite similar. Doesn't the first require a running main loop too?
Either it's mis-naming or lack of documentation (or understanding on
my side ;-)


G*Icon:
=======

Same as above:

GIcon         -> GIOIcon
GFileIcon     -> GIOIconFile
GLoadableIcon -> GIOIconLoadable
GThemedIcon   -> GIOIconThemed


GContentType:
=============

Same namespace issues:

GContent* -> GIOContent*

g_content_type_guess:
  problematic since not all contents can be guessed from file headers
  (which is what this function probably does). Perhaps rather pass
  a GFile or a GInputStream so the file's end is accessible too?


GAppInfo:
=========

GAppInfo -> GIOAppInfo