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

Add irclog search. #5

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

Add irclog search. #5

wants to merge 1 commit into from

Conversation

xet7
Copy link
Member

@xet7 xet7 commented Aug 17, 2015

@Quintus could you review this addition of irclog search?
For example, should I use :plainlogdir setting instead of direct path to plainlogs in code?
The same logic works as shell script.
I also tried it with furbot/cinch on my laptop where furbot did load this code, but I had this network error that I could not solve:
Could not connect to the IRC server. Please check your network: getaddrinfo: Name or service not known

if (completelogline1[logsearchtext1])
logresultcounter1 = logresultcounter1 + 1
msg.channel.send("#{completelogline1}")
exit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A call to #exit will terminate the program (i.e. make furbot quit). I don’t think this is a good idea to use.

@Quintus
Copy link
Member

Quintus commented Aug 17, 2015

For example, should I use :plainlogdir setting instead of direct path to plainlogs in code?

That’d be preferrable in case we ever move the directory around. You’ll have to look it up in Cinch’s docs how to retrieve it, though.

I made some notices on some lines of your code, so be sure to review those.

For testing your changes, you can install a local IRC daemon. I use ngircd, which is very easy to configure.

Vale,
Quintus

@Quintus
Copy link
Member

Quintus commented Aug 17, 2015

It might also be interesting to directly add such functionality to the logplus plugin, as then all logging functionality is capsulated in one plugin rather than scattered around. Then it wil also be easier to retrieve the configuration setting.

Vale,
Quintus

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