GNOME Bugzilla – Bug 341622
DVD burning fails in presence of invalid encoded file(s)
Last modified: 2006-12-04 17:21:43 UTC
When you write DVDs on the fly, the presence of an invalid encoded filename makes the process fail. The disc is written until that file, but n-c-b doesn't signal the error so, you can burn your discs without some data and don't notice that at all.
Created attachment 68632 [details] [review] Patch Preliminary patch that adds a user friendly rename dialog, the message shown needs some love... Now fixing bug 323506 should be really easy as well.
Created attachment 68672 [details] [review] Right patch
Jon?
Hi :) Sorry for the delay. This can only go in after branching (due to ui/feature changes) so I'll take a good look at this as soon as we do... which should be in the next week or two. Thanks.
Hey Fabio. What's the best way to create test data that will trigger this? Can you attach a small test set as a tarball or something? Thanks.
Created attachment 75395 [details] Program Hi Jon, this program creates a file with invalid encoding in its name.
Comment on attachment 68672 [details] [review] Right patch Looks very good. A few minors comments. >+ * >+ * Authors: Fabio Bonelli <fabiobonelli@libero.it> */ Put the end of the comment on a new line. Besides consistency one of the reasons for this is it allows for modification of the line or adding new authors without messing with CVS history. >+#include "config.h" >+ >+#include <string.h> >+ >+#include <gdk/gdk.h> >+ >+#include <glib.h> >+#include <glib/gi18n.h> >+#include <gtk/gtk.h> We try to keep the includes in order from low in the stack to high. So, gdk should come after glib. But in this case gtk already includes gdk so we don't need it explicitly. >+#include <glade/glade.h> >+#include <libgnomevfs/gnome-vfs.h> >+#include <libgnomeui/libgnomeui.h> I see that this pulls in libgnomeui for the mime-type stuff. I'd rather not do this. Is there another way? >+#include "ncb-rename-dialog.h" >+ >+static void ncb_rename_dialog_class_init (NcbRenameDialogClass *klass); >+static void ncb_rename_dialog_init (NcbRenameDialog *dialog); >+static void ncb_rename_dialog_finalize (GObject *object); >+ >+#define NCB_RENAME_DIALOG_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NCB_TYPE_RENAME_DIALOG, NcbRenameDialogPrivate)) >+ >+struct NcbRenameDialogInvalidFile { >+ char *mime_type; >+ char *filename; >+}; >+typedef struct NcbRenameDialogInvalidFile NcbRenameDialogInvalidFile; >+ >+struct NcbRenameDialogPrivate >+{ >+ /* View and model for the list >+ * of files */ >+ GtkTreeView *view; >+ GtkListStore *store; >+ >+ /* Contextual popup menu */ >+ GtkWidget *menu; >+ GtkWidget *rename_item; >+ >+ /* List of NcbRenameDialogInvalidFile invalid filenames */ >+ GSList *invalid_files; >+ >+ /* Root path, used by gnome_vfs_directory_visit() */ >+ char *root_path; >+}; >+ >+/* List store columns */ >+enum { >+ NCB_TREESTORE_PIXBUF, >+ NCB_TREESTORE_OLDNAME, >+ NCB_TREESTORE_NEWNAME >+}; >+ >+G_DEFINE_TYPE (NcbRenameDialog, ncb_rename_dialog, GTK_TYPE_MESSAGE_DIALOG) >+ >+static void >+ncb_rename_dialog_class_init (NcbRenameDialogClass *klass) >+{ >+ GObjectClass *object_class = G_OBJECT_CLASS (klass); >+ >+ object_class->finalize = ncb_rename_dialog_finalize; >+ >+ g_type_class_add_private (klass, sizeof (NcbRenameDialogPrivate)); >+} >+ >+/* treemodel_file_move_cb: >+ * >+ * Callback for gtk_tree_model_foreach (). >+ * Takes the row referred by @iter in the invalid files' model and >+ * moves the file in burn:// associated with it to its valid name. */ >+static gboolean >+treemodel_file_move_cb (GtkTreeModel *model, >+ GtkTreePath *tree_path, >+ GtkTreeIter *iter, >+ gpointer data) >+{ >+ char *source_uri = NULL; >+ char *dest_uri; >+ char *path_uri; >+ char *shortname = NULL; >+ NcbRenameDialog *dialog; >+ GnomeVFSResult res; >+ >+ dialog = (NcbRenameDialog *) data; >+ >+ gtk_tree_model_get (GTK_TREE_MODEL (dialog->priv->store), iter, >+ NCB_TREESTORE_OLDNAME, &source_uri, >+ NCB_TREESTORE_NEWNAME, &shortname, -1); >+ >+ /* Build @dest_uri with a full path */ >+ path_uri = g_path_get_dirname (source_uri); >+ dest_uri = g_strdup_printf ("%s/%s", path_uri, shortname); >+ g_free (shortname); >+ g_free (path_uri); >+ >+ res = gnome_vfs_move (source_uri, dest_uri, FALSE); >+ if (res != GNOME_VFS_OK) { >+ g_warning ("Cannot move `%s' to `%s': %s", source_uri, dest_uri, >+ gnome_vfs_result_to_string (res)); >+ } >+ >+ g_free (source_uri); >+ g_free (dest_uri); >+ >+ /* Done, but call me again for other rows.*/ >+ return FALSE; >+} >+ >+ >+/* ncb_rename_dialog_response_cb: >+ * >+ * Convenience response callback for #NcbRenameDialog. >+ * Once called it renames the files in burn:// if the response >+ * is a GTK_RESPONSE_OK. */ >+void >+ncb_rename_dialog_response_cb (NcbRenameDialog *dialog, >+ int id, >+ gpointer user_data) >+{ >+ g_return_if_fail (dialog != NULL); >+ g_return_if_fail (NCB_IS_RENAME_DIALOG (dialog)); >+ >+ if (id == GTK_RESPONSE_OK) { >+ gtk_tree_model_foreach (GTK_TREE_MODEL (dialog->priv->store), >+ treemodel_file_move_cb, dialog); >+ } >+} >+ >+ >+/* cell_editing_done_cb: >+ * >+ * Callback for the "edited" signal emitted by the text >+ * renderer. >+ * It updates the name in the treemodel with the name typed in. */ >+static void >+cell_editing_done_cb (GtkCellRendererText *renderer, >+ gchar *tree_path, >+ gchar *new_string, >+ gpointer data) >+{ >+ NcbRenameDialog *dialog; >+ GtkTreeIter iter; >+ int r; >+ >+ g_return_if_fail (strlen (new_string) != 0); >+ >+ dialog = (NcbRenameDialog *) data; >+ >+ r = gtk_tree_model_get_iter_from_string (GTK_TREE_MODEL (dialog->priv->store), >+ &iter, tree_path); >+ g_return_if_fail (r); >+ >+ gtk_list_store_set (dialog->priv->store, &iter, >+ NCB_TREESTORE_NEWNAME, new_string, -1); >+} >+ >+/* rename_menu_item_cb: >+ * >+ * Callback for the "activate" signal emitted by the rename popup menu item. >+ * Updates the desired new name for the files represented by >+ * the selected row. We disable that menu item when multiple rows are >+ * selected. */ >+static void >+rename_menu_item_cb (GtkWidget *widget, >+ gpointer user_data) >+{ >+ NcbRenameDialog *dialog; >+ GtkTreeSelection *selection; >+ GtkListStore *store; >+ GtkTreeViewColumn *column; >+ GtkTreePath *path; >+ GList *list; >+ >+ dialog = (NcbRenameDialog *) user_data; >+ >+ store = GTK_LIST_STORE (dialog->priv->store); >+ >+ selection = gtk_tree_view_get_selection (dialog->priv->view); >+ >+ list = gtk_tree_selection_get_selected_rows (selection, NULL); >+ if (g_list_length (list) != 1) { >+ >+ /* This shouldn't happen. Rename makes no sense if more than >+ * one item is selected. */ >+ g_warning ("More than one row selected during a rename?"); >+ } >+ >+ path = (GtkTreePath *) list->data; >+ >+ column = gtk_tree_view_get_column (dialog->priv->view, 0); >+ g_return_if_fail (column != NULL); Generally you only use g_return_if_fail to check input into a public function. If this is just error checking then you should use "if (FALSE) { g_warning(); return; }" if it only occurs when there is a programming error then use g_assert(). >+ /* Start editing the cell */ >+ gtk_tree_view_set_cursor (dialog->priv->view, >+ path, column, TRUE); >+ >+ g_list_foreach (list, (GFunc)gtk_tree_path_free, NULL); >+ g_list_free (list); >+} >+ >+/* delete_menu_item_cb: >+ * >+ * Callback for the "activate" signal emitted by the delete popup menu item. >+ * Removes the file represented by the selected row(s) from burn://. */ >+static void >+delete_menu_item_cb (GtkWidget *widget, >+ gpointer user_data) >+{ >+ NcbRenameDialog *dialog; >+ GtkTreeSelection *selection; >+ GtkTreeModel *model; >+ GList *list; >+ GList *row_references = NULL; >+ GList *tmp; >+ >+ dialog = (NcbRenameDialog *) user_data; >+ >+ model = GTK_TREE_MODEL (dialog->priv->store); >+ >+ selection = gtk_tree_view_get_selection (dialog->priv->view); >+ >+ list = gtk_tree_selection_get_selected_rows (selection, NULL); >+ >+ /* Build a GtkTreeRowReference list out of the GtkTreePath list 'cause >+ * we are going to modify the tree */ >+ for (tmp = list; tmp; tmp = g_list_next (tmp)) { >+ GtkTreeRowReference *reference; >+ >+ reference = gtk_tree_row_reference_new (model, (GtkTreePath *)tmp->data); >+ >+ row_references = g_list_append (row_references, reference); >+ } >+ g_list_foreach (list, (GFunc)gtk_tree_path_free, NULL); >+ g_list_free (list); >+ >+ >+ /* Remove the selected rows and relative files */ >+ for (tmp = row_references; tmp; tmp = g_list_next (tmp)) { >+ GtkTreePath *path; >+ GtkTreeIter iter; >+ >+ path = gtk_tree_row_reference_get_path ((GtkTreeRowReference *)tmp->data); >+ >+ if (gtk_tree_model_get_iter (model, &iter, path)) { >+ char *name = NULL; >+ GnomeVFSResult res; >+ >+ gtk_tree_model_get (model, &iter, >+ NCB_TREESTORE_OLDNAME, &name, -1); Once the line is split somewhat arbitrarily I like to split it completely with an arg per line. >+ res = gnome_vfs_unlink (name); >+ if (res == GNOME_VFS_OK) { >+ gtk_list_store_remove (GTK_LIST_STORE (model), &iter); >+ } else { >+ g_warning ("Cannot remove %s (%s)", name, >+ gnome_vfs_result_to_string (res)); >+ } >+ g_free (name); >+ } >+ } >+ g_list_foreach (row_references, (GFunc)gtk_tree_row_reference_free, NULL); >+ g_list_free (row_references); >+} >+ >+/* show_popup_menu: >+ * >+ * @dialog: the #NcbRenameDialog. >+ * @event: #GdkEventButton that triggered the dialog, >+ * it can be NULL. >+ * @show_rename: wheter to show the rename menu item. >+ * >+ * Shows the contextual menu. */ >+static void >+show_popup_menu (NcbRenameDialog *dialog, >+ GdkEventButton *event, >+ gboolean show_rename) >+{ >+ GtkWidget *menu; >+ GtkWidget *rename_item; >+ >+ menu = dialog->priv->menu; >+ rename_item = dialog->priv->rename_item; >+ >+ gtk_widget_set_sensitive (rename_item, show_rename); >+ >+ gtk_widget_show_all (menu); >+ >+ gtk_menu_popup (GTK_MENU (menu), NULL, NULL, NULL, NULL, >+ (event != NULL) ? event->button : 0, >+ gdk_event_get_time ((GdkEvent *)event)); >+} >+ >+/* treeview_button_pressed_cb: >+ * >+ * Callback for the "button-press-event" signal emitted by the treeview. >+ * On right click, it selects the row under the cursor if no one is >+ * selected and pops up the contextual menu. */ >+static gboolean >+treeview_button_pressed_cb (GtkWidget *treeview, >+ GdkEventButton *event, >+ gpointer user_data) >+{ >+ GtkTreeSelection *selection; >+ NcbRenameDialog *dialog; Spacing issue here. >+ if (event->type == GDK_BUTTON_PRESS && event->button == 3) { >+ int rows; >+ >+ selection = gtk_tree_view_get_selection (GTK_TREE_VIEW (treeview)); >+ >+ /* Select the row under the cursor if there is no other row selected >+ * or if there is another one selected */ >+ rows = gtk_tree_selection_count_selected_rows (selection); >+ if (rows <= 1) { >+ GtkTreePath *path; >+ >+ /* Get tree path for row that was clicked */ >+ if (gtk_tree_view_get_path_at_pos (GTK_TREE_VIEW (treeview), >+ (gint) event->x, >+ (gint) event->y, >+ &path, NULL, NULL, NULL)) { >+ >+ gtk_tree_selection_unselect_all (selection); >+ gtk_tree_selection_select_path (selection, path); >+ gtk_tree_path_free (path); >+ >+ rows = 1; >+ } >+ } >+ >+ dialog = (NcbRenameDialog *) user_data; >+ >+ /* Show the popup menu and show the "rename" menu item only >+ * if we selected just a single row */ >+ show_popup_menu (dialog, event, (rows == 1)); >+ >+ return TRUE; >+ } >+ >+ return FALSE; >+} >+ >+/* treeview_popup_menu_cb: >+ * >+ * Callback for the "popup-menu" signal. >+ * Pops up the contextual menu. */ >+static void >+treeview_popup_menu_cb (GtkWidget *widget, >+ gpointer user_data) >+{ >+ NcbRenameDialog *dialog; >+ >+ dialog = (NcbRenameDialog *) user_data; >+ >+ show_popup_menu (dialog, NULL, TRUE); >+} >+ >+static void >+ncb_rename_dialog_init (NcbRenameDialog *dialog) >+{ >+ GtkListStore *store; >+ GtkWidget *view; >+ GtkWidget *scrolled; >+ GtkTreeSelection *selection; >+ GtkCellRenderer *renderer; >+ GtkCellRenderer *pixbuf_renderer; >+ GtkTreeViewColumn *newname; >+ GtkWidget *menu; >+ GtkWidget *rename_item; >+ GtkWidget *delete_item; >+ char *primary_text; >+ char *primary_markup; >+ >+ dialog->priv = NCB_RENAME_DIALOG_GET_PRIVATE (dialog); Spacing. >+ gtk_window_set_resizable (GTK_WINDOW (dialog), TRUE); >+ >+ /** Set primary and secondary labels **/ >+ primary_text = _("Some files have incorrect filenames"); Since this text is const it is probably ok to just put it as an arg to printf (on a separate line) or alternatively the variable should be marked as const. Since you are adding strings for translation the file should be included in po/POTFILES.in I think it should say "invalid" rather than "incorrect". I think it is better to put this all in a glade file instead of having it all in the code. Can you give that a try? >+ primary_markup = g_strdup_printf ("<span size='larger' weight='bold'>%s</span>", primary_text); >+ gtk_message_dialog_set_markup (GTK_MESSAGE_DIALOG (dialog), >+ primary_markup); >+ g_free (primary_markup); >+ >+ gtk_message_dialog_format_secondary_text (GTK_MESSAGE_DIALOG (dialog), >+ _("The following filenames can't be used, click to " >+ "rename them:")); >+ >+ /** Set message type and buttons **/ >+ g_object_set (G_OBJECT (dialog), >+ "message-type", GTK_MESSAGE_WARNING, >+ "buttons", GTK_BUTTONS_OK_CANCEL, >+ NULL); >+ gtk_dialog_set_default_response (GTK_DIALOG (dialog), GTK_BUTTONS_CANCEL); >+ >+ /** Set list store and tree view **/ >+ store = gtk_list_store_new (3, G_TYPE_OBJECT, G_TYPE_STRING, G_TYPE_STRING); >+ view = gtk_tree_view_new_with_model (GTK_TREE_MODEL (store)); >+ g_signal_connect (G_OBJECT (view), "button-press-event", >+ (GCallback) treeview_button_pressed_cb, dialog); >+ g_signal_connect (G_OBJECT (view), "popup-menu", >+ (GCallback) treeview_popup_menu_cb, dialog); >+ >+ /* Setup selection mode */ >+ selection = gtk_tree_view_get_selection (GTK_TREE_VIEW (view)); >+ gtk_tree_selection_set_mode (selection, GTK_SELECTION_MULTIPLE); >+ >+ /* Icon renderer */ >+ pixbuf_renderer = gtk_cell_renderer_pixbuf_new (); >+ >+ /* Filename renderer */ >+ renderer = gtk_cell_renderer_text_new (); >+ g_object_set (G_OBJECT (renderer), "editable", TRUE, >+ NULL); Spacing. >+ g_signal_connect (G_OBJECT (renderer), "edited", >+ (GCallback) cell_editing_done_cb, dialog); >+ >+ /* Column */ >+ newname = gtk_tree_view_column_new (); >+ gtk_tree_view_column_set_title (newname, _("Filename")); >+ >+ gtk_tree_view_column_pack_start (newname, pixbuf_renderer, FALSE); >+ gtk_tree_view_column_set_attributes (newname, pixbuf_renderer, >+ "pixbuf", NCB_TREESTORE_PIXBUF, NULL); >+ gtk_tree_view_column_pack_start (newname, renderer, FALSE); >+ gtk_tree_view_column_set_attributes (newname, renderer, >+ "text", NCB_TREESTORE_NEWNAME, NULL); >+ >+ /* Append column to the view */ >+ gtk_tree_view_append_column (GTK_TREE_VIEW (view), newname); Spacing. >+ /** Scrolled window **/ >+ scrolled = gtk_scrolled_window_new (NULL, NULL); >+ gtk_scrolled_window_set_policy (GTK_SCROLLED_WINDOW (scrolled), >+ GTK_POLICY_AUTOMATIC, >+ GTK_POLICY_AUTOMATIC); >+ gtk_container_add (GTK_CONTAINER (scrolled), view); >+ >+ /* Set scrolled window's size */ >+ gtk_widget_set_size_request (scrolled, -1, 150); >+ >+ gtk_box_pack_start (GTK_BOX (GTK_DIALOG (dialog)->vbox), >+ scrolled, >+ TRUE, TRUE, 0); A small HIG thing: the box that the list is in should be aligned with the right side of the buttons at the bottom. >+ /** Set popup menu **/ >+ menu = gtk_menu_new (); >+ >+ rename_item = gtk_menu_item_new_with_label (_("Rename file")); >+ g_signal_connect (rename_item, "activate", >+ (GCallback) rename_menu_item_cb, dialog); >+ >+ delete_item = gtk_image_menu_item_new_from_stock (GTK_STOCK_REMOVE, >+ gtk_accel_group_new ()); >+ g_signal_connect (delete_item, "activate", >+ (GCallback) delete_menu_item_cb, dialog); >+ >+ gtk_menu_shell_append (GTK_MENU_SHELL (menu), rename_item); >+ gtk_menu_shell_append (GTK_MENU_SHELL (menu), delete_item); >+ >+ dialog->priv->store = store; >+ dialog->priv->view = GTK_TREE_VIEW (view); >+ dialog->priv->menu = menu; >+ dialog->priv->rename_item = rename_item; >+ >+ gtk_widget_show_all (GTK_WIDGET (GTK_DIALOG (dialog)->vbox)); >+} >+ >+/* make_valid_filename: >+ * >+ * @name: the string containing an invalid filename. >+ * >+ * Return a filename that can be written to a UTF-8 ISO9660 >+ * filesystem. >+ * Adapted from gnome_vfs_make_valid_utf8(). >+ * >+ * Return value: a newly allocated string with a valid >+ * filename based on @name. Free with g_free (). */ >+static char * >+make_valid_filename (const char *name) >+{ >+ GString *string; >+ const char *remainder, *invalid; >+ int remaining_bytes, valid_bytes; Put all declarations on separate lines and align them. >+ string = NULL; >+ remainder = name; >+ remaining_bytes = strlen (name); >+ >+ while (remaining_bytes != 0) { >+ if (g_utf8_validate (remainder, remaining_bytes, &invalid)) >+ break; These days, I like to use braces in all cases. >+ valid_bytes = invalid - remainder; >+ >+ if (string == NULL) { >+ string = g_string_sized_new (remaining_bytes); >+ } Spacing on the brace. >+ g_string_append_len (string, remainder, valid_bytes); >+ /* Use the Unicode replacement character (<?>) as substitute >+ * for invalid chars. */ >+ g_string_append_unichar (string, 0xFFFD); Spacing on the comment. >+ remaining_bytes -= valid_bytes + 1; >+ remainder = invalid + 1; >+ } >+ >+ if (string == NULL) { >+ return g_strdup (name); >+ } Spacing. >+ string = g_string_append (string, remainder); >+ g_assert (g_utf8_validate (string->str, -1, NULL)); >+ >+ return g_string_free (string, FALSE); >+} >+ >+/* check_filename_cb: >+ * >+ * Callback for gnome_vfs_directory_visit(). >+ * Checks each file and adds it to the @dialog->priv->invalid_files list >+ * if the filename is not encoded properly. */ >+static gboolean >+check_filename_cb (const gchar *rel_path, >+ GnomeVFSFileInfo *info, >+ gboolean will_loop, >+ gpointer data, >+ gboolean *recurse) Just use char and not gchar. Possible spacing issue. >+{ >+ NcbRenameDialog *dialog; >+ >+ dialog = (NcbRenameDialog *) data; >+ >+ if (! g_utf8_validate (info->name, -1, NULL)) { >+ NcbRenameDialogInvalidFile *invalid_file; >+ >+ invalid_file = g_new0 (NcbRenameDialogInvalidFile, 1); >+ invalid_file->filename = g_strdup_printf ("%s/%s", >+ dialog->priv->root_path, rel_path); >+ invalid_file->mime_type = g_strdup (info->mime_type); >+ >+ dialog->priv->invalid_files = g_slist_prepend (dialog->priv->invalid_files, >+ invalid_file); >+ } >+ >+ *recurse = (info->type == GNOME_VFS_FILE_TYPE_DIRECTORY && ! will_loop); >+ >+ /* Call me again. */ >+ return TRUE; >+} >+ >+/* pixmap_from_mime_type: >+ * >+ * @mime_type: the MIME type >+ * >+ * Get the icon representing a certain MIME type. >+ * >+ * Return value: #GdkPixbuf icon or NULL. Unref it with g_object_unref(). */ End the comment on a new line. >+static const GdkPixbuf * >+pixmap_from_mime_type (const char *mime_type) >+{ >+ GtkIconTheme *theme; >+ GdkPixbuf *pixbuf; >+ char *icon; Spacing. >+ g_return_val_if_fail (mime_type != NULL, NULL); >+ >+ theme = gtk_icon_theme_get_default (); >+ >+ icon = gnome_icon_lookup (theme, NULL, NULL, NULL, NULL, >+ mime_type, >+ GNOME_ICON_LOOKUP_FLAGS_NONE, NULL); >+ >+ pixbuf = gtk_icon_theme_load_icon (theme, icon, >+ GTK_ICON_SIZE_SMALL_TOOLBAR, 0, NULL); >+ >+ g_free (icon); >+ >+ return pixbuf; >+} >+ Spacing. >+/* add_to_liststore: >+ * >+ * @data: gpointer to a #NcbRenameDialogInvalidFile. >+ * @user_data: gpointer to the #NcbRenameDialog. >+ * >+ * Adds a #NcbRenameDialogInvalidFile to the #NcbRenameDialog's >+ * list store. */ >+static void >+add_to_liststore (gpointer data, >+ gpointer user_data) >+{ >+ GtkTreeIter iter; >+ NcbRenameDialogInvalidFile *invalid_file; >+ char *invalid_basename; >+ NcbRenameDialog *dialog; >+ const GdkPixbuf *pixbuf; >+ char *newname; >+ >+ invalid_file = (NcbRenameDialogInvalidFile *) data; >+ dialog = (NcbRenameDialog *) user_data; >+ >+ /* Use just the basename for the suggested filenames list */ >+ invalid_basename = g_path_get_basename (invalid_file->filename); >+ newname = make_valid_filename (invalid_basename); >+ g_free (invalid_basename); >+ >+ pixbuf = pixmap_from_mime_type (invalid_file->mime_type); >+ >+ gtk_list_store_append (dialog->priv->store, &iter); >+ gtk_list_store_set (dialog->priv->store, &iter, >+ NCB_TREESTORE_PIXBUF, pixbuf, >+ NCB_TREESTORE_OLDNAME, invalid_file->filename, >+ NCB_TREESTORE_NEWNAME, newname, -1); >+ >+ g_free (newname); >+} >+ >+/* ncb_rename_dialog_list_free: >+ * >+ * dialog: the #NcbRenameDialog. >+ * >+ * Free the @dialog list of invalid files. */ Don't really need docs on the private functions. But when you do try to follow the gtk-doc style that we use in the other files (say nautilus-burn-drive.c). >+static void >+ncb_rename_dialog_list_free (NcbRenameDialog *dialog) >+{ >+ GSList *list; >+ >+ list = dialog->priv->invalid_files; >+ for (; list; list = g_slist_next (list)) { Probably good to initialize list in the for statement for symmetry. Can appreviate list as "l" if you want to save space (since it is temporary). >+ NcbRenameDialogInvalidFile *invalid; >+ >+ invalid = (NcbRenameDialogInvalidFile *) list->data; >+ >+ g_free (invalid->mime_type); >+ g_free (invalid->filename); >+ g_free (invalid); >+ } >+ g_slist_free (dialog->priv->invalid_files); >+ >+ dialog->priv->invalid_files = NULL; >+} >+ >+/* burn_folder_get_invalid_filenames: >+ * >+ * Scan the burn folder for invalid filenames (incorrectly encoded, name too long, etc.) >+ * >+ * Returns: a #GHashTable of invalid filenames in the burn folder where >+ * keys are invalid filenames and >+ * values are suggested valid filenames >+ * or NULL if there aren't invalid filenames */ Use the doc style that we use in other places. >+gboolean >+ncb_rename_dialog_set_invalid_filenames (NcbRenameDialog *dialog, >+ const char *uri) >+{ >+ GnomeVFSResult res; >+ gboolean found; >+ >+ g_return_val_if_fail (dialog != NULL, FALSE); >+ g_return_val_if_fail (NCB_IS_RENAME_DIALOG (dialog), FALSE); >+ g_return_val_if_fail (uri != NULL, FALSE); >+ >+ gtk_list_store_clear (dialog->priv->store); >+ >+ dialog->priv->root_path = g_strdup (uri); >+ >+ res = gnome_vfs_directory_visit (uri, >+ GNOME_VFS_FILE_INFO_GET_MIME_TYPE|GNOME_VFS_FILE_INFO_FORCE_FAST_MIME_TYPE, >+ GNOME_VFS_DIRECTORY_VISIT_LOOPCHECK, >+ check_filename_cb, dialog); Need spaces around the "|" and should probably be broken onto separate lines. >+ g_free (dialog->priv->root_path); >+ >+ if (res != GNOME_VFS_OK) { >+ g_warning ("gnome_vfs_directory_visit () failed for %s", >+ uri); >+ >+ return FALSE; >+ } >+ >+ /* Add invalid files to the liststore */ >+ g_slist_foreach (dialog->priv->invalid_files, add_to_liststore, dialog); >+ found = (dialog->priv->invalid_files != NULL); >+ >+ ncb_rename_dialog_list_free (dialog); >+ >+ return found; >+} >+ >+/* ncb_rename_dialog_new: >+ * >+ * Create a new #NcbRenameDialog. >+ * >+ * Return value: a new #NcbRenameDialog or NULL. Free with gtk_widget_destroy() */ Doc style. >+NcbRenameDialog * >+ncb_rename_dialog_new (void) >+{ >+ GObject *object; >+ >+ object = g_object_new (NCB_TYPE_RENAME_DIALOG, NULL); >+ g_return_val_if_fail (object != NULL, NULL); Spacing. And shouldn't be using g_return_* here but an assert if it is needed at all. >+ gtk_window_set_title (GTK_WINDOW (object), ""); >+ gtk_window_set_icon_name (GTK_WINDOW (object), "nautilus-cd-burner"); Spacing. These functions should probably go into the _init() instead. >+ return NCB_RENAME_DIALOG (object); >+} >+ >+static void >+ncb_rename_dialog_finalize (GObject *object) >+{ >+ NcbRenameDialog *dialog; >+ >+ g_return_if_fail (object != NULL); >+ g_return_if_fail (NCB_IS_RENAME_DIALOG (object)); >+ >+ dialog = NCB_RENAME_DIALOG (object); >+ g_return_if_fail (dialog->priv != NULL); >+ >+ gtk_list_store_clear (dialog->priv->store); >+ >+ G_OBJECT_CLASS (ncb_rename_dialog_parent_class)->finalize (object); >+} Spacing. >--- /dev/null 2006-06-19 18:46:33.000000000 +0200 >+++ src/ncb-rename-dialog.h 2006-07-09 15:38:33.000000000 +0200 >@@ -0,0 +1,66 @@ >+/* -*- Mode: C; indent-tabs-mode: t; c-basic-offset: 8; tab-width: 8 -*- >+ * >+ * Copyright (C) 2006 Fabio Bonelli <fabiobonelli@libero.it> >+ * >+ * ncb-rename-dialog: Provide a dialog to rename files when their >+ * name is invalid (eg. not encoded properly, too long, etc.). >+ * >+ * This program is free software; you can redistribute it and/or >+ * modify it under the terms of the GNU General Public License as >+ * published by the Free Software Foundation; either version 2 of the >+ * License, or (at your option) any later version. >+ * >+ * This program is distributed in the hope that it will be useful, >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >+ * General Public License for more details. >+ * >+ * You should have received a copy of the GNU General Public >+ * License along with this program; if not, write to the >+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330, >+ * Boston, MA 02111-1307, USA. >+ * >+ * Authors: Fabio Bonelli <fabiobonelli@libero.it> */ >+ >+#ifndef NCB_RENAME_DIALOG_H >+#define NCB_RENAME_DIALOG_H >+ >+#include <glib-object.h> >+#include <gtk/gtk.h> >+ >+G_BEGIN_DECLS >+ >+#define NCB_TYPE_RENAME_DIALOG (ncb_rename_dialog_get_type ()) >+#define NCB_RENAME_DIALOG(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NCB_TYPE_RENAME_DIALOG, \ >+ NcbRenameDialog)) >+#define NCB_RENAME_DIALOG_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NCB_TYPE_RENAME_DIALOG, \ >+ NcbRenameDialogClass)) >+#define NCB_IS_RENAME_DIALOG(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NCB_TYPE_RENAME_DIALOG)) >+#define NCB_IS_RENAME_DIALOG_CLASS(klass) (G_TYPE_INSTANCE_GET_CLASS ((klass), NCB_TYPE_RENAME_DIALOG)) >+ >+typedef struct NcbRenameDialog NcbRenameDialog; >+typedef struct NcbRenameDialogClass NcbRenameDialogClass; >+typedef struct NcbRenameDialogPrivate NcbRenameDialogPrivate; >+ >+struct NcbRenameDialog { >+ GtkMessageDialog parent; >+ >+ NcbRenameDialogPrivate *priv; >+}; >+ >+struct NcbRenameDialogClass { >+ GtkMessageDialogClass parent_class; >+}; >+ >+GType ncb_rename_dialog_get_type (void); >+ >+NcbRenameDialog * ncb_rename_dialog_new (void); Spacing. >+gboolean ncb_rename_dialog_set_invalid_filenames (NcbRenameDialog *dialog, >+ const char *uri); >+void ncb_rename_dialog_response_cb (NcbRenameDialog *dialog, >+ int id, >+ gpointer user_data); >+ >+G_END_DECLS >+ >+#endif /* ! NCB_RENAME_DIALOG_H */
>>+#include <glade/glade.h> >>+#include <libgnomevfs/gnome-vfs.h> >>+#include <libgnomeui/libgnomeui.h> >I see that this pulls in libgnomeui for the mime-type stuff. I'd rather not do >this. Is there another way? We need a replacement for gnome_icon_lookup(). >I think it is better to put this all in a glade file instead of having it all >in the code. Can you give that a try? The documentation doesn't provide any hint on how to integrate libglade and GObjects. I must admit that sometimes GNOME documentation can be frustrating even for basic procedures.
Created attachment 76840 [details] [review] New patch
*** Bug 341621 has been marked as a duplicate of this bug. ***
Created attachment 77658 [details] [review] patch to use glade This patch is after the above patch was committed to CVS HEAD. It converts it to use glade.
Looks good Fabio! I've committed this to HEAD. Thanks.