diff --git a/changelog.d/3171.bugfix.rst b/changelog.d/3171.bugfix.rst new file mode 100644 index 000000000..97c1ff160 --- /dev/null +++ b/changelog.d/3171.bugfix.rst @@ -0,0 +1 @@ +Handle invalid folder names for data path gracefully in ``redbot-setup`` and ``redbot --edit``. \ No newline at end of file diff --git a/changelog.d/3171.enhance.1.rst b/changelog.d/3171.enhance.1.rst new file mode 100644 index 000000000..6634c4547 --- /dev/null +++ b/changelog.d/3171.enhance.1.rst @@ -0,0 +1 @@ +``redbot-setup`` will now use instance name in default data path to avoid creating second instance with same data path. \ No newline at end of file diff --git a/changelog.d/3171.enhance.2.rst b/changelog.d/3171.enhance.2.rst new file mode 100644 index 000000000..ef0948c40 --- /dev/null +++ b/changelog.d/3171.enhance.2.rst @@ -0,0 +1 @@ +Instance names can now only include characters A-z, numbers, underscores, and hyphens. Old instances are unaffected by this change. \ No newline at end of file diff --git a/redbot/__main__.py b/redbot/__main__.py index bd0ee2511..2fe86a3a9 100644 --- a/redbot/__main__.py +++ b/redbot/__main__.py @@ -130,7 +130,7 @@ async def edit_instance(red, cli_flags): 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) + _edit_data_path(data, name, data_path, copy_data, no_prompt) save_config(name, data) if old_name != name: @@ -208,30 +208,50 @@ def _edit_instance_name(old_name, new_name, confirm_overwrite, no_prompt): name = old_name else: print("Instance name updated.") - print() + else: + print("Instance name updated.") + print() else: name = old_name return name -def _edit_data_path(data, data_path, copy_data, no_prompt): +def _edit_data_path(data, instance_name, data_path, copy_data, no_prompt): # This modifies the passed dict. if data_path: + new_path = Path(data_path) + try: + exists = new_path.exists() + except OSError: + print( + "We were unable to check your chosen directory." + " Provided path may contain an invalid character." + " Data location will remain unchanged." + ) + + if not exists: + try: + new_path.mkdir(parents=True, exist_ok=True) + except OSError: + print( + "We were unable to create your chosen directory." + " Data location will remain unchanged." + ) 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.") + data["DATA_PATH"] = get_data_dir(instance_name) + if confirm("Do you want to copy the data from old location?", default=True): + if 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.") + return + print("Old data has been copied over to the new location.") + print("Data location updated.") def _copy_data(data): diff --git a/redbot/setup.py b/redbot/setup.py index 1ab706bff..ebec96f17 100644 --- a/redbot/setup.py +++ b/redbot/setup.py @@ -4,6 +4,7 @@ import json import logging import os import sys +import re from copy import deepcopy from pathlib import Path from typing import Dict, Any, Optional @@ -59,26 +60,35 @@ def save_config(name, data, remove=False): json.dump(_config, fs, indent=4) -def get_data_dir(): - default_data_dir = Path(appdir.user_data_dir) +def get_data_dir(instance_name: str): + data_path = Path(appdir.user_data_dir) / "data" / instance_name + print() print( "We've attempted to figure out a 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)) + print("Default: {}".format(data_path)) - new_path = input("> ") + data_path_input = input("> ") - if new_path != "": - new_path = Path(new_path) - default_data_dir = new_path + if data_path_input != "": + data_path = Path(data_path_input) - if not default_data_dir.exists(): + try: + exists = data_path.exists() + except OSError: + print( + "We were unable to check your chosen directory." + " Provided path may contain an invalid character." + ) + sys.exit(1) + + if not exists: try: - default_data_dir.mkdir(parents=True, exist_ok=True) + data_path.mkdir(parents=True, exist_ok=True) except OSError: print( "We were unable to create your chosen directory." @@ -87,11 +97,11 @@ def get_data_dir(): ) sys.exit(1) - print("You have chosen {} to be your data directory.".format(default_data_dir)) + print("You have chosen {} to be your data directory.".format(data_path)) if not click.confirm("Please confirm", default=True): print("Please start the process over.") sys.exit(0) - return str(default_data_dir.resolve()) + return str(data_path.resolve()) def get_storage_type(): @@ -113,16 +123,21 @@ def get_storage_type(): return storage -def get_name(): +def get_name() -> str: name = "" while len(name) == 0: - print() print( - "Please enter a name for your instance, this name cannot include spaces" - " and it will be used to run your bot from here on out." + "Please enter a name for your instance," + " it will be used to run your bot from here on out.\n" + "This name is case-sensitive and can only include characters" + " A-z, numbers, underscores, and hyphens." ) name = input("> ") - if " " in name: + if re.fullmatch(r"[a-zA-Z0-9_\-]*", name) is None: + print( + "ERROR: Instance name can only include" + " characters A-z, numbers, underscores, and hyphens!" + ) name = "" return name @@ -134,11 +149,11 @@ def basic_setup(): """ 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." + "Hello! Before we begin, we need to gather some initial information for the new instance." ) - default_data_dir = get_data_dir() + name = get_name() + + default_data_dir = get_data_dir(name) default_dirs = deepcopy(data_manager.basic_config_default) default_dirs["DATA_PATH"] = default_data_dir @@ -151,7 +166,6 @@ def basic_setup(): driver_cls = drivers.get_driver_class(storage_type) 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. "