-
Notifications
You must be signed in to change notification settings - Fork 55
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 support for the draft/extended-monitor capability #254
base: main
Are you sure you want to change the base?
Conversation
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.
Don't abuse side effects from other functions in clever ways, especially when these functions make it easy to exclude the appropriate users from getting duplicate messages without such abuse. This makes it difficult to refactor code without considering how something abuses something else's side effects, making the code harder to work with overall.
I make no comment about whether I think this functionality is useful/good or not, this is simply to correct implementation issues.
@@ -142,6 +143,7 @@ init_builtin_capabs(void) | |||
CLICAP_CAP_NOTIFY = capability_put(cli_capindex, "cap-notify", NULL); | |||
CLICAP_CHGHOST = capability_put(cli_capindex, "chghost", &high_priority); | |||
CLICAP_ECHO_MESSAGE = capability_put(cli_capindex, "echo-message", NULL); | |||
CLICAP_EXTENDED_MONITOR = capability_put(cli_capindex, "draft/extended-monitor", &high_priority); |
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.
Why does this need high priority? It's a draft cap, and anybody supporting it should support 3.2-style CAP LS (which doesn't need priority)
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.
Makes sense 👍
@@ -1637,6 +1637,10 @@ change_nick_user_host(struct Client *target_p, const char *nick, const char *use | |||
sendto_common_channels_local_butone(target_p, CLICAP_CHGHOST, NOCAPS, |
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.
Instead of abusing current_serial semantics, this should exclude users with extended-monitor enabled:
sendto_common_channels_local_butone(target_p, CLICAP_CHGHOST, NOCAPS, | |
sendto_common_channels_local_butone(target_p, CLICAP_CHGHOST, CLICAP_EXTENDED_MONITOR, |
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.
We still want to sent messages to users sharing a channel and who have the extended-monitor cap but who are not currently monitoring the user. That's why we can't just send to neighbours first, excluding those who have the cap, then send to all those who are monitoring the user.
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.
Err, yes. The correct implementation would end up being more complex than this, my brain just failed to process it correctly.
@@ -1637,6 +1637,10 @@ change_nick_user_host(struct Client *target_p, const char *nick, const char *use | |||
sendto_common_channels_local_butone(target_p, CLICAP_CHGHOST, NOCAPS, | |||
":%s!%s@%s CHGHOST %s %s", | |||
target_p->name, target_p->username, target_p->host, user, host); | |||
struct monitor *monptr = find_monitor(target_p->name, 0); | |||
if(monptr) | |||
sendto_monitor_with_capability_butserial(target_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_CHGHOST, NOCAPS, true, ":%s!%s@%s CHGHOST %s %s", |
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.
Eliminate current_serial abuse
sendto_monitor_with_capability_butserial(target_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_CHGHOST, NOCAPS, true, ":%s!%s@%s CHGHOST %s %s", | |
sendto_monitor_with_capability(target_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_CHGHOST, NOCAPS, ":%s!%s@%s CHGHOST %s %s", |
@@ -75,6 +75,7 @@ extern void sendto_match_butone(struct Client *, struct Client *, | |||
extern void sendto_match_servs(struct Client *source_p, const char *mask, | |||
int capab, int, const char *, ...) AFP(5, 6); | |||
|
|||
extern void sendto_monitor_with_capability_butserial(struct Client *, struct monitor *monptr, int caps, int negcaps, bool skipserial, const char *, ...) AFP(6, 7); |
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.
Eliminate current_serial abuse
extern void sendto_monitor_with_capability_butserial(struct Client *, struct monitor *monptr, int caps, int negcaps, bool skipserial, const char *, ...) AFP(6, 7); | |
extern void sendto_monitor_with_capability(struct Client *, struct monitor *monptr, int caps, int negcaps, const char *, ...) AFP(5, 6); |
*/ | ||
void | ||
sendto_monitor(struct Client *source_p, struct monitor *monptr, const char *pattern, ...) | ||
_sendto_monitor_with_capability_butserial(struct Client *source_p, struct monitor *monptr, int caps, int negcaps, bool skipserial, const char *pattern, va_list * args) |
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.
Eliminate current_serial abuse
_sendto_monitor_with_capability_butserial(struct Client *source_p, struct monitor *monptr, int caps, int negcaps, bool skipserial, const char *pattern, va_list * args) | |
_sendto_monitor_with_capability(struct Client *source_p, struct monitor *monptr, int caps, int negcaps, const char *pattern, va_list * args) |
@@ -89,6 +90,10 @@ m_away(struct MsgBuf *msgbuf_p, struct Client *client_p, struct Client *source_p | |||
|
|||
sendto_common_channels_local_butone(source_p, CLICAP_AWAY_NOTIFY, NOCAPS, ":%s!%s@%s AWAY", |
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.
Exclude extended-monitor
sendto_common_channels_local_butone(source_p, CLICAP_AWAY_NOTIFY, NOCAPS, ":%s!%s@%s AWAY", | |
sendto_common_channels_local_butone(source_p, CLICAP_AWAY_NOTIFY, CLICAP_EXTENDED_MONITOR, ":%s!%s@%s AWAY", |
@@ -89,6 +90,10 @@ m_away(struct MsgBuf *msgbuf_p, struct Client *client_p, struct Client *source_p | |||
|
|||
sendto_common_channels_local_butone(source_p, CLICAP_AWAY_NOTIFY, NOCAPS, ":%s!%s@%s AWAY", | |||
source_p->name, source_p->username, source_p->host); | |||
struct monitor *monptr = find_monitor(source_p->name, 0); | |||
if(monptr) | |||
sendto_monitor_with_capability_butserial(source_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_AWAY_NOTIFY, NOCAPS, true, ":%s!%s@%s AWAY", |
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.
Eliminate current_serial abuse
sendto_monitor_with_capability_butserial(source_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_AWAY_NOTIFY, NOCAPS, true, ":%s!%s@%s AWAY", | |
sendto_monitor_with_capability(source_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_AWAY_NOTIFY, NOCAPS, ":%s!%s@%s AWAY", |
@@ -127,6 +132,14 @@ m_away(struct MsgBuf *msgbuf_p, struct Client *client_p, struct Client *source_p | |||
source_p->username, | |||
source_p->host, | |||
source_p->user->away); | |||
struct monitor *monptr = find_monitor(source_p->name, 0); | |||
if(monptr) | |||
sendto_monitor_with_capability_butserial(source_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_AWAY_NOTIFY, NOCAPS, true, |
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.
Adjust the regular away-notify to exclude extended-monitor (due to a GitHub limitation, I can't comment on that line). Eliminate current_serial abuse
sendto_monitor_with_capability_butserial(source_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_AWAY_NOTIFY, NOCAPS, true, | |
sendto_monitor_with_capability(source_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_AWAY_NOTIFY, NOCAPS, |
@@ -139,6 +139,11 @@ me_su(struct MsgBuf *msgbuf_p, struct Client *client_p, struct Client *source_p, | |||
sendto_common_channels_local(target_p, CLICAP_ACCOUNT_NOTIFY, NOCAPS, ":%s!%s@%s ACCOUNT %s", |
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.
Exclude extended-monitor
sendto_common_channels_local(target_p, CLICAP_ACCOUNT_NOTIFY, NOCAPS, ":%s!%s@%s ACCOUNT %s", | |
sendto_common_channels_local(target_p, CLICAP_ACCOUNT_NOTIFY, CLICAP_EXTENDED_MONITOR, ":%s!%s@%s ACCOUNT %s", |
@@ -139,6 +139,11 @@ me_su(struct MsgBuf *msgbuf_p, struct Client *client_p, struct Client *source_p, | |||
sendto_common_channels_local(target_p, CLICAP_ACCOUNT_NOTIFY, NOCAPS, ":%s!%s@%s ACCOUNT %s", | |||
target_p->name, target_p->username, target_p->host, | |||
EmptyString(target_p->user->suser) ? "*" : target_p->user->suser); | |||
struct monitor *monptr = find_monitor(target_p->name, 0); | |||
if(monptr) | |||
sendto_monitor_with_capability_butserial(target_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_ACCOUNT_NOTIFY, NOCAPS, true, ":%s!%s@%s ACCOUNT %s", |
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.
Eliminate current_serial abuse
sendto_monitor_with_capability_butserial(target_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_ACCOUNT_NOTIFY, NOCAPS, true, ":%s!%s@%s ACCOUNT %s", | |
sendto_monitor_with_capability(target_p, monptr, CLICAP_EXTENDED_MONITOR | CLICAP_ACCOUNT_NOTIFY, NOCAPS, ":%s!%s@%s ACCOUNT %s", |
@delthas, any plans to update this PR? |
See: ircv3/ircv3-specifications#466