On Wed, 2005-01-19 at 02:33 +0100, Jean-Yves Lefort wrote:
> Please consider committing the patch.
Here's a quick review before Christian gets to it:
+static void ephy_action_properties_dialog_set_property (GObject
*object,
+ unsigned int prop_id,
+ const GValue *value,
+ GParamSpec *pspec);
+static void ephy_action_properties_dialog_get_property (GObject
*object,
+ unsigned int prop_id,
+ GValue *value,
+ GParamSpec *pspec);
+
+static void ephy_action_properties_dialog_update_title
(EphyActionPropertiesDialog *dialog);
(etc)
Try to avoid declaring functions at the top of the file. Notice how
little it's done elsewhere in epiphany(-extensions)....
+GType
+ephy_action_properties_dialog_register_type (GTypeModule *module)
+{
+ static const GTypeInfo info = {
+ sizeof(EphyActionPropertiesDialogClass),
+ NULL,
Indent with 8-space tabs, not spaces. (See the HACKING file....)
+ if (dialog->priv->id)
+ apply_type = PT_AUTOAPPLY;
+ else
+ {
See the HACKING file....
+ dialog->priv->id = g_strdup_printf("action_%lu_%lu", now.tv_sec,
now.tv_usec);
Is there no better way?
+ dialog->priv->popup_remove_item =
ephy_actions_editor_dialog_append_popup_item(dialog, GTK_STOCK_REMOVE,
G_CALLBACK(ephy_actions_editor_dialog_remove_selected));
As a general guideline, we like to stay below 80 characters wide, too.
(I think that's in the HACKING file.)
+ markup = g_markup_printf_escaped("<span weight=\"bold\">%
s</span>\n%s", formatted, description);
Can't you just use <b></b>?
+ GDK_THREADS_ENTER();
+ ephy_actions_editor_dialog_update_list(user_data);
+ GDK_THREADS_LEAVE();
I'm not 100% sure, but I think extensions are always invoked from within
the same thread. Christian would know for sure.
# List of source files containing translatable strings.
# Please keep this file sorted alphabetically.
+extensions/actions/ephy-action-properties-dialog.c
+extensions/actions/ephy-actions.conf
+extensions/actions/ephy-actions-editor-dialog.c
+extensions/actions/ephy-actions-extension.c
+extensions/actions/ephy-actions-util.c
+extensions/actions/extension.c
I haven't checked them all, but extensions/actions/extension.c doesn't
have translatable strings and I imagine there are others which don't, as
well.
That's all I can see. The code itself looks great as far as I can tell,
but I'm sure Christian will spot other problems....
--
Adam Hooper <adamh densi com>
Attachment:
signature.asc
Description: This is a digitally signed message part