Add support for raw message tags to the plugin api.

[DasBrain]

Closes #1393

[lgtm-com]

This pull request introduces 1 alert when merging 84de43d into cdfc3b9 - view on LGTM.com

new alerts:

• 1 for Lossy pointer cast

[TingPing]

I think we can do better than just dumping a string here.

I think I want an opaque pointer in this struct and add API like:

const char *account = hexchat_event_attrs_get_tag (attrs, "account");

[TingPing]

Also since this is an API change we need a way to expose that. Maybe a #define HEXCHAT_API_VERSION 2 or something.

[DasBrain]

The API change is backward compatible (but not forward compatible - a plugin using the new API doesn't work with an old HexChat without extra care.)

But the API you suggest is not enough: There needs to be a way to list all tags (how? as comma separated list? Who has to free that?)

I think this is like using words_eol[1] and parsing the IRC line yourself. And this is powerful - but also needs extra work to be done by the plugin. The upside is that this doesn't involve any additional allocations on the HexChat side.

I am not against also adding a opaque pointer that can be used to query specific fields.
Insert out-of-scope excuse here

[DasBrain]

After a bit more thoughts:

Why don't we use hexchat_event_attrs for that?
I really like what you did with server_time_utc - parsing that in all the different formats can be a hassle.

So, I think, we could expand hexchat_event_attrs like this:

typedef struct
{
	time_t server_time_utc; /* 0 if not used */
	const char* raw_tags; /* Only for server events. NULL if the server sent no tags. */
	const char* account; /* Only for server events. NULL if the server sent no account tag */
	/* ... */
} hexchat_event_attrs;

This gives the best of both worlds:

• Easy access to an individual tag (if hexchat understands it)
• Raw access to the tags for cases where hexchat doesn't understand/use the tag.

WDYT?

[TingPing]

The API change is backward compatible (but not forward compatible - a plugin using the new API doesn't work with an old HexChat without extra care.)

Yeah, I just want plugins to be able to easily build against both:

#if HEXCHAT_API_VERSION >= 2
    foo();
#endif

(Well bad example since it was undefined before)

But the API you suggest is not enough: There needs to be a way to list all tags (how? as comma separated list? Who has to free that?)

We have list apis already, like hexchat_list_fields(). We can just assert the strings are temporary and must be copied if needed. So an api like const char * const *tags = hexchat_event_attrs_get_tags (attrs); could work.

[DasBrain]

The hexchat-plugin.h is usually (often/sometimes/whatever) bundled with the plugin.
You can't do a compile-time check against which API you have.

You might be able to force a specific API (fail if the hexchat version is not big enough)- but with the current API/ABI design, you are stuck with supporting all old ABIs in every new release.

On the windows side it's simple: All functions are members of the hexchat_plugin struct. As long as you don't change the order, you can add more functions at the end. That's about it - you can't remove a function, and you can't change the signature of a function without breaking stuff.

Renaming a function breaks compiling but already compiled plugins will still work on windows, while on linux they will probably break. (I have no knowledge about how linux handles the binding of the functions ¯\_(ツ)_/¯.)

An other problem about your API suggestion is: What if something is not a string
Your API restricts it to returning a char * - which is not always appropriate.

[TingPing]

The hexchat-plugin.h is usually (often/sometimes/whatever) bundled with the plugin.
You can't do a compile-time check against which API you have.

Thats very much a windows-ism. HexChat installs the header as well as a pkg-config file and plugins can build against an unknown system version.

An other problem about your API suggestion is: What if something is not a string
Your API restricts it to returning a char * - which is not always appropriate.

We can copy the: hexchat_list_str() hexchat_list_int() hexchat_list_time() mess.

[DasBrain]

I'd prefer to parse the known tags and put them into the hexchat_event_attrs struct.
Parsing the tags is easy - and probably best done in the relevant language itself - as that can build a native hash out of it.

[TingPing]

I'd prefer to parse the known tags and put them into the hexchat_event_attrs struct.

So my thought process was that a generic getter will automatically work for all bindings. If you do a struct by struct basis each plugin has to be updated every addition.

I think it was a mistake the struct was ever public as now we have to be concerned about backwards compat at all.

[DasBrain]

You always have to care about backwards compat.

Changing a struct is fine, as long as you only ever add stuff to the end.
(There are some tricks with flags and bit addressing, which is nice I guess and could be used by some tags)

Your generic getter will only work with one data type - char *.
If a plugin wants to use functionality provided by a newer version, then yes, it has to be updated to actually use that.

But there is a real need to access all message tags sent by the server.
While my solution is not the "most elegant" or "better than just dumping a string" - it is pragmatic in several ways:

• It is efficient - no additional allocations.
• It is future proof - as the raw tags are passed, they can be parsed by any binding: For current known tags and for tags that might exist in the future.

Yes, it offloads the tag parsing to the plugin - but that is also not necessary a bad thing. Most languages have their own hash type - and it is probably easier to parse the message tag into each specific hash type than to query one global hash to create the right hash for the language.