-
Notifications
You must be signed in to change notification settings - Fork 294
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
Add setting to disable IRC- and/or Discord-style mentions #537
base: main
Are you sure you want to change the base?
Conversation
@CVSoft in case you still need it |
@Throne3d I know you no longer add code, but this is merge-ready, if you want :) |
This seems simple enough for me - if there is enough interest it should probably be merged, although I'm not really a fan of the names of the new config options. You should, however, update the README as well, advanced options under Example configuration. |
I admit I struggled with the option names; suggestions welcome? :/ |
Naming wise, try to KISS: it could be as simple as allowMentionsWithAtPrefix and allowMentionsWithoutPrefix. However, those names have made me start thinking and propose an alternate solution. I would love your input. (This does not mean I do not approve of your current PR, it solves a problem aptly). This is what this current PR does:The problem you want to solve is:
Your solutioning is:
As a nice-to-have that matches with it:
Possible alternative solutionNow, I'm just going to 'go wild' here and toss out a different possible approach - I would like your input on it as well:
Proposed solutioning
I would imagine the default configuration could look something like this: {
"mentionPrefixes": [
"",
"@"
]
} ConclusionJust some food for thought. This would offer a lot more flexibility in the configuration instead of creating a hard-coded case for every commonly-asked for (combination of..) prefixes (or lack thereof) to mention people with. |
The problem is that "prefix" is misleading, the format it's using isn't just |
Perhaps instead of |
To throw my apprecation into this; Needed exactly this feature, so compiled it into a docker image myself to run it. 😅 |
Hey! Sorry to not have responded in a while – I haven't been tracking this repository very well. I really appreciate the contribution, especially the added tests. I see that there's still some open discussion here, so I'm not going to merge it immediately. (I know it's not a huge difference either way, but still.) If this comes to a consensus I can try to get to merging it then! |
For what it's worth we've been running with this for one of our bots and it's working brilliantly. |
This fixes #528 which is really annoying.