Templating functions changes (Follow up to #639)

[jippi]

Hey,

Following up from #639 where I submitted template function support to the config file.

I came back to this feature today in another repository, and I ended up wasting a solid 2h debugging why

dir: 'internal/mocks/{{ .InterfaceDirRelative | trimPrefix "internal/" }}'

didn't work as expected., turns out that it had to be

dir: 'internal/mocks/{{ trimPrefix .InterfaceDirRelative "internal/" }}' (ordering of the args) instead...

Which was not what my muscle memory from tools like consul-template trimPrefix.

I wanted to submit this ticket to discuss if we should change the behavior to work like "normal" templating filters like "most" of the Go and Python eco-system do or leave it "as is".

I'm happy to submit the change if you think it's work doing :)

[LandonTClipp]

Is this happening because the filter is adding the value as the second argument to trimPrefix? It seems to be the case: https://pkg.go.dev/text/template#hdr-Pipelines

Unfortunately as already being part of the public API, I'm not sure we could simply tell mockery to "swap" the arguments if that breaks the bare function call like {{ trimPrefix .InterfaceDirRelative "internal/" }}. I was really worried about what consul is doing and unfortunately my fears were justified: https://github.com/hashicorp/consul-template/blob/74fa6ffdcec51ade85a8a2008254868d5b670447/template/funcs.go#L1035

That's sad, I didn't have the foresight to see this before the PR was merged. I'm not sure what a good non-breaking solution would be. I suppose we could hide the "correct" behavior behind a feature flag 🤷🏻

[jippi]

I wonder if we can make it compatible by sniffing number of arguments (or an empty 2nd arg) in trimPrefix and adjust the behavior / raise an error from there - with a thin wrapper around it - could even make it a migration path to detect empty 2nd arg and emit warning and deprecate the old? 🤔

[LandonTClipp]

I might just mark this as something to be fixed in v3, I don't like getting into argument sniffing trickery because it just gets convoluted. If you really want, we could hide behind a feature flag and deprecate not having the feature flag turned on? Ugh. On the other hand, how useful is the filter syntax really going to be to people at large? As long as we document that it's not supported, is it really going to be an issue for people?

[jippi]

Works for me - I don't love it either - but the frustration today was real, mostly because it was entirely self-inflected at both consumer and producer side of the problem - no one else to blame 😆

I'm fine with a v3 thing (also happy to do the work at that time), but I think it would be fine either way. I just love when things "looks and feels" the same across my tooling - the mental overhead (clearly, in my case) is real when it doesn't :)

Might just be my personal bias - I'm honestly fine either way - just wanted to flag this to you for thoughts :)

[LandonTClipp]

I'll think about it, if you really really want to, I'd be fine with accepting a feature flag and defaulting to use that in v3, since it is indeed how most other filtering things work. Otherwise I think just documenting it would be sufficient for most people.

[LandonTClipp]

@jalaziz I see, so you probably need something like this (theoretical example)?

dir: "{{ split .InterfaceDirRelative '/' | index 0 }}/mocks/{{ split .InterfaceDirRelative '/' | slice 1 | join '/' }}"

Actually, the only thing in that template that wouldn't work is the join. The other functions I believe would work since those are globally defined by text/template.

I can see the need for it, I'm truly okay with adding a flag like template_pipeline: true that will swap the args and just noting it in the docs.

[jalaziz]

I ended up getting it to work with:

{{ index (splitAfterN .InterfaceDirRelative "/" 2) 0 }}/mocks/{{ index (splitAfterN .InterfaceDirRelative "/" 2) 1 }}'

It's not pretty, but it works. Defining it as a pipeline would be slightly more readable.

Actually, the only thing in that template that wouldn't work is the join. The other functions I believe would work since those are globally defined by text/template.

As far as I can tell, index isn't defined to be pipeline friendly :(.