Global lambdas

[tiawl]

Currently empty.
Will close #23 when finalized.
As stated in the mentioned issue, I will ask for review when a step is ready (POC, test, doc or implementation)

---

TODO:

• global lambdas:
  • native context:
    • 1 parameter global lambdas
    • 2 parameters global lambdas:
      • sections
      • interpolation
  • JSON context
    • 1 parameter global lambdas
    • 2 parameters global lambdas:
      • sections
      • interpolation
• propagate the allocator in LambdaContext for the operations that take an allocator

[tiawl]

Hi,

I am requesting a first review because:

• I have a (very) minimal working version for global lambdas,
• I need to know if I am going in the right direction for the next steps.

Results

Currently only global lambdas for native context with unique parameter works (so with LambdaContext parameter) . It works for interpolation and section. So something like this is expected to work:

const dummy_text = "UPPER TEXT";

const PocLambdas = struct {
    pub fn upper1arg(ctx: LambdaContext) !void {
        try ctx.write(dummy_text);
    }
};

const Dummy = struct { .content: const u8, };

test "interpolation & section: 1 arg" {
    const allocator = std.testing.allocator;
    const template_interpolation = "{{upper1arg}}";
    const template_section = "{{#upper1arg}}poc{{/upper1arg}}"
    const dummy = Dummy{ .content = "text", };
    const ptr = &dummy;
    
    const result_interpolation = try allocRendertextWithOptions(
        allocator,
        template_interpolation,
        ptr,
        .{ .global_lambdas = PocLambdas, },
    );
    defer allocator.free(result_interpolation);
    
    const result_section = try allocRendertextWithOptions(
        allocator,
        template_section,
        ptr,
        .{ .global_lambdas = PocLambdas, },
    );
    defer allocator.free(result_section);
    
    try std.testing.expectEqualString(dummy_text, result_interpolation);
    try std.testing.expectEqualString(dummy_text, result_section);
}

But that's it. More advanced features are not yet implemented.

The changes I made

Most of them are in the DataRender struct (from src/rendering/rendering.zig):

• I added a new stack_global_lambdas field of ContextStack type for the global lambdas type provided by the user,
• When rendering, if an implemented Context method (iterator, capacityHint, interpolate or expandLambda) returns PathResolutionType.not_found_in_context on DataRender.stack, I re-run the method with the DataRender.stack_global_lambdas field instead.

No more.

What next ?

The next step will allow global lambdas to have another parameter. The user could make things more interesting like this:

const PocLambdas = struct {
    pub fn upper(text: *const TextWithAllocator, ctx: LambdaContext) !void {
        const upper_text = try std.ascii.allocUpperString(text.allocator, text.content);
        defer text.allocator.free(upper_text);
        try ctx.write(upper_text);
    }
};

const TextWithAllocator = struct {
    .content: const u8,
    .allocator: std.mem.Allocator,
};

test "interpolation" {
    const allocator = std.testing.allocator;
    const template = "{{upper}}";
    const text = TextWithAllocator{
      .content = "upper text",
      .allocator = allocator,
    };
    const ptr_text = &text;
    
    const result = try allocRendertextWithOptions(
        allocator,
        template,
        ptr_text,
        .{ .global_lambdas = PocLambdas, },
    );
    defer allocator.free(result);
    
    try std.testing.expectEqualString("UPPER TEXT", result);
}

And here, troubles are coming: when I add a new parameter to the global lambda, the mentioned methods (iterator, capacityHint, interpolate and expandLambda) return a PathResolutionType.chain_broken when called with the new DataRender.stack_global_lambdas field.

It is coming from the isValidLambdaFunction (comptime TData: type, comptime TFn: type) bool which returns true only if the first parameter of the function signature is the TData type (or a pointer to the TData type).

In the zig code above, TData is PocLambdas and the function signature is fn upper(text: *const TextWithAllocator, ctx: LambdaContext) !void (TextWithAllocator != PocLambdas so isValidLambdaFunction returns false).

The solution I am thinking about to make isValidLambdaFunction happy is to give it the DataRender.stack.ctx Data type as TData parameter when findLambdaPath (depth: Depth, comptime TValue: type, action_param: anytype, data: anytype, current_path_part: []const u8) is called with the global lambda type provided by user as TValue parameter. However I do not have any good or clean idea to achieve this.

This is why, before going further, I am wondering if I choosed the right approach. If not, what can I do instead ? If yes, do you have any tips to go further ?

Thank you.

[batiati]

Hey! Thanks for the first review, I'll start reading through!

Just a comment about lambdas with only the ctx parameter.
They are intended to be applied over the rendered children tags, like this:

const PocLambdas = struct {
      pub fn upper(ctx: LambdaContext) !void {
          var text = try ctx.renderAlloc(testing.allocator, ctx.inner_text);
          defer testing.allocator.free(text);

          for (text, 0..) |char, i| {
              text[i] = std.ascii.toUpper(char);
          }

          try ctx.write(text);
      }
}

const Dummy = struct { content: []const u8 };

test "interpolation & section: 1 arg" {
    const template = "{{#upper}}{{content}}{{/upper}}"
    const dummy = Dummy{ .content = "text", };
    
    const result_interpolation = try allocRenderTextWithOptions(
        std.testing.allocator,
        template,
        &dummy,
        .{ .global_lambdas = PocLambdas, },
    );
    defer allocator.free(result_interpolation);
        
    try std.testing.expectEqualString(result_interpolation, "TEXT");
}

It just reminded me that I also planned to propagate the allocator in LambdaContext for the operations that take an allocator (e.g. allocRender*).

[tiawl]

Here some news: I found a solution to make isValidLambdaFunction happy. So the POC is almost done. I am going to make some minimal cleanup before merging my draft branch to the PR branch tomorrow.

[tiawl]

I was a little bit too enthusiast: Currently, in a native context, only 2-parameters global lambdas are working for interpolation. So this exemple is expected to work:

const PocLambdas = struct {
      pub fn upper(text: *Text, ctx: LambdaContext) !void {
          // Now the user allocator is propagated
          text.content = try std.ascii.allocUpperString(ctx.allocator.?, text.content);
          defer ctx.allocator.?.free(text.content);
          try ctx.write(text.content);
      }
}

const Text = struct { content: []const u8 };

test "interpolation: 2 parameters" {
    const allocator = std.testing.allocator;
    const lower_text = "text";
    const upper_text = "TEXT";
    
    const template = "{{upper}}";
    var text = Text{
        .content = lower_text,
    };
    const ptr_text = &text;
    
    const result = try mustache.allocRenderTextWithOptions(
        allocator,
        template,
        ptr_text,
        .{ .global_lambdas = PocLambdas, },
    };
    defer allocator.free(result);
    
    try std.testing.expectEqualStrings(upper_text, result);
}

but because I am using section instead of interpolation in this one, it does not work:

const PocLambdas = struct {
      pub fn lower_then_upper(text: *Text, ctx: LambdaContext) !void {
          try ctx.write(text.content);
      
          var content = try ctx.renderAlloc(ctx.allocator.?, ctx.inner_text);
          defer ctx.allocator.?.free(content);

          for (content, 0..) |char, i| {
              content[i] = std.ascii.toUpper(char);
          }
          try ctx.write(" ");
          try ctx.write(content);
      }
}

const Text = struct { content: []const u8 };

test "section: 2 parameters" {
    const allocator = std.testing.allocator;
    const lower_text = "text";
    const upper_text = "TEXT";
    
    const template = "{{#lower_then_upper}}{{content}}{{/lower_then_upper}";
    var text = Text{
        .content = lower_text,
    };
    const ptr_text = &text;
    
    const result = try mustache.allocRenderTextWithOptions(
        allocator,
        template,
        ptr_text,
        .{ .global_lambdas = PocLambdas, },
    };
    defer allocator.free(result);
    
    try std.testing.expectEqualStrings(lower_text ++ " " ++ upper_text, result);
}

I will investigate on this issue during the next days.

Again, if you have any idea to help me in this task or to improve what I have already done, please do not hesitate to tell me. I really would like to have your feedback about what I am doing.

[tiawl]

Ok get it. Finally it was not a big deal. I expected a more sneaky edge case. Hopefully it was not.

So now the PoC is ready for review.

Maybe before thinking how we can implement the same logic for JSON we probably should spend some time to clean dirty hacks I used to make the PoC works. To be totally honest I used some shortcuts to make things works but I am not really proud of what I have done. If you think a voice call could make the review process faster, I am not against this eventuality.

Thank you.

[batiati]

before we start ...

I must say, this was my first project in Zig for which I went beyond the trivial. Besides learning and getting familiar with Zig, one of my goals was to measure how maintainable a Zig codebase could be.
That is, I planned to leave something intentionally unimplemented and get back to it 1+ year later, just for fun, to see if after all this time I would react like "What the hack does it do? 🫤 " or "Actually, it makes a lot of sense 💡".

So, congrats (and thank you) for doing that in my place!
In my opinion, this experiment worked even better with a fresh pair of eyes: I can conclude that Zig code is really great to read (and therefore to maintain). However, YOU deserve a fair share of this! I am impressed by how fast you figured it out, brilliant work!

now let's go ...

I'm halfway through the review, still want to test how we should handle sections for lambdas with 2 arguments.

Please, add your comments about the "shortcuts" you took, and maybe I can help.

[batiati]

😄

[tiawl]

Hi @batiati, here some news:

I am currently more confident with what I have done. I fixed most of shortcuts I used and for the remaining ones (#24 (comment) and #24 (comment)), it is "elegance" issues. I prefer going further and eventually coming back on this later.

Please let me know if:

• you find any other issue with what I have already done,
• you have any tips before going for JSON content.

I will be happy to read you.

[tiawl]

Hi,

I think to close this PR:

I worked on this, some months ago, when I had free time. I was stuck when implementing Global Lambdas for JSON context. So I decided to take some weeks to get fresh air, think about this and come back later.

So more recently, I came back on this and I realized that I do not really need to implement global lambdas for JSON context. I find another way to achieve what I want to do without this feature.

What I made here could be useful if someone wants to go further. I let you decide what to do with.

Thank you for your help, your time, your reviews and this awesome project.

[batiati]

Hello @tiawl!

I find another way to achieve what I want to do without this feature.

I'm curious about how you solved it!
I believe some use cases might be solved by lambda functions used as blocks, for example:

{{#lambda}}
  {{json_value}}
{{/lambda}}

The drawback is that the lambda function must operate over the string rendered from the JSON value, limiting the possibilities.

I let you decide what to do with.

Let's keep it open!
I still think we should land this PR, although I couldn't take the time to review it properly. But independently of the JSON thing, global lambdas are an excellent improvement (I still have some concerns about their usage).