Merge branch 'master' into osx-functional

src/command/cmd_defs.c

@@ -803,9 +803,9 @@ static struct cmd_t command_defs[] =
         CMD_SYN(
             "/bookmark",
             "/bookmark list",
-            "/bookmark add <room> [nick <nick>] [password <password>] [autojoin on|off]",
+            "/bookmark add [<room>] [nick <nick>] [password <password>] [autojoin on|off]",
             "/bookmark update <room> [nick <nick>] [password <password>] [autojoin on|off]",
-            "/bookmark remove <room>",
+            "/bookmark remove [<room>]",
             "/bookmark join <room>",
             "/bookmark invites on|off")
         CMD_DESC(
@@ -813,8 +813,8 @@ static struct cmd_t command_defs[] =
             "In a chat room, no arguments will bookmark the current room, setting autojoin to \"on\".")
         CMD_ARGS(
             { "list", "List all bookmarks." },
-            { "add <room>", "Add a bookmark." },
+            { "add [<room>]", "Add a bookmark, passing no room will bookmark the current room, setting autojoin to \"on\"." },
-            { "remove <room>", "Remove a bookmark." },
+            { "remove [<room>]", "Remove a bookmark, passing no room will remove the bookmark for the current room, if one exists." },
             { "update <room>", "Update the properties associated with a bookmark." },
             { "nick <nick>", "Nickname used in the chat room." },
             { "password <password>", "Password if required, may be stored in plaintext on your server." },

src/command/cmd_funcs.c

@@ -4325,7 +4325,7 @@ cmd_bookmark(ProfWin *window, const char *const command, gchar **args)
     }
 
     gchar *cmd = args[0];
-    if (window->type == WIN_MUC && cmd == NULL) {
+    if (window->type == WIN_MUC && (cmd == NULL || g_strcmp0(cmd, "add") == 0)) {
         // default to current nickname, password, and autojoin "on"
         ProfMucWin *mucwin = (ProfMucWin*)window;
         assert(mucwin->memcheck == PROFMUCWIN_MEMCHECK);
@@ -4338,103 +4338,123 @@ cmd_bookmark(ProfWin *window, const char *const command, gchar **args)
             ui_current_print_formatted_line('!', 0, "Bookmark already exists for %s.", mucwin->roomjid);
         }
         return TRUE;
-
-    } else {
-        if (cmd == NULL) {
-            cons_bad_cmd_usage(command);
-            return TRUE;
-        }
-
-        if (strcmp(cmd, "invites") == 0) {
-            if (g_strcmp0(args[1], "on") == 0) {
-                prefs_set_boolean(PREF_BOOKMARK_INVITE, TRUE);
-                cons_show("Auto bookmarking accepted invites enabled.");
-            } else if (g_strcmp0(args[1], "off") == 0) {
-                prefs_set_boolean(PREF_BOOKMARK_INVITE, FALSE);
-                cons_show("Auto bookmarking accepted invites disabled.");
-            } else {
-                cons_bad_cmd_usage(command);
-                cons_show("");
-            }
-            return TRUE;
-        } else if (strcmp(cmd, "list") == 0) {
-            GList *bookmarks = bookmark_get_list();
-            cons_show_bookmarks(bookmarks);
-            g_list_free(bookmarks);
-        } else {
-            char *jid = args[1];
-            if (jid == NULL) {
-                cons_bad_cmd_usage(command);
-                cons_show("");
-                return TRUE;
-            }
-
-            if (strcmp(cmd, "remove") == 0) {
-                gboolean removed = bookmark_remove(jid);
-                if (removed) {
-                    cons_show("Bookmark removed for %s.", jid);
-                } else {
-                    cons_show("No bookmark exists for %s.", jid);
-                }
-                return TRUE;
-            }
-
-            if (strcmp(cmd, "join") == 0) {
-                gboolean joined = bookmark_join(jid);
-                if (!joined) {
-                    cons_show("No bookmark exists for %s.", jid);
-                }
-                return TRUE;
-            }
-
-            gchar *opt_keys[] = { "autojoin", "nick", "password", NULL };
-            gboolean parsed;
-
-            GHashTable *options = parse_options(&args[2], opt_keys, &parsed);
-            if (!parsed) {
-                cons_bad_cmd_usage(command);
-                cons_show("");
-                return TRUE;
-            }
-
-            char *nick = g_hash_table_lookup(options, "nick");
-            char *password = g_hash_table_lookup(options, "password");
-            char *autojoin = g_hash_table_lookup(options, "autojoin");
-
-            if (autojoin) {
-                if ((strcmp(autojoin, "on") != 0) && (strcmp(autojoin, "off") != 0)) {
-                    cons_bad_cmd_usage(command);
-                    cons_show("");
-                    return TRUE;
-                }
-            }
-
-            if (strcmp(cmd, "add") == 0) {
-                if (strchr(jid, '@')==NULL) {
-                    cons_show("Can't add bookmark with JID '%s'; should be '%s@domain.tld'", jid, jid);
-                } else {
-                    gboolean added = bookmark_add(jid, nick, password, autojoin);
-                    if (added) {
-                        cons_show("Bookmark added for %s.", jid);
-                    } else {
-                        cons_show("Bookmark already exists, use /bookmark update to edit.");
-                    }
-                }
-            } else if (strcmp(cmd, "update") == 0) {
-                gboolean updated = bookmark_update(jid, nick, password, autojoin);
-                if (updated) {
-                    cons_show("Bookmark updated.");
-                } else {
-                    cons_show("No bookmark exists for %s.", jid);
-                }
-            } else {
-                cons_bad_cmd_usage(command);
-            }
-
-            options_destroy(options);
-        }
     }
 
+    if (window->type == WIN_MUC && g_strcmp0(cmd, "remove") == 0) {
+        ProfMucWin *mucwin = (ProfMucWin*)window;
+        assert(mucwin->memcheck == PROFMUCWIN_MEMCHECK);
+        gboolean removed = bookmark_remove(mucwin->roomjid);
+        if (removed) {
+            ui_current_print_formatted_line('!', 0, "Bookmark removed for %s.", mucwin->roomjid);
+        } else {
+            ui_current_print_formatted_line('!', 0, "Bookmark does not exist for %s.", mucwin->roomjid);
+        }
+        return TRUE;
+    }
+
+    if (cmd == NULL) {
+        cons_bad_cmd_usage(command);
+        return TRUE;
+    }
+
+    if (strcmp(cmd, "invites") == 0) {
+        if (g_strcmp0(args[1], "on") == 0) {
+            prefs_set_boolean(PREF_BOOKMARK_INVITE, TRUE);
+            cons_show("Auto bookmarking accepted invites enabled.");
+        } else if (g_strcmp0(args[1], "off") == 0) {
+            prefs_set_boolean(PREF_BOOKMARK_INVITE, FALSE);
+            cons_show("Auto bookmarking accepted invites disabled.");
+        } else {
+            cons_bad_cmd_usage(command);
+            cons_show("");
+        }
+        return TRUE;
+    }
+
+    if (strcmp(cmd, "list") == 0) {
+        GList *bookmarks = bookmark_get_list();
+        cons_show_bookmarks(bookmarks);
+        g_list_free(bookmarks);
+        return TRUE;
+    }
+
+    char *jid = args[1];
+    if (jid == NULL) {
+        cons_bad_cmd_usage(command);
+        cons_show("");
+        return TRUE;
+    }
+    if (strchr(jid, '@') == NULL) {
+        cons_show("Invalid room, must be of the form room@domain.tld");
+        cons_show("");
+        return TRUE;
+    }
+
+    if (strcmp(cmd, "remove") == 0) {
+        gboolean removed = bookmark_remove(jid);
+        if (removed) {
+            cons_show("Bookmark removed for %s.", jid);
+        } else {
+            cons_show("No bookmark exists for %s.", jid);
+        }
+        return TRUE;
+    }
+
+    if (strcmp(cmd, "join") == 0) {
+        gboolean joined = bookmark_join(jid);
+        if (!joined) {
+            cons_show("No bookmark exists for %s.", jid);
+        }
+        return TRUE;
+    }
+
+    gchar *opt_keys[] = { "autojoin", "nick", "password", NULL };
+    gboolean parsed;
+
+    GHashTable *options = parse_options(&args[2], opt_keys, &parsed);
+    if (!parsed) {
+        cons_bad_cmd_usage(command);
+        cons_show("");
+        return TRUE;
+    }
+
+    char *autojoin = g_hash_table_lookup(options, "autojoin");
+
+    if (autojoin && ((strcmp(autojoin, "on") != 0) && (strcmp(autojoin, "off") != 0))) {
+        cons_bad_cmd_usage(command);
+        cons_show("");
+        options_destroy(options);
+        return TRUE;
+    }
+
+    char *nick = g_hash_table_lookup(options, "nick");
+    char *password = g_hash_table_lookup(options, "password");
+
+    if (strcmp(cmd, "add") == 0) {
+        gboolean added = bookmark_add(jid, nick, password, autojoin);
+        if (added) {
+            cons_show("Bookmark added for %s.", jid);
+        } else {
+            cons_show("Bookmark already exists, use /bookmark update to edit.");
+        }
+        options_destroy(options);
+        return TRUE;
+    }
+
+    if (strcmp(cmd, "update") == 0) {
+        gboolean updated = bookmark_update(jid, nick, password, autojoin);
+        if (updated) {
+            cons_show("Bookmark updated.");
+        } else {
+            cons_show("No bookmark exists for %s.", jid);
+        }
+        options_destroy(options);
+        return TRUE;
+    }
+
+    cons_bad_cmd_usage(command);
+    options_destroy(options);
+
     return TRUE;
 }
 

tests/unittests/test_cmd_bookmark.c

@@ -137,7 +137,8 @@ void cmd_bookmark_add_shows_message_when_invalid_jid(void **state)
 
     will_return(connection_get_status, JABBER_CONNECTED);
 
-    expect_cons_show("Can't add bookmark with JID 'room'; should be 'room@domain.tld'");
+    expect_cons_show("Invalid room, must be of the form room@domain.tld");
+    expect_cons_show("");
 
     gboolean result = cmd_bookmark(&window, CMD_BOOKMARK, args);
     assert_true(result);