-
-
Notifications
You must be signed in to change notification settings - Fork 402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
help: add config to hide server host from help listing #1459
Conversation
Fiiinee.. |
bd837da
to
1c30b70
Compare
d9816b8
to
1f87da9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring nitpicks, and a proposal for the setting cache. I'm mixing it up a bit…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I think going without a shutdown()
method that removes the setting cache from memory makes sense. Reloading the module shouldn't reset the cached setting values, nor the cached pastebin URL. It makes sense to me right now, and it can easily be changed later if it stops making sense.
Maybe just fix the commit message format since the original and revising commits were made so far apart. Squashing would destroy that bit of history (feature development timeline), so I'm not actually inclined to ask for a straight-up rebase this time around.
e273764
to
3803366
Compare
Done. |
Bleh, this needs rebasing. :/ It's #1451 what did it. |
90e2552
to
2f47078
Compare
Boy, that was a fun one 🙃 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If only I'd just hit "Merge" and not gone back for one final check to make sure the rebase didn't break anything obvious… Now we have more change requests. I guess @HumorBaby won't be bored today!
Adds a config section to help.py that has a setting to show the server host in the first line of a full help listing. Defaults to true. Additionally implements cache validation accounting for changes to config during runtime.
Co-Authored-By: dgw <dgw@technobabbl.es>
2f47078
to
232bb09
Compare
Can confirm... was not bored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yerp, gerd ter ger.
Regression introduced by #1459 (`dict.iteritems()` no longer exists in Python 3; only `dict.items()` does, with the same internal behavior as what `dict.iteritems()` did in Python 2)
I have been disabling the
help
module because it shows the name of my server in a full.help
listing. This would be okay if the bot connected directly to a public server, but if it goes through a private bouncer, it may not be desirable to share this information.Of course I can modify
help.py
to prevent this, but I thought "Why not make it configurable?". So, here it is 🎉A config section is defined for the
help
module (section calledhelp
, of course), which for now, only has one setting:show_server_host
, a boolean that determines whether the server name will be printed or not; e.g.,Command listing for ro-bot-o@znc.roboto.org
(true
) vsCommand listing for ro-bot-o
(false
). Aconfigure
method was also added to provide config wizard support.This required implementing cache validation of a sorts because a change in the setting would not have trigger a new help listing to be generated unless the total number of available commands changed somehow. I think this caching bust/validation implementation could also help #1451 if still needed.