Skip to content

Filterx switch range case#1093

Draft
sodomelle wants to merge 4 commits into
axoflow:mainfrom
sodomelle:filterx-switch-range-case
Draft

Filterx switch range case#1093
sodomelle wants to merge 4 commits into
axoflow:mainfrom
sodomelle:filterx-switch-range-case

Conversation

@sodomelle
Copy link
Copy Markdown
Contributor

Range is inclusive on both ends.

Example usage:

switch (${values.int}) {
    case 1..4:
        result = "below";
        break;
    case 5:
        result = "exact";
        break;
    default:
        result = "above";
        break;
};

sodomelle added 4 commits May 26, 2026 13:23
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Tamás Kosztyu <tamas.kosztyu@axoflow.com>
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Tamás Kosztyu <tamas.kosztyu@axoflow.com>
We can't cache a range, so an early return is added too.

Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Tamás Kosztyu <tamas.kosztyu@axoflow.com>
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Tamás Kosztyu <tamas.kosztyu@axoflow.com>
Comment thread lib/filterx/expr-switch.c
self->super.super.free_fn = _switch_case_free;
self->super.super.walk_children = _switch_case_walk;
self->super.operand = lower;
self->upper = upper;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This probably belongs to the previous commit.

Comment thread lib/filterx/expr-switch.c
{
FilterXSwitchCase *self = g_new0(FilterXSwitchCase, 1);
filterx_unary_op_init_instance(&self->super, FILTERX_EXPR_TYPE_NAME(switch_case), FXE_READ, value);
self->upper = NULL;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We usually don't do this after g_new0.

Comment thread lib/filterx/expr-switch.c
if (!value)
continue;
if (switch_case->upper)
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we extract this block into an gboolean _is_switch_range_case_matching() or anything similar?

(The same can be done with the non-ranged case, but it's small enough to be left inside the else block.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have been told the PR will be refactored to have an inherited "case" class, which handles ranges more elegantly; that invalidate this note.

@MrAnno
Copy link
Copy Markdown
Contributor

MrAnno commented May 27, 2026

We'll need a news entry for this.

@bazsi
Copy link
Copy Markdown
Member

bazsi commented May 28, 2026

$.02: I like this, but depending on the use-case we might want to cache the ranged values as well, especially if those are "small", enumerable ranges (like the ones in the example).

Copy link
Copy Markdown
Contributor

@mitzkia mitzkia left a comment

Choose a reason for hiding this comment

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

Thanks for the Light testcases, I can approve them.

@sodomelle sodomelle marked this pull request as draft June 2, 2026 10:49
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.

4 participants