Skip to content
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

Parsing message tags #334

Closed
progval opened this issue Aug 29, 2021 · 3 comments · Fixed by #337
Closed

Parsing message tags #334

progval opened this issue Aug 29, 2021 · 3 comments · Fixed by #337

Comments

@progval
Copy link
Member

progval commented Aug 29, 2021

Hi, I'd like to try adding support for message tags. They would have to be stored in SircMessage, and I can see four ways to do it:

  1. their own structure: typedef struct { char *key; char *value } SircMessageTag (with value NULLable if it's a tag with no value or an empty value) and SircMessage storing an array of those + a count
  2. storing an array of key + value pair, either NULL-terminated or with a count
  3. GHashTable
  4. adding an attribute to SircMessage for each tag name, populated directly by the parser; and discard unknown tags

Option 3 would probably be the most convenient, but it will be an issue if you want to add support for extensions/plugins/... in the future; as they can't use tags the parser isn't aware of.

Ditto for SircCommandBuilder.

What do you think?

(related issue: GH-75)

@SilverRainZ
Copy link
Member

I prefer the 1st way, it also can work with 4st way: adding frequence uesed SircMessageTag to SircMessage for each tag name, for unknown tag, we adding it into an array.

@SilverRainZ SilverRainZ modified the milestone: 1.3 Aug 30, 2021
@SilverRainZ SilverRainZ linked a pull request Dec 6, 2021 that will close this issue
@SilverRainZ
Copy link
Member

I think this can be closed?

@progval
Copy link
Member Author

progval commented Dec 6, 2021

Indeed!

@progval progval closed this as completed Dec 6, 2021
@SilverRainZ SilverRainZ mentioned this issue Jan 29, 2022
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants