-
Notifications
You must be signed in to change notification settings - Fork 379
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
Log user quit messages #1395
Log user quit messages #1395
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1395 +/- ##
==========================================
- Coverage 40.82% 40.79% -0.03%
==========================================
Files 109 109
Lines 21777 21785 +8
==========================================
- Hits 8890 8888 -2
- Misses 12887 12897 +10
Continue to review full report at Codecov.
|
An alternative way to handle this would be to add a new module hook that fires when the user quits, something like |
Looks like |
modules/log.cpp
Outdated
@@ -114,6 +114,8 @@ class CLogMod : public CModule { | |||
const vector<CChan*>& vChans) override; | |||
EModRet OnTopic(CNick& Nick, CChan& Channel, CString& sTopic) override; | |||
|
|||
EModRet OnSendToIRC(CString &sLine) override; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move & to left
modules/log.cpp
Outdated
CIRCNetwork* pNetwork = GetNetwork(); | ||
CMessage Message(sLine); | ||
OnQuit(pNetwork->GetIRCNick(), | ||
static_cast<CQuitMessage>(Message).GetReason(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is Message.As()
modules/log.cpp
Outdated
@@ -399,6 +401,18 @@ void CLogMod::OnQuit(const CNick& Nick, const CString& sMessage, | |||
} | |||
} | |||
|
|||
CModule::EModRet CLogMod::OnSendToIRC(CString &sLine) { | |||
if (sLine.Token(0) != "QUIT") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may break if any tag is sent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. That being said, do we ever send quit messages with tags?
abfc029
to
5ac3ca8
Compare
If #1398 is merged before this, I can rebase this on top of that to use |
5ac3ca8
to
67444cd
Compare
Since OnIRCDisconnected() doesn't include the quit message, and OnQuit() isn't called when the user quits, we need to hook OnSendToIRCMessage().
67444cd
to
d32ac7c
Compare
Reworked to use |
Since
OnIRCDisconnected()
doesn't include the quit message, andOnQuit()
isn't called when the user quits, we need to hookOnSendToIRC()
.