First off, thanks for the module!

I'm using this to run two bots now, one on #foo, and one on #foo-private. The problem is that going to http://example.com/bot/log/foo shows logs from BOTH channels.

The patch attached fixes this by adding a $ to the end of the REGEX in the SQL statement.

CommentFileSizeAuthor
#3 correct_logs3.patch824 bytesSteven Merrill
#1 correct_logs2.patch819 bytesSteven Merrill
correct_logs.patch863 bytesSteven Merrill

Comments

Steven Merrill’s picture

Steven Merrill commented
StatusFileSize
new correct_logs2.patch819 bytes

Oops. New patch against the real, released 6.x-1.0 version. In any case, it's a one-character change.

morbus iff’s picture

morbus iff
they/them
commented
Status: Needs review » Needs work

Unfortunately, this won't work. Consider a user who is in five channels and then QUITs. What is stored in the database's 'channel' column is "#channel1 #channel2 #channel3 #channel4 #channel5". Now, with your patch, if someone heads to the web-based logs for #channel3, they wouldn't get the QUIT message - it would appear as if the user never left the channel. This isn't that big of a deal, I heartily agree, but I think the stronger patch would be "(\s|$)" instead of "$" (untested). This way, we'd catch #channel3 in the above example, but also #channel5 (assuming that log was called, of course).

Thoughts?

Steven Merrill’s picture

Steven Merrill commented
Status: Needs work » Needs review
StatusFileSize
new correct_logs3.patch824 bytes

Morbus,

That appears to work fine - I still get the proper messages on each channel when someone quits both, but no message pollution.

Here it is, re-rolled by hand.

And again, thanks - this is a great module!

Steven Merrill’s picture

Steven Merrill commented
Status: Needs review » Needs work

Apologies - I was having a bit of a blonde moment.

I'm now _not_ getting quit messages on both channels, but no more message pollution.

morbus iff’s picture

morbus iff
they/them
commented

My bad - I wasn't immediately clear. Try .*#%s(\s|$) for the *entirety* of the regexp (your current patch has a " ?" at the end of it which I suspect is causing the problem. If that doesn't work, I'll have to take a closer look at what's going on a bit later. Warhammer Online is calling ;)

morbus iff’s picture

morbus iff
they/them
commented
Status: Needs work » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.