From 53fa91bb028fc115847700f9f3640f1b107c4592 Mon Sep 17 00:00:00 2001 Message-Id: <53fa91bb028fc115847700f9f3640f1b107c4592.1715543237.git.mdw@distorted.org.uk> From: Mark Wooding Date: Sun, 8 Jun 2008 11:49:49 +0100 Subject: [PATCH] eclient no_response calls all now have errors reported to the per-call callback rather than the generic one. Slightly less convenient due to the formerly widespread practice of passing a null callback, which is now prohibited. Organization: Straylight/Edgeware From: Richard Kettlewell --- disobedience/choose.c | 13 +++-- disobedience/control.c | 12 +++-- disobedience/disobedience.c | 4 +- disobedience/disobedience.h | 5 +- disobedience/properties.c | 46 ++++++++-------- disobedience/queue.c | 31 ++++++++--- disobedience/users.c | 101 +++++++++++++++++------------------- lib/eclient.c | 13 ++--- lib/eclient.h | 8 ++- 9 files changed, 135 insertions(+), 98 deletions(-) diff --git a/disobedience/choose.c b/disobedience/choose.c index 2348267..bfeae10 100644 --- a/disobedience/choose.c +++ b/disobedience/choose.c @@ -1088,6 +1088,13 @@ static void clear_selection(struct choosenode *cn) { /* User actions ------------------------------------------------------------ */ +/** @brief Called when disorder_eclient_play completes */ +void play_completed(void attribute((unused)) *v, + const char *error) { + if(error) + popup_protocol_error(0, error); +} + /** @brief Clicked on something * * This implements playing, all the modifiers for selection, etc. @@ -1162,7 +1169,7 @@ static void clicked_choosenode(GtkWidget attribute((unused)) *widget, clear_selection(root); set_selection(cn, 1); gtk_label_set_text(GTK_LABEL(report_label), "adding track to queue"); - disorder_eclient_play(client, cn->path, 0, 0); + disorder_eclient_play(client, cn->path, play_completed, 0); last_click = 0; } } else if(event->type == GDK_BUTTON_PRESS @@ -1238,7 +1245,7 @@ static void activate_track_play(GtkMenuItem attribute((unused)) *menuitem, gtk_label_set_text(GTK_LABEL(report_label), "adding track to queue"); for(n = 0; tracks[n]; ++n) - disorder_eclient_play(client, tracks[n], 0, 0); + disorder_eclient_play(client, tracks[n], play_completed, 0); } /** @brief Called when the menu's properties option is activated */ @@ -1286,7 +1293,7 @@ static void play_dir(struct choosenode *cn, gtk_label_set_text(GTK_LABEL(report_label), "adding track to queue"); for(n = 0; n < ntracks; ++n) - disorder_eclient_play(client, tracks[n], 0, 0); + disorder_eclient_play(client, tracks[n], play_completed, 0); } static void properties_dir(struct choosenode *cn, diff --git a/disobedience/control.c b/disobedience/control.c index 57931be..15c58e9 100644 --- a/disobedience/control.c +++ b/disobedience/control.c @@ -405,12 +405,18 @@ static void update_nortp(const struct icon *icon) { update_icon(icon, visible, usable); } +static void icon_action_completed(void attribute((unused)) *v, + const char *error) { + if(error) + popup_protocol_error(0, error); +} + static void clicked_icon(GtkButton attribute((unused)) *button, gpointer userdata) { const struct icon *icon = userdata; if(!suppress_actions) - icon->action(client, 0, 0); + icon->action(client, icon_action_completed, 0); } static void clicked_menu(GtkMenuItem attribute((unused)) *menuitem, @@ -418,7 +424,7 @@ static void clicked_menu(GtkMenuItem attribute((unused)) *menuitem, const struct icon *icon = userdata; if(!suppress_actions) - icon->action(client, 0, 0); + icon->action(client, icon_action_completed, 0); } static void toggled_menu(GtkCheckMenuItem *menuitem, @@ -436,7 +442,7 @@ static void toggled_menu(GtkCheckMenuItem *menuitem, && !!(icons[n].flags & ICON_INACTIVE) == !!menuitem->active) break; if(n < NICONS) - icons[n].action(client, 0, 0); + icons[n].action(client, icon_action_completed, 0); } /** @brief Called when the volume has been adjusted */ diff --git a/disobedience/disobedience.c b/disobedience/disobedience.c index d6aa599..a3b09af 100644 --- a/disobedience/disobedience.c +++ b/disobedience/disobedience.c @@ -325,7 +325,9 @@ static gboolean periodic_fast(gpointer attribute((unused)) data) { } /** @brief Called when a NOP completes */ -static void nop_completed(void attribute((unused)) *v) { +static void nop_completed(void attribute((unused)) *v, + const char attribute((unused)) *error) { + /* TODO report the error somewhere */ nop_in_flight = 0; } diff --git a/disobedience/disobedience.h b/disobedience/disobedience.h index db1e8e9..cbe06f4 100644 --- a/disobedience/disobedience.h +++ b/disobedience/disobedience.h @@ -45,6 +45,7 @@ #include "configuration.h" #include "hash.h" #include "selection.h" +#include "kvp.h" #include #include @@ -234,7 +235,6 @@ void namepart_update(const char *track, const char *part); /* Called when a namepart might have changed */ - /* Choose */ GtkWidget *choose_widget(void); @@ -243,6 +243,9 @@ GtkWidget *choose_widget(void); void choose_update(void); /* Called when we think the choose tree might need updating */ +void play_completed(void *v, + const char *error); + /* Login details */ void login_box(void); diff --git a/disobedience/properties.c b/disobedience/properties.c index ec7b46a..fe2bbf5 100644 --- a/disobedience/properties.c +++ b/disobedience/properties.c @@ -29,7 +29,7 @@ static void completed_namepart(struct prefdata *f); static const char *get_edited_namepart(struct prefdata *f); static void set_edited_namepart(struct prefdata *f, const char *value); static void set_namepart(struct prefdata *f, const char *value); -static void set_namepart_completed(void *v); +static void set_namepart_completed(void *v, const char *error); static void kickoff_string(struct prefdata *f); static void completed_string(struct prefdata *f); @@ -324,24 +324,25 @@ static void set_edited_namepart(struct prefdata *f, const char *value) { static void set_namepart(struct prefdata *f, const char *value) { char *s; - struct callbackdata *cbd = xmalloc(sizeof *cbd); - cbd->u.f = f; byte_xasprintf(&s, "trackname_display_%s", f->p->part); /* We don't know what the default is so can never unset. This is a bug * relative to the original design, which is supposed to only ever allow for * non-trivial namepart preferences. I suppose the server could spot a * default being set and translate it into an unset. */ disorder_eclient_set(client, set_namepart_completed, f->track, s, value, - cbd); + f); } /* Called when we've set a namepart */ -static void set_namepart_completed(void *v) { - struct callbackdata *cbd = v; - struct prefdata *f = cbd->u.f; - - namepart_update(f->track, "display", f->p->part); +static void set_namepart_completed(void *v, const char *error) { + if(error) + popup_protocol_error(0, error); + else { + struct prefdata *f = v; + + namepart_update(f->track, "display", f->p->part); + } } /* String preferences ------------------------------------------------------ */ @@ -366,15 +367,15 @@ static void set_edited_string(struct prefdata *f, const char *value) { gtk_entry_set_text(GTK_ENTRY(f->widget), value); } +static void set_string_completed(void attribute((unused)) *v, + const char *error) { + if(error) + popup_protocol_error(0, error); +} + static void set_string(struct prefdata *f, const char *value) { - if(strcmp(f->p->default_value, value)) - /* Different from default, set it */ - disorder_eclient_set(client, 0/*completed*/, f->track, f->p->part, - value, 0/*v*/); - else - /* Same as default, just unset */ - disorder_eclient_unset(client, 0/*completed*/, f->track, f->p->part, - 0/*v*/); + disorder_eclient_set(client, set_string_completed, f->track, f->p->part, + value, 0/*v*/); } /* Boolean preferences ----------------------------------------------------- */ @@ -402,17 +403,14 @@ static void set_edited_boolean(struct prefdata *f, const char *value) { strcmp(value, "0")); } +#define set_boolean_completed set_string_completed + static void set_boolean(struct prefdata *f, const char *value) { char *s; byte_xasprintf(&s, "trackname_display_%s", f->p->part); - if(strcmp(value, f->p->default_value)) - disorder_eclient_set(client, 0/*completed*/, f->track, f->p->part, value, - 0/*v*/); - else - /* If default value then delete the pref */ - disorder_eclient_unset(client, 0/*completed*/, f->track, f->p->part, - 0/*v*/); + disorder_eclient_set(client, set_boolean_completed, + f->track, f->p->part, value, 0/*v*/); } /* Querying preferences ---------------------------------------------------- */ diff --git a/disobedience/queue.c b/disobedience/queue.c index 291fab9..7240417 100644 --- a/disobedience/queue.c +++ b/disobedience/queue.c @@ -774,6 +774,12 @@ static struct queue_entry *findentry(struct queuelike *ql, return q; } +static void move_completed(void attribute((unused)) *v, + const char *error) { + if(error) + popup_protocol_error(0, error); +} + /** @brief Called when data is dropped */ static gboolean queue_drag_drop(GtkWidget attribute((unused)) *widget, GdkDragContext *drag_context, @@ -793,7 +799,7 @@ static gboolean queue_drag_drop(GtkWidget attribute((unused)) *widget, if(q != playing_track && selection_selected(ql->selection, q->id)) vector_append(&vec, (char *)q->id); disorder_eclient_moveafter(client, id, vec.nvec, (const char **)vec.vec, - 0/*completed*/, 0/*v*/); + move_completed, 0/*v*/); gtk_drag_finish(drag_context, TRUE, TRUE, when); /* Destroy dropzones */ remove_drag_targets(ql); @@ -1190,11 +1196,18 @@ static int scratch_sensitive(struct queuelike attribute((unused)) *ql, && selection_selected(ql->selection, playing_track->id)); } +/** @brief Called when disorder_eclient_scratch completes */ +static void scratch_completed(void attribute((unused)) *v, + const char *error) { + if(error) + popup_protocol_error(0, error); +} + /** @brief Scratch the playing track */ static void scratch_activate(GtkMenuItem attribute((unused)) *menuitem, gpointer attribute((unused)) user_data) { if(playing_track) - disorder_eclient_scratch(client, playing_track->id, 0, 0); + disorder_eclient_scratch(client, playing_track->id, scratch_completed, 0); } /** @brief Determine whether the remove option should be sensitive */ @@ -1209,6 +1222,12 @@ static int remove_sensitive(struct queuelike *ql, || count_selected_nonplaying(ql))); } +static void remove_completed(void attribute((unused)) *v, + const char *error) { + if(error) + popup_protocol_error(0, error); +} + /** @brief Remove selected track(s) */ static void remove_activate(GtkMenuItem attribute((unused)) *menuitem, gpointer user_data) { @@ -1220,10 +1239,10 @@ static void remove_activate(GtkMenuItem attribute((unused)) *menuitem, /* Remove selected tracks */ for(q = ql->q; q; q = q->next) if(selection_selected(ql->selection, q->id) && q != playing_track) - disorder_eclient_remove(client, q->id, 0, 0); + disorder_eclient_remove(client, q->id, move_completed, 0); } else if(q) /* Remove just the hovered track */ - disorder_eclient_remove(client, q->id, 0, 0); + disorder_eclient_remove(client, q->id, remove_completed, 0); } /** @brief Determine whether the properties menu option should be sensitive */ @@ -1293,10 +1312,10 @@ static void play_activate(GtkMenuItem attribute((unused)) *menuitem, /* Play selected tracks */ for(q = ql->q; q; q = q->next) if(selection_selected(ql->selection, q->id)) - disorder_eclient_play(client, q->track, 0, 0); + disorder_eclient_play(client, q->track, play_completed, 0); } else if(q) /* Nothing is selected, so play the hovered track */ - disorder_eclient_play(client, q->track, 0, 0); + disorder_eclient_play(client, q->track, play_completed, 0); } /* The queue --------------------------------------------------------------- */ diff --git a/disobedience/users.c b/disobedience/users.c index cb44be5..589716e 100644 --- a/disobedience/users.c +++ b/disobedience/users.c @@ -380,42 +380,38 @@ static rights_type users_get_rights(void) { return r; } -/** @brief Called when various things fail */ -static void users_op_failed(struct callbackdata attribute((unused)) *cbd, - int attribute((unused)) code, - const char *msg) { - popup_submsg(users_window, GTK_MESSAGE_ERROR, msg); +/** @brief Called when a user setting has been edited */ +static void users_edituser_completed(void attribute((unused)) *v, + const char *error) { + if(error) + popup_submsg(users_window, GTK_MESSAGE_ERROR, error); } /** @brief Called when a new user has been created */ -static void users_adduser_completed(void *v) { - struct callbackdata *cbd = v; - - /* Now the user is created we can go ahead and set the email address */ - if(*cbd->u.edituser.email) { - struct callbackdata *ncbd = xmalloc(sizeof *cbd); - ncbd->onerror = users_op_failed; - disorder_eclient_edituser(client, NULL, cbd->u.edituser.user, - "email", cbd->u.edituser.email, ncbd); +static void users_adduser_completed(void *v, + const char *error) { + if(error) { + popup_submsg(users_window, GTK_MESSAGE_ERROR, error); + mode(ADD); /* Let the user try again */ + } else { + const struct kvp *const kvp = v; + const char *user = kvp_get(kvp, "user"); + const char *email = kvp_get(kvp, "email"); /* maybe NULL */ + + /* Now the user is created we can go ahead and set the email address */ + if(email) + disorder_eclient_edituser(client, users_edituser_completed, user, + "email", email, NULL); + /* Refresh the list of users */ + disorder_eclient_users(client, users_got_list, 0); + /* We'll select the newly created user */ + users_deferred_select = user; } - /* Refresh the list of users */ - disorder_eclient_users(client, users_got_list, 0); - /* We'll select the newly created user */ - users_deferred_select = cbd->u.edituser.user; -} - -/** @brief Called if creating a new user fails */ -static void users_adduser_failed(struct callbackdata attribute((unused)) *cbd, - int attribute((unused)) code, - const char *msg) { - popup_submsg(users_window, GTK_MESSAGE_ERROR, msg); - mode(ADD); /* Let the user try again */ } /** @brief Called when the 'Apply' button is pressed */ static void users_apply(GtkButton attribute((unused)) *button, gpointer attribute((unused)) userdata) { - struct callbackdata *cbd; const char *password; const char *password2; const char *name; @@ -447,15 +443,14 @@ static void users_apply(GtkButton attribute((unused)) *button, popup_submsg(users_window, GTK_MESSAGE_ERROR, "Invalid email address"); return; } - cbd = xmalloc(sizeof *cbd); - cbd->onerror = users_adduser_failed; - cbd->u.edituser.user = name; - cbd->u.edituser.email = email; - disorder_eclient_adduser(client, users_adduser_completed, + disorder_eclient_adduser(client, + users_adduser_completed, name, password, rights_string(users_get_rights()), - cbd); + kvp_make("user", name, + "email", email, + (char *)0)); /* We switch to no-op mode while creating the user */ mode(NONE); break; @@ -472,26 +467,30 @@ static void users_apply(GtkButton attribute((unused)) *button, popup_submsg(users_window, GTK_MESSAGE_ERROR, "Invalid email address"); return; } - cbd = xmalloc(sizeof *cbd); - cbd->onerror = users_op_failed; - disorder_eclient_edituser(client, NULL, users_selected, - "email", email, cbd); - disorder_eclient_edituser(client, NULL, users_selected, - "password", password, cbd); - disorder_eclient_edituser(client, NULL, users_selected, - "rights", rights_string(users_get_rights()), cbd); + disorder_eclient_edituser(client, users_edituser_completed, users_selected, + "email", email, NULL); + disorder_eclient_edituser(client, users_edituser_completed, users_selected, + "password", password, NULL); + disorder_eclient_edituser(client, users_edituser_completed, users_selected, + "rights", rights_string(users_get_rights()), NULL); /* We remain in edit mode */ break; } } /** @brief Called when a user has been deleted */ -static void users_deleted(void *v) { - const struct callbackdata *const cbd = v; - GtkTreeIter iter[1]; - - if(!users_find_user(cbd->u.user, iter)) /* Find the user... */ - gtk_list_store_remove(users_list, iter); /* ...and remove them */ +static void users_delete_completed(void *v, + const char *error) { + if(error) + popup_submsg(users_window, GTK_MESSAGE_ERROR, error); + else { + const struct kvp *const kvp = v; + const char *const user = kvp_get(kvp, "user"); + GtkTreeIter iter[1]; + + if(!users_find_user(user, iter)) /* Find the user... */ + gtk_list_store_remove(users_list, iter); /* ...and remove them */ + } } /** @brief Called when the 'Delete' button is pressed */ @@ -499,7 +498,6 @@ static void users_delete(GtkButton attribute((unused)) *button, gpointer attribute((unused)) userdata) { GtkWidget *yesno; int res; - struct callbackdata *cbd; if(!users_selected) return; @@ -513,10 +511,9 @@ static void users_delete(GtkButton attribute((unused)) *button, res = gtk_dialog_run(GTK_DIALOG(yesno)); gtk_widget_destroy(yesno); if(res == GTK_RESPONSE_YES) { - cbd = xmalloc(sizeof *cbd); - cbd->onerror = users_op_failed; - cbd->u.user = users_selected; - disorder_eclient_deluser(client, users_deleted, cbd->u.user, cbd); + disorder_eclient_deluser(client, users_delete_completed, users_selected, + kvp_make("user", users_selected, + (char *)0)); } } diff --git a/lib/eclient.c b/lib/eclient.c index 96df2c0..d784d41 100644 --- a/lib/eclient.c +++ b/lib/eclient.c @@ -884,13 +884,14 @@ static void integer_response_opcallback(disorder_eclient *c, /* for commands with no response */ static void no_response_opcallback(disorder_eclient *c, struct operation *op) { + disorder_eclient_no_response *completed + = (disorder_eclient_no_response *)op->completed; + D(("no_response_callback")); - if(c->rc / 100 == 2) { - if(op->completed) - ((disorder_eclient_no_response *)op->completed)(op->v); - } else - /* TODO don't use protocol_error here */ - protocol_error(c, op, c->rc, "%s: %s", c->ident, c->line); + if(c->rc / 100 == 2) + completed(op->v, NULL); + else + completed(op->v, errorstring(c)); } /* error callback for queue_unmarshall */ diff --git a/lib/eclient.h b/lib/eclient.h index e459619..3ea8b15 100644 --- a/lib/eclient.h +++ b/lib/eclient.h @@ -147,8 +147,12 @@ struct sink; * It is always allowed for these to be null pointers if you don't care about * the result. */ -typedef void disorder_eclient_no_response(void *v); -/* completion callback with no data */ +/** @brief Trivial completion callback + * @param v User data + * @param error Error string or NULL on succes + */ +typedef void disorder_eclient_no_response(void *v, + const char *error); /** @brief String result completion callback * @param v User data -- [mdw]