[Core] Add redbot --edit cli flag (replacement for [p]set owner&token) (#3060)

* feat(core): add `redbot --edit` cli flag

* chore(changelog): add towncrier entries

* refactor(core): clean up `redbot --edit`, few fixes

* fix(core): prepare for review

* chore(changelog): update towncrier entry to use double ticks :p

* style(black): ugh, Sinbad's git hook isn't perfect (using worktrees)

* fix: Address Flame's first review

changelog.d/3060.enhance.rst

@@ -0,0 +1 @@
+All ``y/n`` confirmations in cli commands are now unified.

changelog.d/3060.feature.rst

@@ -0,0 +1 @@
+Added ``redbot --edit`` cli flag that can be used to edit instance name, token, owner and datapath.

changelog.d/3060.fix.rst

@@ -0,0 +1 @@
+Arguments ``--co-owner`` and ``--load-cogs`` now properly require at least one argument to be passed.

redbot/__main__.py

@@ -6,7 +6,10 @@ import asyncio
 import json
 import logging
 import os
+import shutil
 import sys
+from copy import deepcopy
+from pathlib import Path
 
 import discord
 
@@ -23,6 +26,7 @@ from redbot.core.cog_manager import CogManagerUI
 from redbot.core.global_checks import init_global_checks
 from redbot.core.events import init_events
 from redbot.core.cli import interactive_config, confirm, parse_cli_flags
+from redbot.setup import get_data_dir, get_name, save_config
 from redbot.core.core_commands import Core
 from redbot.core.dev_commands import Dev
 from redbot.core import __version__, modlog, bank, data_manager, drivers
@@ -48,6 +52,12 @@ async def _get_prefix_and_token(red, indict):
     indict["prefix"] = await red._config.prefix()
 
 
+def _get_instance_names():
+    with data_manager.config_file.open(encoding="utf-8") as fs:
+        data = json.load(fs)
+    return sorted(data.keys())
+
+
 def list_instances():
     if not data_manager.config_file.exists():
         print(
@@ -56,15 +66,157 @@ def list_instances():
         )
         sys.exit(1)
     else:
-        with data_manager.config_file.open(encoding="utf-8") as fs:
-            data = json.load(fs)
         text = "Configured Instances:\n\n"
-        for instance_name in sorted(data.keys()):
+        for instance_name in _get_instance_names():
             text += "{}\n".format(instance_name)
         print(text)
         sys.exit(0)
 
 
+def edit_instance(red, cli_flags):
+    no_prompt = cli_flags.no_prompt
+    token = cli_flags.token
+    owner = cli_flags.owner
+    old_name = cli_flags.instance_name
+    new_name = cli_flags.edit_instance_name
+    data_path = cli_flags.edit_data_path
+    copy_data = cli_flags.copy_data
+    confirm_overwrite = cli_flags.overwrite_existing_instance
+
+    if data_path is None and copy_data:
+        print("--copy-data can't be used without --edit-data-path argument")
+        sys.exit(1)
+    if new_name is None and confirm_overwrite:
+        print("--overwrite-existing-instance can't be used without --edit-instance-name argument")
+        sys.exit(1)
+    if no_prompt and all(to_change is None for to_change in (token, owner, new_name, data_path)):
+        print(
+            "No arguments to edit were provided. Available arguments (check help for more "
+            "information): --edit-instance-name, --edit-data-path, --copy-data, --owner, --token"
+        )
+        sys.exit(1)
+
+    _edit_token(red, token, no_prompt)
+    _edit_owner(red, owner, no_prompt)
+
+    data = deepcopy(data_manager.basic_config)
+    name = _edit_instance_name(old_name, new_name, confirm_overwrite, no_prompt)
+    _edit_data_path(data, data_path, copy_data, no_prompt)
+
+    save_config(name, data)
+    if old_name != name:
+        save_config(old_name, {}, remove=True)
+
+
+def _edit_token(red, token, no_prompt):
+    if token:
+        if not len(token) >= 50:
+            print(
+                "The provided token doesn't look a valid Discord bot token."
+                " Instance's token will remain unchanged.\n"
+            )
+            return
+        red.loop.run_until_complete(red._config.token.set(token))
+    elif not no_prompt and confirm("Would you like to change instance's token?", default=False):
+        interactive_config(red, False, True, print_header=False)
+        print("Token updated.\n")
+
+
+def _edit_owner(red, owner, no_prompt):
+    if owner:
+        if not (15 <= len(str(owner)) <= 21):
+            print(
+                "The provided owner id doesn't look like a valid Discord user id."
+                " Instance's owner will remain unchanged."
+            )
+            return
+        red.loop.run_until_complete(red._config.owner.set(owner))
+    elif not no_prompt and confirm("Would you like to change instance's owner?", default=False):
+        print(
+            "Remember:\n"
+            "ONLY the person who is hosting Red should be owner."
+            " This has SERIOUS security implications."
+            " The owner can access any data that is present on the host system.\n"
+        )
+        if confirm("Are you sure you want to change instance's owner?", default=False):
+            print("Please enter a Discord user id for new owner:")
+            while True:
+                owner_id = input("> ").strip()
+                if not (15 <= len(owner_id) <= 21 and owner_id.isdecimal()):
+                    print("That doesn't look like a valid Discord user id.")
+                    continue
+                owner_id = int(owner_id)
+                red.loop.run_until_complete(red._config.owner.set(owner_id))
+                print("Owner updated.")
+                break
+        else:
+            print("Instance's owner will remain unchanged.")
+        print()
+
+
+def _edit_instance_name(old_name, new_name, confirm_overwrite, no_prompt):
+    if new_name:
+        name = new_name
+        if name in _get_instance_names() and not confirm_overwrite:
+            name = old_name
+            print(
+                "An instance with this name already exists.\n"
+                "If you want to remove the existing instance and replace it with this one,"
+                " run this command with --overwrite-existing-instance flag."
+            )
+    elif not no_prompt and confirm("Would you like to change the instance name?", default=False):
+        name = get_name()
+        if name in _get_instance_names():
+            print(
+                "WARNING: An instance already exists with this name. "
+                "Continuing will overwrite the existing instance config."
+            )
+            if not confirm(
+                "Are you absolutely certain you want to continue with this instance name?",
+                default=False,
+            ):
+                print("Instance name will remain unchanged.")
+                name = old_name
+            else:
+                print("Instance name updated.")
+            print()
+    else:
+        name = old_name
+    return name
+
+
+def _edit_data_path(data, data_path, copy_data, no_prompt):
+    # This modifies the passed dict.
+    if data_path:
+        data["DATA_PATH"] = data_path
+        if copy_data and not _copy_data(data):
+            print("Can't copy data to non-empty location. Data location will remain unchanged.")
+            data["DATA_PATH"] = data_manager.basic_config["DATA_PATH"]
+    elif not no_prompt and confirm("Would you like to change the data location?", default=False):
+        data["DATA_PATH"] = get_data_dir()
+        if confirm(
+            "Do you want to copy the data from old location?", default=True
+        ) and not _copy_data(data):
+            print("Can't copy the data to non-empty location.")
+            if not confirm("Do you still want to use the new data location?"):
+                data["DATA_PATH"] = data_manager.basic_config["DATA_PATH"]
+                print("Data location will remain unchanged.")
+            else:
+                print("Data location updated.")
+
+
+def _copy_data(data):
+    if Path(data["DATA_PATH"]).exists():
+        if any(os.scandir(data["DATA_PATH"])):
+            return False
+        else:
+            # this is needed because copytree doesn't work when destination folder exists
+            # Python 3.8 has `dirs_exist_ok` option for that
+            os.rmdir(data["DATA_PATH"])
+    shutil.copytree(data_manager.basic_config["DATA_PATH"], data["DATA_PATH"])
+    return True
+
+
 async def sigterm_handler(red, log):
     log.info("SIGTERM received. Quitting...")
     await red.shutdown(restart=False)
@@ -79,7 +231,7 @@ def main():
         print(description)
         print("Current Version: {}".format(__version__))
         sys.exit(0)
-    elif not cli_flags.instance_name and not cli_flags.no_instance:
+    elif not cli_flags.instance_name and (not cli_flags.no_instance or cli_flags.edit):
         print("Error: No instance name was provided!")
         sys.exit(1)
     if cli_flags.no_instance:
@@ -108,6 +260,16 @@ def main():
         cli_flags=cli_flags, description=description, dm_help=None, fetch_offline_members=True
     )
     loop.run_until_complete(red._maybe_update_config())
+
+    if cli_flags.edit:
+        try:
+            edit_instance(red, cli_flags)
+        except (KeyboardInterrupt, EOFError):
+            print("Aborted!")
+        finally:
+            loop.run_until_complete(driver_cls.teardown())
+        sys.exit(0)
+
     init_global_checks(red)
     init_events(red, cli_flags)
 
@@ -154,8 +316,7 @@ def main():
         log.critical("This token doesn't seem to be valid.")
         db_token = loop.run_until_complete(red._config.token())
         if db_token and not cli_flags.no_prompt:
-            print("\nDo you want to reset the token? (y/n)")
+            if confirm("\nDo you want to reset the token?"):
-            if confirm("> "):
                 loop.run_until_complete(red._config.token.set(""))
                 print("Token has been reset.")
     except KeyboardInterrupt:

redbot/core/cli.py

@@ -1,17 +1,42 @@
 import argparse
 import asyncio
 import logging
+import sys
+from typing import Optional
 
 
-def confirm(m=""):
+def confirm(text: str, default: Optional[bool] = None) -> bool:
-    return input(m).lower().strip() in ("y", "yes")
+    if default is None:
+        options = "y/n"
+    elif default is True:
+        options = "Y/n"
+    elif default is False:
+        options = "y/N"
+    else:
+        raise TypeError(f"expected bool, not {type(default)}")
+
+    while True:
+        try:
+            value = input(f"{text}: [{options}] ").lower().strip()
+        except (KeyboardInterrupt, EOFError):
+            print("\nAborted!")
+            sys.exit(1)
+        if value in ("y", "yes"):
+            return True
+        if value in ("n", "no"):
+            return False
+        if value == "":
+            if default is not None:
+                return default
+        print("Error: invalid input")
 
 
-def interactive_config(red, token_set, prefix_set):
+def interactive_config(red, token_set, prefix_set, *, print_header=True):
     loop = asyncio.get_event_loop()
     token = ""
 
-    print("Red - Discord Bot | Configuration process\n")
+    if print_header:
+        print("Red - Discord Bot | Configuration process\n")
 
     if not token_set:
         print("Please enter a valid token:")
@@ -35,8 +60,7 @@ def interactive_config(red, token_set, prefix_set):
         while not prefix:
             prefix = input("Prefix> ")
             if len(prefix) > 10:
-                print("Your prefix seems overly long. Are you sure that it's correct? (y/n)")
+                if not confirm("Your prefix seems overly long. Are you sure that it's correct?"):
-                if not confirm("> "):
                     prefix = ""
             if prefix:
                 loop.run_until_complete(red._config.prefix.set([prefix]))
@@ -54,6 +78,37 @@ def parse_cli_flags(args):
         action="store_true",
         help="List all instance names setup with 'redbot-setup'",
     )
+    parser.add_argument(
+        "--edit",
+        action="store_true",
+        help="Edit the instance. This can be done without console interaction "
+        "by passing --no-prompt and arguments that you want to change (available arguments: "
+        "--edit-instance-name, --edit-data-path, --copy-data, --owner, --token).",
+    )
+    parser.add_argument(
+        "--edit-instance-name",
+        type=str,
+        help="New name for the instance. This argument only works with --edit argument passed.",
+    )
+    parser.add_argument(
+        "--overwrite-existing-instance",
+        action="store_true",
+        help="Confirm overwriting of existing instance when changing name."
+        " This argument only works with --edit argument passed.",
+    )
+    parser.add_argument(
+        "--edit-data-path",
+        type=str,
+        help=(
+            "New data path for the instance. This argument only works with --edit argument passed."
+        ),
+    )
+    parser.add_argument(
+        "--copy-data",
+        action="store_true",
+        help="Copy data from old location. This argument only works "
+        "with --edit and --edit-data-path arguments passed.",
+    )
     parser.add_argument(
         "--owner",
         type=int,
@@ -65,7 +120,7 @@ def parse_cli_flags(args):
         "--co-owner",
         type=int,
         default=[],
-        nargs="*",
+        nargs="+",
         help="ID of a co-owner. Only people who have access "
         "to the system that is hosting Red should be  "
         "co-owners, as this gives them complete access "
@@ -87,7 +142,7 @@ def parse_cli_flags(args):
     parser.add_argument(
         "--load-cogs",
         type=str,
-        nargs="*",
+        nargs="+",
         help="Force loading specified cogs from the installed packages. "
         "Can be used with the --no-cogs flag to load these cogs exclusively.",
     )

redbot/launcher.py

@@ -264,7 +264,7 @@ async def reset_red():
         print("Cancelling...")
         return
 
-    if confirm("\nDo you want to create a backup for an instance? (y/n) "):
+    if confirm("\nDo you want to create a backup for an instance?"):
         for index, instance in instances.items():
             print("\nRemoving {}...".format(index))
             await create_backup(index)

redbot/setup.py

@@ -53,16 +53,6 @@ def save_config(name, data, remove=False):
     if remove and name in _config:
         _config.pop(name)
     else:
-        if name in _config:
-            print(
-                "WARNING: An instance already exists with this name. "
-                "Continuing will overwrite the existing instance config."
-            )
-            if not click.confirm(
-                "Are you absolutely certain you want to continue?", default=False
-            ):
-                print("Not continuing")
-                sys.exit(0)
         _config[name] = data
 
     with config_file.open("w", encoding="utf-8") as fs:
@@ -73,12 +63,9 @@ def get_data_dir():
     default_data_dir = Path(appdir.user_data_dir)
 
     print(
-        "Hello! Before we begin the full configuration process we need to"
+        "We've attempted to figure out a sane default data location which is printed below."
-        " gather some initial information about where you'd like us"
+        " If you don't want to change this default please press [ENTER],"
-        " to store your bot's data. We've attempted to figure out a"
+        " otherwise input your desired data location."
-        " sane default data location which is printed below. If you don't"
-        " want to change this default please press [ENTER], otherwise"
-        " input your desired data location."
     )
     print()
     print("Default: {}".format(default_data_dir))
@@ -104,7 +91,7 @@ def get_data_dir():
     if not click.confirm("Please confirm", default=True):
         print("Please start the process over.")
         sys.exit(0)
-    return default_data_dir
+    return str(default_data_dir.resolve())
 
 
 def get_storage_type():
@@ -147,10 +134,15 @@ def basic_setup():
     :return:
     """
 
+    print(
+        "Hello! Before we begin the full configuration process we need to"
+        " gather some initial information about where you'd like us"
+        " to store your bot's data."
+    )
     default_data_dir = get_data_dir()
 
     default_dirs = deepcopy(data_manager.basic_config_default)
-    default_dirs["DATA_PATH"] = str(default_data_dir.resolve())
+    default_dirs["DATA_PATH"] = default_data_dir
 
     storage = get_storage_type()
 
@@ -161,6 +153,14 @@ def basic_setup():
     default_dirs["STORAGE_DETAILS"] = driver_cls.get_config_details()
 
     name = get_name()
+    if name in instance_data:
+        print(
+            "WARNING: An instance already exists with this name. "
+            "Continuing will overwrite the existing instance config."
+        )
+        if not click.confirm("Are you absolutely certain you want to continue?", default=False):
+            print("Not continuing")
+            sys.exit(0)
     save_config(name, default_dirs)
 
     print()
@@ -236,53 +236,6 @@ async def mongov1_to_json() -> Dict[str, Any]:
     return {}
 
 
-async def edit_instance():
-    _instance_list = load_existing_config()
-    if not _instance_list:
-        print("No instances have been set up!")
-        return
-
-    print(
-        "You have chosen to edit an instance. The following "
-        "is a list of instances that currently exist:\n"
-    )
-    for instance in _instance_list.keys():
-        print("{}\n".format(instance))
-    print("Please select one of the above by entering its name")
-    selected = input("> ")
-
-    if selected not in _instance_list.keys():
-        print("That isn't a valid instance!")
-        return
-    _instance_data = _instance_list[selected]
-    default_dirs = deepcopy(data_manager.basic_config_default)
-
-    current_data_dir = Path(_instance_data["DATA_PATH"])
-    print("You have selected '{}' as the instance to modify.".format(selected))
-    if not click.confirm("Please confirm", default=True):
-        print("Ok, we will not continue then.")
-        return
-
-    print("Ok, we will continue on.")
-    print()
-    if click.confirm("Would you like to change the instance name?", default=False):
-        name = get_name()
-    else:
-        name = selected
-
-    if click.confirm("Would you like to change the data location?", default=False):
-        default_data_dir = get_data_dir()
-        default_dirs["DATA_PATH"] = str(default_data_dir.resolve())
-    else:
-        default_dirs["DATA_PATH"] = str(current_data_dir.resolve())
-
-    if name != selected:
-        save_config(selected, {}, remove=True)
-    save_config(name, default_dirs)
-
-    print("Your basic configuration has been edited")
-
-
 async def create_backup(instance: str) -> None:
     data_manager.load_basic_configuration(instance)
     backend_type = get_current_backend(instance)