[Core] Fix user output on cog load/reload (#2767)

* [Core] Fix user output on cog load/reload

  - Properly fixes the load/reload exception handling
  - Fixes some i18n use here to not make assumptions about other
  languages pluralization rules.

* Fix some typos

* Address Flame's Feedback

* It's important to save before committing ...

* formatting

* Fix some formats...

redbot/core/bot.py

@@ -303,7 +303,7 @@ class RedBase(commands.GroupMixin, commands.bot.BotBase, RPCMixin):  # pylint: d
         except Exception as e:
             self._remove_module_references(lib.__name__)
             self._call_module_finalizers(lib, name)
-            raise errors.CogLoadError(e) from e
+            raise
         else:
             self._BotBase__extensions[name] = lib
 

redbot/core/core_commands.py

@@ -34,9 +34,9 @@ from redbot.core import (
 )
 from .utils.predicates import MessagePredicate
 from .utils.chat_formatting import humanize_timedelta, pagify, box, inline, humanize_list
-
 from .commands.requires import PrivilegeLevel
 
+
 if TYPE_CHECKING:
     from redbot.core.bot import Red
 
@@ -149,25 +149,6 @@ class CoreLogic:
         for child_name, lib in children.items():
             importlib._bootstrap._exec(lib.__spec__, lib)
 
-    @staticmethod
-    def _get_package_strings(
-        packages: List[str], fmt: str, other: Optional[Tuple[str, ...]] = None
-    ) -> str:
-        """
-        Gets the strings needed for the load, unload and reload commands
-        """
-        packages = [inline(name) for name in packages]
-
-        if other is None:
-            other = ("", "")
-        plural = "s" if len(packages) > 1 else ""
-        use_and, other = ("", other[0]) if len(packages) == 1 else (" and ", other[1])
-        packages_string = ", ".join(packages[:-1]) + use_and + packages[-1]
-
-        form = {"plural": plural, "packs": packages_string, "other": other}
-        final_string = fmt.format(**form)
-        return final_string
-
     async def _unload(self, cog_names: Iterable[str]) -> Tuple[List[str], List[str]]:
         """
         Unloads cogs with the given names.
@@ -586,38 +567,64 @@ class Core(commands.Cog, CoreLogic):
         async with ctx.typing():
             loaded, failed, not_found, already_loaded, failed_with_reason = await self._load(cogs)
 
+        output = []
+
         if loaded:
-            fmt = _("Loaded {packs}.")
+            loaded_packages = humanize_list([inline(package) for package in loaded])
-            formed = self._get_package_strings(loaded, fmt)
+            formed = _("Loaded {packs}.").format(packs=loaded_packages)
-            await ctx.send(formed)
+            output.append(formed)
 
         if already_loaded:
-            fmt = _("The package{plural} {packs} {other} already loaded.")
+            if len(already_loaded) == 1:
-            formed = self._get_package_strings(already_loaded, fmt, (_("is"), _("are")))
+                formed = _("The following package is already loaded: {pack}").format(
-            await ctx.send(formed)
+                    pack=inline(already_loaded[0])
+                )
+            else:
+                formed = _("The following packages are already loaded: {packs}").format(
+                    packs=humanize_list([inline(package) for package in already_loaded])
+                )
+            output.append(formed)
 
         if failed:
-            fmt = _(
+            if len(failed) == 1:
-                "Failed to load package{plural} {packs}. Check your console or "
+                formed = _(
-                "logs for details."
+                    "Failed to load the following package: {pack}."
-            )
+                    "\nCheck your console or logs for details."
-            formed = self._get_package_strings(failed, fmt)
+                ).format(pack=inline(failed[0]))
-            await ctx.send(formed)
+            else:
+                formed = _(
+                    "Failed to load the following packages: {packs}"
+                    "\nCheck your console or logs for details."
+                ).format(packs=humanize_list([inline(package) for package in failed]))
+            output.append(formed)
 
         if not_found:
-            fmt = _("The package{plural} {packs} {other} not found in any cog path.")
+            if len(not_found) == 1:
-            formed = self._get_package_strings(not_found, fmt, (_("was"), _("were")))
+                formed = _("The following package was not found in any cog path: {pack}.").format(
-            await ctx.send(formed)
+                    pack=inline(not_found[0])
+                )
+            else:
+                formed = _(
+                    "The following packages were not found in any cog path: {packs}"
+                ).format(packs=humanize_list([inline(package) for package in not_found]))
+            output.append(formed)
 
         if failed_with_reason:
-            fmt = _(
-                "{other} package{plural} could not be loaded for the following reason{plural}:\n\n"
-            )
             reasons = "\n".join([f"`{x}`: {y}" for x, y in failed_with_reason])
-            formed = self._get_package_strings(
+            if len(failed_with_reason) == 1:
-                [x for x, y in failed_with_reason], fmt, (_("This"), _("These"))
+                formed = _(
-            )
+                    "This package could not be loaded for the following reason:\n\n{reason}"
-            await ctx.send(formed + reasons)
+                ).format(reason=reasons)
+            else:
+                formed = _(
+                    "These packages could not be loaded for the following reasons:\n\n{reasons}"
+                ).format(reasons=reasons)
+            output.append(formed)
+
+        if output:
+            total_message = "\n\n".join(output)
+            for page in pagify(total_message):
+                await ctx.send(page)
 
     @commands.command()
     @checks.is_owner()
@@ -628,15 +635,34 @@ class Core(commands.Cog, CoreLogic):
         cogs = tuple(map(lambda cog: cog.rstrip(","), cogs))
         unloaded, failed = await self._unload(cogs)
 
+        output = []
+
         if unloaded:
-            fmt = _("Package{plural} {packs} {other} unloaded.")
+            if len(unloaded) == 1:
-            formed = self._get_package_strings(unloaded, fmt, (_("was"), _("were")))
+                formed = _("The following package was unloaded: {pack}.").format(
-            await ctx.send(formed)
+                    pack=inline(unloaded[0])
+                )
+            else:
+                formed = _("The following packages were unloaded: {packs}.").format(
+                    packs=humanize_list([inline(package) for package in unloaded])
+                )
+            output.append(formed)
 
         if failed:
-            fmt = _("The package{plural} {packs} {other} not loaded.")
+            if len(failed) == 1:
-            formed = self._get_package_strings(failed, fmt, (_("is"), _("are")))
+                formed = _("The following package was not loaded: {pack}.").format(
-            await ctx.send(formed)
+                    pack=inline(failed[0])
+                )
+            else:
+                formed = _("The following packages were not loaded: {packs}.").format(
+                    packs=humanize_list([inline(package) for package in failed])
+                )
+            output.append(formed)
+
+        if output:
+            total_message = "\n\n".join(output)
+            for page in pagify(total_message):
+                await ctx.send(page)
 
     @commands.command(name="reload")
     @checks.is_owner()
@@ -650,30 +676,53 @@ class Core(commands.Cog, CoreLogic):
                 cogs
             )
 
+        output = []
+
         if loaded:
-            fmt = _("Package{plural} {packs} {other} reloaded.")
+            loaded_packages = humanize_list([inline(package) for package in loaded])
-            formed = self._get_package_strings(loaded, fmt, (_("was"), _("were")))
+            formed = _("Reloaded {packs}.").format(packs=loaded_packages)
-            await ctx.send(formed)
+            output.append(formed)
 
         if failed:
-            fmt = _("Failed to reload package{plural} {packs}. Check your logs for details")
+            if len(failed) == 1:
-            formed = self._get_package_strings(failed, fmt)
+                formed = _(
-            await ctx.send(formed)
+                    "Failed to reload the following package: {pack}."
+                    "\nCheck your console or logs for details."
+                ).format(pack=inline(failed[0]))
+            else:
+                formed = _(
+                    "Failed to reload the following packages: {packs}"
+                    "\nCheck your console or logs for details."
+                ).format(packs=humanize_list([inline(package) for package in failed]))
+            output.append(formed)
 
         if not_found:
-            fmt = _("The package{plural} {packs} {other} not found in any cog path.")
+            if len(not_found) == 1:
-            formed = self._get_package_strings(not_found, fmt, (_("was"), _("were")))
+                formed = _("The following package was not found in any cog path: {pack}.").format(
-            await ctx.send(formed)
+                    pack=inline(not_found[0])
+                )
+            else:
+                formed = _(
+                    "The following packages were not found in any cog path: {packs}"
+                ).format(packs=humanize_list([inline(package) for package in not_found]))
+            output.append(formed)
 
         if failed_with_reason:
-            fmt = _(
-                "{other} package{plural} could not be reloaded for the following reason{plural}:\n\n"
-            )
             reasons = "\n".join([f"`{x}`: {y}" for x, y in failed_with_reason])
-            formed = self._get_package_strings(
+            if len(failed_with_reason) == 1:
-                [x for x, y in failed_with_reason], fmt, (_("This"), _("These"))
+                formed = _(
-            )
+                    "This package could not be reloaded for the following reason:\n\n{reason}"
-            await ctx.send(formed + reasons)
+                ).format(reason=reasons)
+            else:
+                formed = _(
+                    "These packages could not be reloaded for the following reasons:\n\n{reasons}"
+                ).format(reasons=reasons)
+            output.append(formed)
+
+        if output:
+            total_message = "\n\n".join(output)
+            for page in pagify(total_message):
+                await ctx.send(page)
 
     @commands.command(name="shutdown")
     @checks.is_owner()