[Setup] Use instance name in default data path (#3171)

* enhance(setup): use instance name in default data path

* chore(changelog): add towncrier entries

* enhance(setup): tell user that instance name is case-sensitive

changelog.d/3171.bugfix.rst

@@ -0,0 +1 @@
+Handle invalid folder names for data path gracefully in ``redbot-setup`` and ``redbot --edit``.

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.

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.

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,29 +208,49 @@ def _edit_instance_name(old_name, new_name, confirm_overwrite, no_prompt):
                 name = old_name
             else:
                 print("Instance name updated.")
+        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()
+        data["DATA_PATH"] = get_data_dir(instance_name)
-        if confirm(
+        if confirm("Do you want to copy the data from old location?", default=True):
-            "Do you want to copy the data from old location?", default=True
+            if not _copy_data(data):
-        ) 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:
+                    return
+            print("Old data has been copied over to the new location.")
         print("Data location updated.")
 
 

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():
+def get_data_dir(instance_name: str):
-    default_data_dir = Path(appdir.user_data_dir)
+    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 != "":
+    if data_path_input != "":
-        new_path = Path(new_path)
+        data_path = Path(data_path_input)
-        default_data_dir = new_path
 
-    if not default_data_dir.exists():
     try:
-            default_data_dir.mkdir(parents=True, exist_ok=True)
+        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:
+            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"
+            "Please enter a name for your instance,"
-            " and it will be used to run your bot from here on out."
+            " 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"
+        "Hello! Before we begin, we need to gather some initial information for the new instance."
-        " gather some initial information about where you'd like us"
-        " to store your bot's data."
     )
-    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. "