Tag Builder: init - Githubissues

museun / twitchchat

interface to the irc portion of Twitch's chat

Apache License 2.0

103 stars 23 forks source link

Tag Builder: init #206

Closed Horki closed 4 years ago

Horki commented 4 years ago

@museun hi, this is an initial version of Tag Builder, I created in a special namespace as a helper

TODO:

should values be unique by key?
if not, can we have multiple key-value values?
what sort of sanitization should we have when building tags? (remove empty space, allow ':', ',' in key-value?)

museun commented 4 years ago

keys should be unique. I imagine 'last' key wins (the normal insertion rules for a hashmap)
- note, IRCv3 allows for duplicate keys but Twitch does not (yet? they don't send duplicate keys)
there are some quoting rules about tags, I'll have to find them. the Twitch docs are notably lacking in many areas.

There should be a merge function so you can take a pre-parsed message and modify its existed tags with a new set of tags.

This is what I came up with, but there are various edge cases I haven't bothered to handle. I've been exceedingly busy lately: https://github.com/museun/still_shaken/blob/master/crates/still_shaken/src/bot/test.rs#L264-L318

I think the one provided in this crate should operate directly on Tags/TagsIndices though.

museun commented 4 years ago

I ended up rewriting it using some of your design.

I had to do some reworking to support escaping and unescaping.

I apologize for all of my rewrites of your contributions. This crate is structured weirdly (with a lot of hidden 'magic') that makes it non-trivial (or obvious, to be honest) on how to do things.

Your contributions are motivating me, though. This 'feature' would've stayed unimplemented forever because I would just do it locally for whatever I use this crate for.

museun commented 4 years ago

As for your questions: I decided to implement it according to the IRCv3 spec:

keys are unique
- but it is not an error to replace a key
values can be empty
it escapes the keys and values when it encodes the tags
this won't be exposed off existing types, but through a new 'test' module.
- the builder would only be used for users to create test messages for their own use.
- the crate, internally will build up messages outside of tags (via the 'commands' module).

museun commented 4 years ago

I'm satisfied with the design. Merging this

Horki commented 4 years ago

@museun that's a lot of work in a short period :+1: