Created attachment 88596 [details]
eggfileformat.h

Created attachment 88597 [details]
eggfileformat.c

Created attachment 88598 [details]
eggfileformatchooser.h

Created attachment 88599 [details]
eggfileformatchooser.c

Sample implementation (without egg_file_format_chooser_guess_by_extension).

short review on the EggFileFormat implementation:

* I would really like this to be a GtkFileFilter by adding a gtk_file_filter_add_extensions();

* there are many self->priv->whatever indirections; you should shave them off using an intermediate variable, and let the compiler optimise them;

* no need to check if the parent class has a dispose method: it's GObject, we know for sure it has it, and by removing it you save a type check;

* why using a GValueArray? aren't all extensions just strings? a GPtrArray would seem more appropriate; a gchar** would be even simpler;

Screenshots and links to other implementations: http://live.gnome.org/GTK%2B/FileFormatChooser

Just echoing Emmanueles comments: this should be thought through in the context 
of file filtering in the file open dialog, and it shouldn't use a value array.

Is there a strong case for exposing formats as objects here ? We could just go:
file_format_chooser_add_format (const char *name, const char *ext1, ...);

Does the format chooser handle formats without extension ok ?

Is there a need to set a default format ?

See the discussion in bug #135901 (you may want to make that one a dupe of this
one).

There's also some discussion in http://live.gnome.org/GtkFileChooser - grep for
"drop-down with types".

(In reply to comment #5)
> short review on the EggFileFormat implementation:
> 
> * I would really like this to be a GtkFileFilter by adding a
> gtk_file_filter_add_extensions();

Enhancing GtkFileFilter conflicts with my plan to implementing loading/saving by extending GtkFileFormat:

    app_file_format_save (APP_FILE_FORMAT (format), filename);

An AppFileFormat class extending GtkFileFilter doesn't feel natural for me.

> * there are many self->priv->whatever indirections; you should shave them off
> using an intermediate variable, and let the compiler optimise them;

None of the indirections happens at performance critical points, so I've choosen to waste some CPU cycles over cluttering the code with intermediate variables. 

> * why using a GValueArray? aren't all extensions just strings? a GPtrArray
> would seem more appropriate; a gchar** would be even simpler;

Initially I had gchar** for extensions - but I really do not know how to wrap it within a property. So I need the GValueArray anyway. Rewriting my test-code from gchar** to GValueArray didn't add one single line of code to the test program. So I decided to avoid needless array allocations and directly expose the internal GValueArray.

(In reply to comment #7)
> Is there a strong case for exposing formats as objects here ? We could just go:
> file_format_chooser_add_format (const char *name, const char *ext1, ...);

As written above: It allows and encourages implementing file formats as objects:

    struct AppFileFormatClass
    {
      GtkFileFormatClass parent;

      gboolean (*save)(AppFileFormat *self,
                       AppDataModel  *model,
                       GError       **error);
    };

Beside that case of (good coding practice?) there is no reason for exposing a GtkFileFormat object.

> As written above: It allows and encourages implementing file formats as
objects:

I don't really see how? I agree that implementing file formats as objects is a good coding practice in general, but how does the API encourage that?

The current API is inconvenient to use, since it needs more boiler-plate code than necessary (two calls). It is not as decoupled as it could be, since the API user needs to know about "GtkFileFormat"s. Please make the API as simple and convenient to use as possible.

Also the allocated formats are never freed (I guess the widget gains ownership of the object reference), which is inconsistent with most modern glib-/gtk-based APIs.

I fail to see the attractiveness of implementing file formats like that.
 
In most cases, it will only cause extra overhead and require people to 
invent glue layers to put this GtkFileFormat structure on top of their
preexisting file format infrastructure. 

And the win of being able to use the objects as input to a file format 
selector in the file chooser is very marginal.

Bug #379608 has some work to fix niggles with Totem's implementation of a file format chooser, which might be useful in this bug.

Created attachment 88865 [details]
eggfileformatchooser.c

Created attachment 88866 [details]
eggfileformatchooser.h

Created attachment 88867 [details]
testfileformatchooser.c

Does this API has more fans?

GType           egg_file_format_chooser_get_type       (void) G_GNUC_CONST;
GtkWidget*      egg_file_format_chooser_new            (void);

void            egg_file_format_chooser_add_format     (EggFileFormatChooser *self,
                                                        GdkPixbuf            *icon,
                                                        const gchar          *name,
                                                        ...) G_GNUC_NULL_TERMINATED;
void            egg_file_format_chooser_add_sub_format (EggFileFormatChooser *self,
                                                        GdkPixbuf            *icon,
                                                        const gchar          *name,
                                                        ...) G_GNUC_NULL_TERMINATED;

void            egg_file_format_chooser_set_format     (EggFileFormatChooser *self,
                                                        gint                  format,
                                                        gint                  subformat);
gint            egg_file_format_chooser_get_format     (EggFileFormatChooser *self,
                                                        const gchar          *filename,
                                                        gint                 *subformat);

Usage Example:

  egg_file_format_chooser_add_format (chooser, html,
                                      "HTML Document", NULL);
  egg_file_format_chooser_add_sub_format (chooser, html,
                                          "Full HTML Document",
                                          ".html", ".htm", NULL);
  egg_file_format_chooser_add_sub_format (chooser, html,
                                          "HTML Document Fragment",
                                          ".html", ".htm", NULL);

Created attachment 88868 [details]
Look of this approach.

It might be an idea to allow the parent format of a sub-format to be specified when adding one, although I can't think of many use-cases where you'd be adding formats anywhere other than creation of the widget, so you could probably ignore this. :-P

What about returning a the ID of a format when you add one, so that you can use it later with egg_file_format_chooser_[get|set]_format?

* Adding the index of the parent format when creating a subformat is easy right now.
* Initially I also had the idea to use a surrogate key (ID) for identifying formats, but it turned out being a mess to find them again in the tree-view model. On the other hand: There is gtk_tree_model_foreach. Using it also would cleanup the file-type guessing code. Ok, back on the workbench.

Created attachment 88875 [details]
eggfileformatchooser.c

Created attachment 88876 [details]
eggfileformatchooser.h

Created attachment 88877 [details]
testfileformatchooser.c

Changes of the latest version:

- Introduce a surrogate ID. This gives a much nicer API:

    guint egg_file_format_chooser_add_format (EggFileFormatChooser *self,
                                              guint                 parent,
                                              const gchar          *name,
                                              const gchar          *icon,
                                              ...) G_GNUC_NULL_TERMINATED;

- Implement egg_file_format_chooser_remove_format.
- Implement a "selection-changed" signal.

Still missing:

- Call for adding extensions to filename.

To discuss:

- Should the widget optionally contain the frequently requested "Append file extension" checkbox?
- Should egg_file_format_chooser_add_format provide a "user_data" argument for injecting additional, application specific data into the model?

Created attachment 88880 [details]
eggfileformatchooser.c

Forgot egg_file_format_chooser_append_extension.

Created attachment 88881 [details]
eggfileformatchooser.h

Created attachment 88882 [details]
testfileformatchooser.c

Created attachment 88954 [details]
eggfileformatchooser.c

Added support for saveable GdkPixbufFormats and for attaching custom data to file formats. Federico's checklist is finished, so I guess its time to consider moving this to the SVN repository. Either libegg or directly gtk+.

Created attachment 88955 [details]
eggfileformatchooser.h

Created attachment 88956 [details]
testfileformatchooser.c

Added the code to libegg: http://svn.gnome.org/viewcvs/libegg/trunk/libegg/fileformatchooser/

Does anyone remember Seth's original design? I like that one much better than the expander.

http://www.gnome.org/~seth/designs/filechooser-spec/

Didn't remember that design and have to admit it looks cute, but it has problems. So for instance I would not know how to use that approach in Gnumeric or OpenOffice.org where you have to support several subvariants of the same base format - like for instance Excel spreadsheets. I try to address this isssue with some file type hierarchy. Also Seth's approach doesn't leave much space for descriptive file type names. Already the various OpenOffice.org file types would not fit into that button - well and I doubt we help our users with using cryptical file type names like "ODF Document". - "WTF! is ODF?"

Another obvious choice would be using a combobox, but personally I do not feel comfortable with putting some hierachy into a combobox popup. Also having a popup would not work for applications like the GIMP supporting myriads of file types. Guess current list of GIMP supported file types takes more than two screen pages!

What I'd really like to change: Maybe the file type selection widget can be moved above the file browser - but that's not possible with the API GtkFileChooser currently provides.

Most users don't grasp hierarchies very well so it would be good if they could be avoided.

Descriptive file type names? Maybe they could be put in drop down somehow?

Also, is there really a need for a hierarchial list in most cases? I mean, most apps don't have tons of different file types so designing primarily with this case in mind probably isn't the way to go. Maybe some special design can be done for apps like the GIMP. Don't know.

As for the location of the widget. Why does this have to be a completely different widget that is added by way of the normal "embed widget" interface of the file chooser? Sure, that's how most apps do it now because there is no such thing in the file chooser itself.

I don't know how Seth imagined the drop down to look. Maybe we should ask him. Is he still around?

I also wondering about ugly UI of current state of format selector. Is there any sense to duplicate information in combobox and listview? It also doesn't fit into widget dimensions well. I've just removed fileformatchooser from evince for this reason.

Well, not ugly of course, excuse me. Inconsistent I'd say

(In reply to comment #33)
> I also wondering about ugly UI of current state of format selector. Is there
> any sense to duplicate information in combobox and listview? It also doesn't
> fit into widget dimensions well. I've just removed fileformatchooser from
> evince for this reason.
> 

Well, both widgets have a different purpose: One filters the list on fuzzy criterions like "All Supported Formats", "Image Files", "Web Pages", ... The file format chooser tries to solve the problem, of actually choosing a concrete file format.

The guess-from-extension and filter selection approach doesn't work for non-technical users, I believe. There was some OpenOffice based usability study regarding that topic.

Also the filter combo box is hidden, when you collapse the file browser. Very unintuitive that you have to execute the task "browse for files", when you want to "choose the file format". I've fallen accross this usability barrier by myself several times.

Still I recognize the issue you have. So what can be done?
Maybe there should be no file filters, when a format chooser is attached?

> Also the filter combo box is hidden, when you collapse the file browser. Very
unintuitive that you have to execute the task "browse for files", when you want
to "choose the file format". I've fallen accross this usability barrier by
myself several times.

Sound reasonable, thanks for explanation.

I think you can at least remove filter without much harm. And I agree with Martin that original Seth's proposal with dropdown was probably better. 

Btw, there is an issue with icons too. Nowadays we have a very limited set of document icons so with very high probability icon will be the same for all formats supported by application, for all pixbuf formats for example.

*** Bug 135901 has been marked as a duplicate of this bug. ***

My 2 cents from OpenOffice; is that we need an "All Files" alias - for when we're browsing files to open - and in OO.o we use a type of "*.*" which we map to a filter of "*" - not sure if that's easy with the new filter model.

Really - it's best to just read the OO.o source in this area - we don't access the icons currently, but that's easy enough to fix, and add I guess; a good place to start is here:

http://lxr.go-oo.org/source/gsl/fpicker/source/unx/gnome/SalGtkFilePicker.cxx#1865

We have 'groups' of filters exposed to us, but discard these as we populate the existing list; it -might- be nice to show those as italic/separated headings somehow in the drop-down [ if possible ], but we can live without.

this didn't happen, time to close this request