Nice help

[qsantos]

A nicer and more intuitive help function

[elad661]

Since you didn't touch reload, reloading/loading modules won't change the help. Wouldn't it be better to have module loading update a dict of help messages instead of making the modules available at runtime and iterating on them?

[elad661]

Overall, looks good. Apart from my minor nitpicks above (note they refer to specific lines) and the reload/load thing, it's almost good to be merged.

It's really awesome and it will greatly improve the user experience of the bot.

[qsantos]

Thanks for the comments, I'll look into that.

I was assuming a particular list of mods, but obviously, ensure the presence of a space is easy. I tried combining spaces for CLI clients and '\t' for GUI clients, but '\t' appears weird in CLI (in my case irssi shows an "I" on white background).

I am fixing the situation for module loading anyway.

[elad661]

No, sorry, load() and unload() is not something we want. When I talked about reloading module I talked about reload.py that is in the modules directory. This just overcomplicates everything and makes it harder to review.

We don't want an unload method because there's no real safe way to unload things and having such method might cause miscommunications about it. Even reload is presented as "it might work, but in some cases it will be buggy and you'd have to restart".

[elad661]

Let me rephrase the requirements for this to be merged, just so it'll be clear:

1. no unload() or load() methods in core.
2. have some dict in bot.py to keep track of the help messages & module categories
3. bot.register() and bot.unregister() should handle updating said dict.
4. Don't make list of modules available at runtime unless there's a good reason for it.
5. Focus on the help feature in this pull request. removing code duplication from module loading is a possible thing that can be done, but in a separate pull request

[elad661]

I want this in for 5.0... Are you going to work on this or should I do it myself?

[embolalia]

@qsantos are you going to address the above?

[qsantos]

Sorry for the long wait. I have removed the commits that were off-topic and use a dictionnary of available commands per module instead of a dictionnary of modules.

Also, *register() takes the list of symbols as a parameter whereas I need the module name. I have thus added *register_help() which just takes the module. An alternative would be to change *register() so that they take a module as a parameter.

[embolalia]

@qsantos: Sorry for leaving this open for so long. The change has been implemented and will be in 6.0. Thanks for the contribution.