Add --cog-path CLI argument

[cswimr]

Description of the changes

This PR adds an argument, --cog-path, to the redbot CLI that accepts a path, validates that the provided path exists, and adds the path via self._cog_mgr.add_path(), so that cogs can be loaded from that path. The argument can be passed multiple times to add more than one path at a time. These paths are stored in memory and never to disk, so restarting the bot without the argument will "remove" the paths.

Additionally, this PR adds a persist argument to CogManager.add_path(), which defaults to True. The original behavior of appending the added path to a list and then calling .set_path() on that list is retained through this argument being set to True. Setting this argument to False will append the path to a module-level list of paths, stored in memory. These are, as I mentioned earlier, never written to the bot's config, and are effectively discarded on shutdown.

Closes #6506.

Have the changes in this PR been tested?

Yeah, but not super thoroughly

[Flame442]

This PR does not behave as outlined in the issue. self._cog_mgr.add_path() is the method used by [p]addpath to add a new permanent cog path to the bot. The issue described this flag as a temporary path which would only exist as long as the path was present. The method does not duplicate paths, so it is not a huge problem, but if we want this flag to specifically be for temporary paths then a new method for doing so will be needed.

[cswimr]

Oops, good catch! I'll take another look at this.

[cswimr]

So, I propose the following:
Temporary paths would be stored in config, separately from ones added via [p]addpath. This is the safest place that I can think of to store them. When the bot is started, that config value would be overwritten with the contents (if any) of the --cog-path CLI argument. If nothing is passed to --cog-path, then the config value is emptied. A new function would have to be added to CogManager to retrieve these temporary paths, and CogManagerUI would have to be updated to check this in [p]paths. Is there a better way Red has to do something like this? It feels kind of clunky imo but it'd work I think.

[Flame442]

Why not just store it in memory during the run the flag is used? Why does the disk need to be used at all?

[cswimr]

Why not just store it in memory during the run the flag is used? Why does the disk need to be used at all?

Latest commit stores it in memory. Assuming I didn't miss anything, should be ready for review now.

[Flame442]

[p]removepath (and by extension, bot._cog_mgr.remove_path abd .add_path) has incorrect behavior with the way you have temporarily added the path. These methods pull from bot._cog_mgr.user_defined_paths to get the list of user-defined paths to modify. By including the temporary paths in this method's output, they will attempt to append to and remove from the list containing the temporary paths, then set the list of user paths including any temporary paths that remain after that change. This will cause those remaining paths to become permanent paths.

All of the frontend (CogManagerUI cog) and backend (CogManager class) logic needs to properly handle not only processing both permanent and temporary user-defined paths, but also listing and modifying them.

[cswimr]

[p]removepath (and by extension, bot._cog_mgr.remove_path abd .add_path) has incorrect behavior with the way you have temporarily added the path. These methods pull from bot._cog_mgr.user_defined_paths to get the list of user-defined paths to modify. By including the temporary paths in this method's output, they will attempt to append to and remove from the list containing the temporary paths, then set the list of user paths including any temporary paths that remain after that change. This will cause those remaining paths to become permanent paths.

Good catch, fixed this in the latest commit.

All of the frontend (CogManagerUI cog) and backend (CogManager class) logic needs to properly handle not only processing both permanent and temporary user-defined paths, but also listing and modifying them.

Updated in latest commit to make [p]paths list temporary paths - this comes with a change to a localization string, not sure how to regenerate locale files if necessary? Is it just redgettext, same as for a third party cog?

Out of curiosity, why should temporary paths be modifiable through Discord? I struggle to see a reason to use this feature in that way, as you would need to re-run the command to add a temporary path after each time the bot restarts. This would also result in any cogs loaded from a previously set temporary path failing to load on restart. With a CLI argument, you can just have your service runner (systemd, docker, etc.) use the argument every time it starts the bot. The temporary paths list is also populated before cogs begin loading at startup when using the CLI argument.

[Flame442]

This comes with a change to a localization string, not sure how to regenerate locale files if necessary?

.pot files are automatically generated and pushed to crowdin using a branch push workflow. Our release workflow handles pulling localized strings from crowdin.

Out of curiosity, why should temporary paths be modifiable through Discord?

In an attempt to speak succinctly, I said something I did not mean. What I should have said is that all frontend and backend logic needs to be updated such that it does not cause unexpected behavior when temporary paths are present. This includes logic that is currently used to modify paths, as it had the incorrect behavior I described. I agree that there is no reason to allow modifying temporary paths within discord.

[Flame442]

Thank you for your contribution!