Add support for server-time #345
Conversation
|
Can use use a GVariantDict for such params? |
|
Hmm, the possible items should be known at compile time so I don't think a dict is needed. Unless you want to add support for adding items via plugins in the future? |
|
If all items are known at compile time, how about adding a struct? |
https://ircv3.net/specs/extensions/server-time This adds a new `const SircMessageContext*` parameter to all callback functions, which for now only contains the time specified by the server if any, and the current time otherwise.
|
Hey, I finally found some free time to work on this. I rebased the PR on master and rewrote it to use an opaque structure. |
|
By the way, the way I use to test this is to connect to irc.ergo.chat, join an existing room with some messages, and type It looks like this:
(HistServ is a pseudo-user used by Ergo to announce non-messages, when clients don't support the |
src/sirc/sirc.c
Outdated
| g_return_if_fail(sirc->events->connect); | ||
| sirc->events->connect(sirc, "CONNECT"); | ||
| if (!sirc->events->connect) { | ||
| sirc_message_context_free(context); |
There was a problem hiding this comment.
How about using g_auto for freeing context?
There was a problem hiding this comment.
Oh nice, thanks! I'll use g_autoptr (which seems to be the right one here)
There was a problem hiding this comment.
There are other calls to sirc_message_context_free in the code, please check them : )
There was a problem hiding this comment.
including a couple of memory leaks, oops
There was a problem hiding this comment.
We need a memory-safey language :-P
src/sirc/sirc_context.c
Outdated
| time = g_date_time_new_now_local(); | ||
| } | ||
|
|
||
| g_return_val_if_fail(time, NULL); |
There was a problem hiding this comment.
This check can be omitted, I think.
There was a problem hiding this comment.
"This function will always succeed unless GLib is still being used after the year 9999."
fair enough
src/sirc/sirc.c
Outdated
| g_return_if_fail(sirc->events->connect); | ||
| sirc->events->connect(sirc, "CONNECT"); | ||
| if (!sirc->events->connect) { | ||
| sirc_message_context_free(context); |
There was a problem hiding this comment.
There are other calls to sirc_message_context_free in the code, please check them : )
https://ircv3.net/specs/extensions/server-time
This is used by servers and bouncers when replaying history.
This commit adds a new
const GDateTime *timeconst SircMessageContext*parameter to all callback functions,containing the time specified by the server if any, and
NULLthe current time otherwise.I wrote this a while ago. In retrospect, I'm not sure it's a good solution to add a newargument just for this; because such arguments would just accumulate with more
context info if Srain gets support for other specifications in the future (eg.
message-specific display names,
bot message tags,
message IDs and replies, etc.)
So I wonder if I should rewrite this PR to pass some sort of structure withall the context info instead. Possibly pass all the tags directly.
What do you think?
EDIT: I replaced it with a structure