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

Filterx add object str method #516

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

bazsi
Copy link
Member

@bazsi bazsi commented Feb 16, 2025

This branch separates repr() and str() methods for objects. This works similarly to Python.

The goal of this patch is to resolve an issue around datetime, which is formatted differently when embedded in a JSON (where map_to_json is used) and when formatted using format_json (where repr is used).

The branch also changes a number of call sites to use str() instead of repr(). Convention is:

  • whenever we display an object to the developer, we should use repr()
  • whenever we include an object in some form of output intended for the user, we should use str()

By default, str == repr so the formats can be the same.

@bazsi bazsi force-pushed the filterx-add-object-str-method branch 3 times, most recently from 2cee8b1 to 3ead54f Compare February 16, 2025 11:50
@@ -195,10 +195,15 @@ _filterx_ref_repr(FilterXObject *s, GString *repr)
{
FilterXRef *self = (FilterXRef *) s;

if (self->value->type->repr)
return self->value->type->repr(self->value, repr);
return filterx_object_repr(self->value, repr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks append(), because filterx_object_repr() truncates the string.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have called this function _filterx_ref_repr_append to avoid future mistakes.
(The vtable pointer should also be called append, but that would look ugly.)

{
FilterXRef *self = (FilterXRef *) s;

return filterx_object_str(self->value, str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should call the virtual method directly to avoid the truncation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we fix my original mistake and call it _filterx_ref_str_append?

if (!object)
return NULL;

GString *buf = scratch_buffers_alloc();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is this reclaimed? Shouldn't we do it here just to be sure?

@@ -195,10 +195,15 @@ _filterx_ref_repr(FilterXObject *s, GString *repr)
{
FilterXRef *self = (FilterXRef *) s;

if (self->value->type->repr)
return self->value->type->repr(self->value, repr);
return filterx_object_repr(self->value, repr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have called this function _filterx_ref_repr_append to avoid future mistakes.
(The vtable pointer should also be called append, but that would look ugly.)

{
FilterXRef *self = (FilterXRef *) s;

return filterx_object_str(self->value, str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we fix my original mistake and call it _filterx_ref_str_append?

Just like Python, sometimes we want to turn an object into a string to add
it to trace messages (repr) indended to the developer, in other cases we
want a simpler string representation aimed at customers (str).

This patch adds str() in addition to the existing repr().  If str() is not
implemented it defaults to using repr, so defaults doing the same as today.

Signed-off-by: Balazs Scheidler <[email protected]>
str() is a UNIX timestamp since epoch, including fractions of a digits,
but no timezone.

repr() remains an ISO date so that logs are easier to read.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi bazsi force-pushed the filterx-add-object-str-method branch from 3ead54f to 7d9a701 Compare February 19, 2025 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants