addressed shouldn't be required in PMs
Morbus Iff - July 11, 2008 - 11:36
| Project: | Bot |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Description
Some of our features require addressing (BOTNAME: [feature]) when in a public channel. This forced addressing should NOT be required, however, when it's happening through a PM. Likewise, various Druplicon responses prepend the nick in public channels - it shouldn't need to do that for PMs either.
#1
subscribe (will take a look at this when i get back home).
From Morbus on IRC: "probably a wipe on $addressed if $from_query = TRUE".
#2
This could be implemented on a per module basis, because when one preg_match-es the user entered line, then its important to know if $matched was eventually there, or not. Example:
<?php
function bot_search_irc_msg_channel($data, $from_query = FALSE) {
$to = $from_query ? $data->nick : $data->channel;
$addressed = $from_query ? '' : bot_name_regexp();
$botcall = 'search';
if (preg_match("/^". $addressed . $botcall ."\s(.*?)$/i", $data->message, $matches)) {
if ($addressed) {
bot_search_perform_search($matches[2], $from_query, $to);
}
else {
bot_search_perform_search($matches[1], $from_query, $to);
}
}
return;
}
?>
This one works fine, thanks for the hint!
#3
First take. I left the conditions a bit more verbouse than would perhaps be ideal for the sake of testing and clarity. Tested with factoids in pm/public, and help.
Rest of factoids and modules is yet untouched.
#4
Snufkin: I don't think I like the fact that we need an if/else whenever we do this trick, due to the fact that the $matches count changes - it's just a lot of duplicate code with just ints changing. Instead of returning '', can we return (()) instead, which should keep all code the same? And, should we instead pass $from_query to bot_name_regexp() itself, to let that function determine what to return?
#5
passing $from_query to bot_name_regexp: definitely a good point there, I'll rewrite it accordingly. Should make the code a lot cleaner at the start of the msg_query.
returning () is a good idea, will do and reroll.
#6
Be sure that it returns (()) - there's two captures in the returned regexp.
#7
No, there's not. Nevermind. Heh.
#8
Although some modules use the $addressed inconsistently, some use it in capture, like
"($addressed)", some do${addressed}. This might be because we didnt know for sure if $messaged contains () or not. But we do know now!tell about stg doesnt work in pm mode still for some reason. Didn't want to dig deeper for the other supplied modules until you say go on the implementation.
#9
This is looking good, yep - I'm wondering why I did use ($addressed) at some times and {$addressed} at others. Not entirely sure. I'll have to test things heavily to ensure happiness.
#10
I think you placed () around $addressed when you wanted it to be optional, like in bot_agotchi.module:
<?phpif (preg_match("/^($addressed)?(bot(\s|\-)?snack)/i", $data->message)) {
?>
Elsewhere it wasnt {$addressed}, but ${addressed}. I think the reason could've been not to break the flow regexp string (strictly DX only, functionality would've been the same), because like this you can write
"${addressed}karma"instead of"/^". $addressed ."karma..".#11
Replaced all instances of
($addressed)to${addressed}(style for consistency), and added the $from_query to the bot_name_regexp. Haven't tested yet.#12
Yeah, I suspect the ($addressed) are a legacy from before the bot_name_regexp() function came around - there's no real reason for ($addressed)? when {$addressed}? will work just fine.
#13
Getting this work for factoids is quite a pain, I wonder if its a good idea at all.
Nevertheless seen and karma work now, although for karma i had to use
(${addressed})to have it correctly captured.