After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 440431 - Common widget for choosing file formats
Common widget for choosing file formats
Status: RESOLVED WONTFIX
Product: gtk+
Classification: Platform
Component: Widget: GtkFileChooser
2.12.x
Other All
: High enhancement
: Medium API
Assigned To: gtk-bugs
Federico Mena Quintero
: 135901 (view as bug list)
Depends on:
Blocks: 379608
 
 
Reported: 2007-05-22 12:33 UTC by Mathias Hasselmann (IRC: tbf)
Modified: 2015-02-18 00:27 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
eggfileformat.h (1.87 KB, text/x-chdr)
2007-05-22 12:34 UTC, Mathias Hasselmann (IRC: tbf)
Details
eggfileformat.c (4.00 KB, text/x-csrc)
2007-05-22 12:35 UTC, Mathias Hasselmann (IRC: tbf)
Details
eggfileformatchooser.h (1.82 KB, text/x-chdr)
2007-05-22 12:35 UTC, Mathias Hasselmann (IRC: tbf)
Details
eggfileformatchooser.c (6.03 KB, text/x-csrc)
2007-05-22 12:36 UTC, Mathias Hasselmann (IRC: tbf)
Details
eggfileformatchooser.c (10.28 KB, text/x-csrc)
2007-05-26 21:43 UTC, Mathias Hasselmann (IRC: tbf)
Details
eggfileformatchooser.h (2.57 KB, text/x-chdr)
2007-05-26 21:43 UTC, Mathias Hasselmann (IRC: tbf)
Details
testfileformatchooser.c (4.43 KB, text/x-csrc)
2007-05-26 21:45 UTC, Mathias Hasselmann (IRC: tbf)
Details
Look of this approach. (52.71 KB, image/png)
2007-05-26 21:48 UTC, Mathias Hasselmann (IRC: tbf)
Details
eggfileformatchooser.c (9.96 KB, text/x-csrc)
2007-05-27 10:10 UTC, Mathias Hasselmann (IRC: tbf)
Details
eggfileformatchooser.h (2.37 KB, text/x-chdr)
2007-05-27 10:10 UTC, Mathias Hasselmann (IRC: tbf)
Details
testfileformatchooser.c (3.87 KB, text/x-csrc)
2007-05-27 10:10 UTC, Mathias Hasselmann (IRC: tbf)
Details
eggfileformatchooser.c (11.55 KB, text/x-csrc)
2007-05-27 12:02 UTC, Mathias Hasselmann (IRC: tbf)
Details
eggfileformatchooser.h (2.66 KB, text/x-chdr)
2007-05-27 12:02 UTC, Mathias Hasselmann (IRC: tbf)
Details
testfileformatchooser.c (4.85 KB, text/x-csrc)
2007-05-27 12:03 UTC, Mathias Hasselmann (IRC: tbf)
Details
eggfileformatchooser.c (17.46 KB, text/plain)
2007-05-28 18:50 UTC, Mathias Hasselmann (IRC: tbf)
Details
eggfileformatchooser.h (3.48 KB, text/plain)
2007-05-28 18:51 UTC, Mathias Hasselmann (IRC: tbf)
Details
testfileformatchooser.c (5.70 KB, text/plain)
2007-05-28 18:51 UTC, Mathias Hasselmann (IRC: tbf)
Details

Description Mathias Hasselmann (IRC: tbf) 2007-05-22 12:33:33 UTC
Applications supporting multiple file formats needs a file-format chooser in their file saving dialog. Several GNOME apps implement very similiar file-format choosers - so it makes sense to me, to add this widget to GTK+.

The widget would be used like this:

    GtkWidget *format_chooser = egg_file_format_chooser_new();
    gtk_file_chooser_set_extra_widget(GTK_FILE_CHOOSER(dialog), format_chooser);

    EggFileFormat *format;

    format = egg_file_format_new (_("Scalable Vector Graphics (SVG)"), "svg", NULL);
    egg_file_format_chooser_add_format(EGG_FILE_FORMAT_CHOOSER(format_chooser), format);

    format = egg_file_format_new (_("Compiliertes Layout (C-Header)"), "h", NULL);
    egg_file_format_chooser_add_format(EGG_FILE_FORMAT_CHOOSER(format_chooser), format);

    ...

    if (GTK_RESPONSE_ACCEPT == gtk_dialog_run (file_chooser))
      {
        EggFileFormat *format = egg_file_format_chooser_get_format (format_chooser);
        gchar *filename = gtk_file_chooser_get_filename (chooser);

        if (NULL == format)
          format = egg_file_format_chooser_guess_by_extension (format_chooser, filename);

        app_file_format_save (APP_FILE_FORMAT (format), filename);
        g_free (filename);
      }
Comment 1 Mathias Hasselmann (IRC: tbf) 2007-05-22 12:34:20 UTC
Created attachment 88596 [details]
eggfileformat.h
Comment 2 Mathias Hasselmann (IRC: tbf) 2007-05-22 12:35:09 UTC
Created attachment 88597 [details]
eggfileformat.c
Comment 3 Mathias Hasselmann (IRC: tbf) 2007-05-22 12:35:42 UTC
Created attachment 88598 [details]
eggfileformatchooser.h
Comment 4 Mathias Hasselmann (IRC: tbf) 2007-05-22 12:36:43 UTC
Created attachment 88599 [details]
eggfileformatchooser.c

Sample implementation (without egg_file_format_chooser_guess_by_extension).
Comment 5 Emmanuele Bassi (:ebassi) 2007-05-22 13:01:46 UTC
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;
Comment 6 Mathias Hasselmann (IRC: tbf) 2007-05-22 15:27:21 UTC
Screenshots and links to other implementations: http://live.gnome.org/GTK%2B/FileFormatChooser
Comment 7 Matthias Clasen 2007-05-22 15:39:59 UTC
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 ?
Comment 8 Federico Mena Quintero 2007-05-22 15:52:38 UTC
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".
Comment 9 Mathias Hasselmann (IRC: tbf) 2007-05-22 16:17:21 UTC
(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.
Comment 10 Sebastian Rittau 2007-05-25 13:37:29 UTC
> 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.
Comment 11 Matthias Clasen 2007-05-25 20:45:15 UTC
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.
Comment 12 Philip Withnall 2007-05-26 14:11:07 UTC
Bug #379608 has some work to fix niggles with Totem's implementation of a file format chooser, which might be useful in this bug.
Comment 13 Mathias Hasselmann (IRC: tbf) 2007-05-26 21:43:19 UTC
Created attachment 88865 [details]
eggfileformatchooser.c
Comment 14 Mathias Hasselmann (IRC: tbf) 2007-05-26 21:43:36 UTC
Created attachment 88866 [details]
eggfileformatchooser.h
Comment 15 Mathias Hasselmann (IRC: tbf) 2007-05-26 21:45:26 UTC
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);
Comment 16 Mathias Hasselmann (IRC: tbf) 2007-05-26 21:48:37 UTC
Created attachment 88868 [details]
Look of this approach.
Comment 17 Philip Withnall 2007-05-26 22:50:08 UTC
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?
Comment 18 Mathias Hasselmann (IRC: tbf) 2007-05-27 08:27:34 UTC
* 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.
Comment 19 Mathias Hasselmann (IRC: tbf) 2007-05-27 10:10:08 UTC
Created attachment 88875 [details]
eggfileformatchooser.c
Comment 20 Mathias Hasselmann (IRC: tbf) 2007-05-27 10:10:24 UTC
Created attachment 88876 [details]
eggfileformatchooser.h
Comment 21 Mathias Hasselmann (IRC: tbf) 2007-05-27 10:10:42 UTC
Created attachment 88877 [details]
testfileformatchooser.c
Comment 22 Mathias Hasselmann (IRC: tbf) 2007-05-27 10:20:19 UTC
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?
Comment 23 Mathias Hasselmann (IRC: tbf) 2007-05-27 12:02:17 UTC
Created attachment 88880 [details]
eggfileformatchooser.c

Forgot egg_file_format_chooser_append_extension.
Comment 24 Mathias Hasselmann (IRC: tbf) 2007-05-27 12:02:41 UTC
Created attachment 88881 [details]
eggfileformatchooser.h
Comment 25 Mathias Hasselmann (IRC: tbf) 2007-05-27 12:03:06 UTC
Created attachment 88882 [details]
testfileformatchooser.c
Comment 26 Mathias Hasselmann (IRC: tbf) 2007-05-28 18:50:55 UTC
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+.
Comment 27 Mathias Hasselmann (IRC: tbf) 2007-05-28 18:51:18 UTC
Created attachment 88955 [details]
eggfileformatchooser.h
Comment 28 Mathias Hasselmann (IRC: tbf) 2007-05-28 18:51:43 UTC
Created attachment 88956 [details]
testfileformatchooser.c
Comment 29 Mathias Hasselmann (IRC: tbf) 2007-07-31 21:05:54 UTC
Added the code to libegg: http://svn.gnome.org/viewcvs/libegg/trunk/libegg/fileformatchooser/
Comment 30 Martin Ejdestig 2007-08-02 11:25:13 UTC
Does anyone remember Seth's original design? I like that one much better than the expander.

http://www.gnome.org/~seth/designs/filechooser-spec/
Comment 31 Mathias Hasselmann (IRC: tbf) 2007-08-02 12:19:41 UTC
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.
Comment 32 Martin Ejdestig 2007-08-02 12:42:25 UTC
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?
Comment 33 Nickolay V. Shmyrev 2008-01-15 01:02:57 UTC
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.
Comment 34 Nickolay V. Shmyrev 2008-01-15 01:11:01 UTC
Well, not ugly of course, excuse me. Inconsistent I'd say
Comment 35 Mathias Hasselmann (IRC: tbf) 2008-01-15 08:33:07 UTC
(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?
Comment 36 Nickolay V. Shmyrev 2008-01-15 08:46:18 UTC
> 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.
Comment 37 Federico Mena Quintero 2008-07-29 16:00:13 UTC
*** Bug 135901 has been marked as a duplicate of this bug. ***
Comment 38 Michael Meeks 2008-08-28 14:29:50 UTC
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.
Comment 39 Matthias Clasen 2015-02-18 00:27:19 UTC
this didn't happen, time to close this request