--- a/libervia/cli/cmd_bookmarks.py	Wed Nov 20 11:38:44 2024 +0100
+++ b/libervia/cli/cmd_bookmarks.py	Wed Nov 20 11:43:27 2024 +0100
@@ -17,86 +17,70 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from . import base
+from rich.table import Table
+
 from libervia.backend.core.i18n import _
+from libervia.backend.tools.common import data_format
 from libervia.cli.constants import Const as C
 
+from . import base
+from .bookmarks_legacy import BookmarksLegacy
+
 __commands__ = ["Bookmarks"]
 
-STORAGE_LOCATIONS = ("local", "private", "pubsub")
-TYPES = ("muc", "url")
 
-
-class BookmarksCommon(base.CommandBase):
-    """Class used to group common options of bookmarks subcommands"""
-
-    def add_parser_options(self, location_default="all"):
-        self.parser.add_argument(
-            "-l",
-            "--location",
-            type=str,
-            choices=(location_default,) + STORAGE_LOCATIONS,
-            default=location_default,
-            help=_("storage location (default: %(default)s)"),
-        )
-        self.parser.add_argument(
-            "-t",
-            "--type",
-            type=str,
-            choices=TYPES,
-            default=TYPES[0],
-            help=_("bookmarks type (default: %(default)s)"),
+class BookmarksList(base.CommandBase):
+    def __init__(self, host):
+        extra_outputs = {"default": self.default_output}
+        super().__init__(
+            host, "list", help=_("list bookmarks"),
+            use_output=C.OUTPUT_COMPLEX,
+            extra_outputs=extra_outputs
         )
 
+    def add_parser_options(self):
+        pass
 
-class BookmarksList(BookmarksCommon):
-    def __init__(self, host):
-        super(BookmarksList, self).__init__(host, "list", help=_("list bookmarks"))
+    def default_output(self, data: dict) -> None:
+        table = Table(title="📚 " + _("Group Chat Bookmarks"))
+        table.add_column("🌐 JID")
+        table.add_column("📝 " + _("Name"))
+        table.add_column("👤 " + _("Nick"))
+        table.add_column("🔒 " + _("Password"))
+        table.add_column("🚪 " + _("Joined"))
+
+        for jid, conference_data in data.items():
+            table.add_row(
+                str(jid),
+                conference_data.get("name", ""),
+                conference_data.get("nick", ""),
+                conference_data.get("password", ""),
+                "✅" if conference_data.get("autojoin", False) else "❌"
+            )
+
+        self.console.print(table)
 
     async def start(self):
         try:
-            data = await self.host.bridge.bookmarks_list(
-                self.args.type, self.args.location, self.host.profile
-            )
+            data = data_format.deserialise(await self.host.bridge.bookmarks_list(
+                "", self.host.profile
+            ))
         except Exception as e:
             self.disp(f"can't get bookmarks list: {e}", error=True)
             self.host.quit(C.EXIT_BRIDGE_ERRBACK)
+            return
 
-        mess = []
-        for location in STORAGE_LOCATIONS:
-            if not data[location]:
-                continue
-            loc_mess = []
-            loc_mess.append(f"{location}:")
-            book_mess = []
-            for book_link, book_data in list(data[location].items()):
-                name = book_data.get("name")
-                autojoin = book_data.get("autojoin", "false") == "true"
-                nick = book_data.get("nick")
-                book_mess.append(
-                    "\t%s[%s%s]%s"
-                    % (
-                        (name + " ") if name else "",
-                        book_link,
-                        " (%s)" % nick if nick else "",
-                        " (*)" if autojoin else "",
-                    )
-                )
-            loc_mess.append("\n".join(book_mess))
-            mess.append("\n".join(loc_mess))
-
-        print("\n\n".join(mess))
+        await self.output(data)
         self.host.quit()
 
 
-class BookmarksRemove(BookmarksCommon):
+class BookmarksRemove(base.CommandBase):
     def __init__(self, host):
-        super(BookmarksRemove, self).__init__(host, "remove", help=_("remove a bookmark"))
+        super().__init__(host, "remove", help=_("remove a bookmark"))
 
     def add_parser_options(self):
-        super(BookmarksRemove, self).add_parser_options()
         self.parser.add_argument(
-            "bookmark", help=_("jid (for muc bookmark) or url of to remove")
+            "bookmark", help=_("jid of the bookmark to remove")
         )
         self.parser.add_argument(
             "-f",
@@ -107,11 +91,14 @@
 
     async def start(self):
         if not self.args.force:
-            await self.host.confirm_or_quit(_("Are you sure to delete this bookmark?"))
+            await self.host.confirm_or_quit(
+                _("Are you sure to delete the bookmark {bookmark_id!r}?")
+                .format(bookmark_id=self.args.bookmark)
+            )
 
         try:
-            await self.host.bridge.bookmarks_remove(
-                self.args.type, self.args.bookmark, self.args.location, self.host.profile
+            await self.host.bridge.bookmark_remove(
+                self.args.bookmark, self.host.profile
             )
         except Exception as e:
             self.disp(_("can't delete bookmark: {e}").format(e=e), error=True)
@@ -121,55 +108,88 @@
             self.host.quit()
 
 
-class BookmarksAdd(BookmarksCommon):
+class BookmarksSet(base.CommandBase):
     def __init__(self, host):
-        super(BookmarksAdd, self).__init__(host, "add", help=_("add a bookmark"))
+        super().__init__(
+            host, "set", help=_("add or update a bookmark")
+        )
 
     def add_parser_options(self):
-        super(BookmarksAdd, self).add_parser_options(location_default="auto")
+        self.parser.add_argument("bookmark", help=_("jid of the chat room"))
+        self.parser.add_argument("-n", "--name", help=_("bookmark name"), dest="name")
         self.parser.add_argument(
-            "bookmark", help=_("jid (for muc bookmark) or url of to remove")
+            "-j",
+            "--join",
+            nargs="?",
+            # Value use when option is missing.
+            default=None,
+            # Value use when option is used, but value is not specified.
+            const=True,
+            type=base.optional_bool_decoder,
+            # The bookmark attribute is called "autojoin" for historical reason, but it's
+            # now used a "join" flag, so we use ``join`` here for the option.
+            dest="autojoin",
+            metavar="BOOL",
+            help=_("join the conference room"),
         )
-        self.parser.add_argument("-n", "--name", help=_("bookmark name"))
-        muc_group = self.parser.add_argument_group(_("MUC specific options"))
-        muc_group.add_argument("-N", "--nick", help=_("nickname"))
-        muc_group.add_argument(
-            "-a",
-            "--autojoin",
+        self.parser.add_argument(
+            "-N",
+            "--nick", help=_("preferred roomnick for the chatroom")
+        )
+        self.parser.add_argument(
+            "-P",
+            "--password", help=_("password used to access the chatroom")
+        )
+        self.parser.add_argument(
+            "-u",
+            "--update",
             action="store_true",
-            help=_("join room on profile connection"),
+            help=_("update bookmark data instead of replacing")
         )
 
     async def start(self):
-        if self.args.type == "url" and (self.args.autojoin or self.args.nick is not None):
-            self.parser.error(_("You can't use --autojoin or --nick with --type url"))
-        data = {}
-        if self.args.autojoin:
-            data["autojoin"] = "true"
-        if self.args.nick is not None:
-            data["nick"] = self.args.nick
-        if self.args.name is not None:
-            data["name"] = self.args.name
+        conference_data = {
+            "autojoin": self.args.autojoin,
+            "name": self.args.name,
+            "nick": self.args.nick,
+            "password": self.args.password,
+        }
+
+        conference_data = {k: v for k, v in conference_data.items() if v is not None}
+        if self.args.update:
+            try:
+                old_conference_data = data_format.deserialise(
+                    await self.host.bridge.bookmark_get(
+                        self.args.bookmark, self.host.profile
+                    )
+                )
+            except Exception as e:
+                self.disp(
+                    f"Can't find existing bookmark {self.args.bookmark!r}: {e}. We "
+                    "create it.",
+                    error=True
+                )
+            else:
+                old_conference_data.update(conference_data)
+                conference_data = old_conference_data
+
         try:
-            await self.host.bridge.bookmarks_add(
-                self.args.type,
-                self.args.bookmark,
-                data,
-                self.args.location,
+            await self.host.bridge.bookmarks_set(
+                data_format.serialise({self.args.bookmark: conference_data}),
                 self.host.profile,
             )
         except Exception as e:
             self.disp(f"can't add bookmark: {e}", error=True)
             self.host.quit(C.EXIT_BRIDGE_ERRBACK)
         else:
-            self.disp(_("bookmark successfully added"))
+            self.disp(_("bookmark successfully set"))
             self.host.quit()
 
 
 class Bookmarks(base.CommandBase):
-    subcommands = (BookmarksList, BookmarksRemove, BookmarksAdd)
+    subcommands = (BookmarksList, BookmarksSet, BookmarksRemove, BookmarksLegacy)
 
     def __init__(self, host):
-        super(Bookmarks, self).__init__(
+        super().__init__(
             host, "bookmarks", use_profile=False, help=_("manage bookmarks")
         )