Summary

Create IbisTags

Review Request #3167 — Created May 7, 2024 and submitted May 10, 2024, 5:35 a.m.

Information

Owner

grim

Repository

ibis/ibis

Branch

default

Bugs

IBIS-1

Depends On

~~3166~~

Blocks

3168

Reviewers

Groups

pidgin

People

Description

This object simplifies the interactions with tags. The user only ever deals
with the raw tag values. The parser will unescape the tags and store them in
the tags objects. Likewise, the serializer will escape the tags as it outputs
them.

Right now the tags object works like an ordered hash table. This is mostly for
unit tests, but is probably fine going forward. That said, I'm not particularly
fond of the implementation, but it does work so it's good enough for now.

Testing Done

Ran the parser and tags tests under valgrind.
Called in the turtles as well.

Commits

Summary	ID
Create IbisTags This object simplifies the interactions with tags. The user only ever deals with the raw tag values. The parser will unescape the tags and store them in the tags objects. Likewise, the serializer will escape the tags as it outputs them. Right now the tags object works like an ordered hash table. This is mostly for unit tests, but is probably fine going forward. That said, I'm not particularly fond of the implementation, but it does work so it's good enough for now.	5a6bbdc3dcdee62a29a6407747aec522a821c614

Summary

Create IbisTags

This object simplifies the interactions with tags. The user only ever deals with the raw tag values. The parser will unescape the tags and store them in the tags objects. Likewise, the serializer will escape the tags as it outputs them. Right now the tags object works like an ordered hash table. This is mostly for unit tests, but is probably fine going forward. That said, I'm not particularly fond of the implementation, but it does work so it's good enough for now.

5a6bbdc3dcdee62a29a6407747aec522a821c614

Issues

Description	From	Last Updated
Don't need the outer if since g_set_object should handle that.	QuLogic	May 10, 2024, 4:29 a.m.
'for managing'? or 'to manage'?	QuLogic	May 10, 2024, 4:29 a.m.
I don't think this correctly handles duplicate calls with the same key. Since the hash table owns the string, the …	QuLogic	May 10, 2024, 4:37 a.m.
Comment should be deleted now.	QuLogic	May 10, 2024, 4:30 a.m.
empty/missing values, I think, not tags.	QuLogic	May 10, 2024, 5:24 a.m.
Remove?	QuLogic	May 10, 2024, 5:23 a.m.
Can also add a parser test with duplicate tags in the input; I believe the spec says to use the …	QuLogic	May 10, 2024, 5:26 a.m.
escapes?	QuLogic	May 10, 2024, 5:26 a.m.

Change Summary:

Make sure seralized tags starts with an @

Commits:

	Summary	ID
	Create IbisTags This object simplifies the interactions with tags. The user only ever deals with the raw tag values. The parser will unescape the tags and store them in the tags objects. Likewise, the serializer will escape the tags as it outputs them. Right now the tags object works like an ordered hash table. This is mostly for unit tests, but is probably fine going forward. That said, I'm not particularly fond of the implementation, but it does work so it's good enough for now.	116b26c445498b1df7aac87c610891fc86b9f6d8
	Create IbisTags This object simplifies the interactions with tags. The user only ever deals with the raw tag values. The parser will unescape the tags and store them in the tags objects. Likewise, the serializer will escape the tags as it outputs them. Right now the tags object works like an ordered hash table. This is mostly for unit tests, but is probably fine going forward. That said, I'm not particularly fond of the implementation, but it does work so it's good enough for now.	013e05c0ab4bf7fdd71d1705858977f7f6afd3bc

Diff:

Revision 2 (+1128 -322)

Show changes

	ibis/ibismessage.h
	ibis/ibismessage.c
	ibis/ibisparser.c
	ibis/ibistags.h
	ibis/ibistags.c
	ibis/tests/test_parser.c
	ibis/tests/test_tags.c

ibis/ibismessage.c (Diff revision 2)
The issue has been resolved. Show all issues
```
Don't need the outer if since g_set_object should handle that.
```
ibis/ibistags.h (Diff revision 2)
The issue has been resolved. Show all issues
```
'for managing'? or 'to manage'?
```

ibis/ibistags.c (Diff revision 2)

The issue has been resolved. Show all issues

I don't think this correctly handles duplicate calls with the same key. Since the hash table owns the string, the queue needs to be in sync with it. But g_hash_table_insert will free new_key if it's a duplicate, making this insertion into the queue invalid. You could use g_hash_table_replace instead, but then you need to remove the old key pointer from the queue.

I see only one test with repeated keys, but it's used for comparing with ibis_tags_parse, so won't iterate over tags->keys.

So there's a missing test for duplicate keys with ibis_tags_serialize, which would access tags->keys.

QuLogic May 10, 2024, 3:33 a.m.

Sorry, the test with repeated keys is only for ibis_parser_parse, not ibis_tags_parse (though I guess it does effectively call that too).

ibis/ibistags.c (Diff revision 2)
The issue has been resolved. Show all issues
```
Comment should be deleted now.
```

Change Summary:

Addressed issues, added parser unit tests, added the escape tests from the spec, reworked how the ordered stuff works.

Commits:

	Summary	ID
	Create IbisTags This object simplifies the interactions with tags. The user only ever deals with the raw tag values. The parser will unescape the tags and store them in the tags objects. Likewise, the serializer will escape the tags as it outputs them. Right now the tags object works like an ordered hash table. This is mostly for unit tests, but is probably fine going forward. That said, I'm not particularly fond of the implementation, but it does work so it's good enough for now.	013e05c0ab4bf7fdd71d1705858977f7f6afd3bc
	Create IbisTags This object simplifies the interactions with tags. The user only ever deals with the raw tag values. The parser will unescape the tags and store them in the tags objects. Likewise, the serializer will escape the tags as it outputs them. Right now the tags object works like an ordered hash table. This is mostly for unit tests, but is probably fine going forward. That said, I'm not particularly fond of the implementation, but it does work so it's good enough for now.	d3fab358f284f5d5069724447ca313c05e587e31

Diff:

Revision 3 (+1664 -328)

Show changes

	ibis/ibismessage.h
	ibis/ibismessage.c
	ibis/ibisparser.c
	ibis/ibistags.h
	ibis/ibistags.c
	ibis/tests/test_parser.c
	ibis/tests/test_tags.c

ibis/ibistags.c (Diff revisions 2 - 3)
The issue has been resolved. Show all issues
```
empty/missing values, I think, not tags.
```
ibis/tests/test_tags.c (Diff revisions 2 - 3)
The issue has been resolved. Show all issues
```
Remove?
```

ibis/tests/test_tags.c (Diff revisions 2 - 3)

The issue has been resolved. Show all issues

Can also add a parser test with duplicate tags in the input; I believe the spec says to use the last one.

ibis/tests/test_tags.c (Diff revisions 2 - 3)
The issue has been resolved. Show all issues
```
escapes?
```

Change Summary:

Addressed issues, fixed other tests that were broken that i missed, also added tests for equals and contains.

Commits:

	Summary	ID
	Create IbisTags This object simplifies the interactions with tags. The user only ever deals with the raw tag values. The parser will unescape the tags and store them in the tags objects. Likewise, the serializer will escape the tags as it outputs them. Right now the tags object works like an ordered hash table. This is mostly for unit tests, but is probably fine going forward. That said, I'm not particularly fond of the implementation, but it does work so it's good enough for now.	d3fab358f284f5d5069724447ca313c05e587e31
	Create IbisTags This object simplifies the interactions with tags. The user only ever deals with the raw tag values. The parser will unescape the tags and store them in the tags objects. Likewise, the serializer will escape the tags as it outputs them. Right now the tags object works like an ordered hash table. This is mostly for unit tests, but is probably fine going forward. That said, I'm not particularly fond of the implementation, but it does work so it's good enough for now.	5a6bbdc3dcdee62a29a6407747aec522a821c614

Diff:

Revision 4 (+2134 -362)

Show changes

	ibis/ibismessage.h
	ibis/ibismessage.c
	ibis/ibisparser.c
	ibis/ibistags.h
	ibis/ibistags.c
	ibis/tests/test_message.c
	ibis/tests/test_parser.c
	ibis/tests/test_tags.c

Ship it!

```
Ship It!
```

Status:: Completed