Skip to content
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

Google module is broken #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Google module is broken #16

wants to merge 1 commit into from

Conversation

nilicule
Copy link

Google module is not returning results. Fixed.

@cjones
Copy link
Owner

cjones commented Feb 17, 2014

heya, just saw this. can you explain better what needed fixing? this seems to have just disabled the addressing requirement for this module (which requires you address the bot, in other words say, "botname: google foo" instead of just "google foo"). I'm open for making this a config option if you prefer it that way, but turning it off completely isn't gonna work as a general approach because a number of people do not want the bot triggering on accident, especially in work irc situations. Also worth pointing out, I think, that all modules that output a response to chat require addressing in the same way (save but a few exceptional cases like jinx.py and factoids.py, both of which come with a "spaminess is implied" factor in a way google.py definitely doesn't).

sorry if missing something beyond that, you'll have to elaborate on how it's broken if that wasn't the salient change.

@nilicule
Copy link
Author

On Tue, Feb 18, 2014 at 12:44 AM, Chris Jones [email protected]:

heya, just saw this. can you explain better what needed fixing? this seems
to have just disabled the addressing requirement for this module (which
requires you address the bot, in other words say, "botname: google foo"
instead of just "google foo"). I'm open for making this a config option if
you prefer it that way, but turning it off completely isn't gonna work as a
general approach because a number of people do not want the bot triggering
on accident, especially in work irc situations.

Ideally I'd have this as a config option. One of the channels I run this
bot in has the bot set up to react without being addressed and this is the
one feature that doesn't seem to trigger without the patch I supplied.

Having this as a config option would allow for both situations to work, so
that would be perfect indeed.

Also worth pointing out, I think, that all modules that output a response
to chat require addressing in the same way (save but a few exceptional
cases like jinx.py and factoids.py, both of which come with a "spaminess is
implied" factor in a way google.py definitely doesn't).

Correct, didn't notice that before because those modules aren't enabled in
my case.

regards,
Remco

@cjones
Copy link
Owner

cjones commented Mar 9, 2015

Sorry for the late reply, very busy with a startup these days, and all the email notifications go to my spam-overrun gmail.

So, I think I'm still missing something here. Can you elaborate a bit on what you did to avoid the addressing requirement with other modules? Just looking real quick at the google.py module on its own, I don't think it's different than any other module with require_addressing set. I actually thought I had a config option to turn off that requirement entirely, but I don't see it anywhere in my own config, so I maybe just meant to do it and never did, or... something. I have some ideas for how this can be done easily, but feel like I possibly don't have all the facts, since afaik every module except for a few special ones behave the same way as the google one. Is it just that you only use a few modules?

(You probably know this, but on the off- chance you don't: you can set easier-to-type aliases for the nick which count as addressing. Most people use ! or . -- or both, even -- so it is just "!google " to trigger, for example, a google search).

(Which leads me to a thought: You might try putting the empty string as an alias and see what that does..I honestly don't know, but feel like it should have the effect of disabling aliasing requirement, logically, by matching every line. If it doesn't work that way, it's likely something like a "if alias: do_alias_handling" check that can be removed, as long as it treated an empty array of aliases as "no aliases" still.

@cjones cjones force-pushed the master branch 2 times, most recently from e94c6c5 to 6f8257b Compare May 31, 2015 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants