Replace github.com/masterminds/sprig template functions

[twpayne]

Is your feature request related to a problem? Please describe.

github.com/masterminds/sprig, used by chezmoi, has unfortunately become a popular library of template functions despite several critical issues:

• It is unmaintained.
• The argument order of its functions is incompatible with Go template's pipelining (see Add more config management functions #2215).
• Its naming convention does not match Go's (e.g. toJson should be toJSON).

Describe the solution you'd like

chezmoi's template functions should be up-to-date, compatible with Go template pipelining, and match Go's naming conventions.

Describe alternatives you've considered

Keeping minorminds/sprig template functions as-is.

Additional context

A smooth transition should be provided, with both old and replacement template functions being available to users for some time. Users can use .chezmoiversion to help the transition.

[bradenhilton]

Perhaps we could also take the opportunity to implement a higher-order function (which would add to the funcMap, not something done at runtime) similar to the one in this issue?

As you know, there is the quoteList template function, which was implemented explicitly. So far, there hasn't been much of a demand sufficient to add similar functions, which would probably be necessary to add an explicit function (and thus create clutter and perhaps some complexity).

An each function separates the iteration logic from the action being taken on the item in the list. Going forward, implementing stringFuncEach would likely be as simple as implementing stringFunc and then setting funcMap["stringFuncEach"] to something like eachFunc(stringFunc).

The each{Func} functions could be as generic as possible or type specific if needed.

Maybe this could even be done dynamically with reflection? So we could do something like:

1. Group functions by type if needed
2. Iterate through each function/group
3. Get function name (with reflection?)
4. funcMap[funcName + "Each"] = eachFunc{Type}(func) (probably not syntactically correct)

Separating the logic would remove the need for explicit functions in all but the most specialized cases, which would remove clutter and be trivial to extend as new functions are added.

The alternative, which would be better, but also likely increase complexity, would be to add iteration logic to every requisite function, so that you can pass it a single item or a list of items and get the expected output, e.g.

$string | quote
"foobarbaz"


$list | quote
["foo", "bar", "baz"]

This approach would be easier if we had control over how each function is implemented, as sprig is essentially a black box in this case.

Likely v3 because this would mean removing/replacing/renaming quoteList at the very least.

Interested in any thoughts anyone might have.

[twpayne]

Perhaps we could also take the opportunity to implement a higher-order function

The tricky thing here is that text/template is, by design, a simple language and does not support higher-order functions, and is unlikely to ever do so in the future. So, without permanently forking text/template (which would be an immeasurable burden), this is not possible to do.

[bradenhilton]

Doesn't it just add function permutations to the funcMap?

I know it's probably not possible to do something like

{{ $list | map | squote }}

but that is different, no?

I tried dropping the example in the linked issue in to test but it's not compatible without some tweaks (...interface{} vs []string etc.) and I ran out of time that day so I just left it there.

Are you saying it's impossible to do something like

funcMap["squoteEach"] = each(squote)
funcMap["add1Each"] = each(add1)

without a text/template fork?

Making all template functions list-aware somehow is probably more user friendly anyway, but I'm not sure exactly what that implementation would look like.

Should we create a funcs repo in the chezmoi organization?

[twpayne]

Ah, sorry I misunderstood. My misunderstanding was that I thought that each was to be a template function that took a function and a list as arguments and returned a new list of the function applied to each element in the list. This isn't possible with text/template as text/template does not support functions as values.

If I now I understand correctly, each should be a Go function that makes it easier to write template functions that apply a Go function to every element in a list. This is certainly possible, and doesn't need to wait for v3 as it's an internal implementation detail that is not visible to users (with the exception of a few more template functions being added).

I have to ask, though: given that text/template already supports range/end actions, how much are these extra functions really needed? quoteList is certainly useful as you often need to quote multiple strings for command arguments, but what other functions do you anticipate being regularly useful?

[twpayne]

Note it's also possible to extend template functions so that they operate on lists as well as isolated values. For example, we can extend quote to return a list of quoted strings if it is passed a list of strings, without breaking its existing functionality of returning a quoted string if passed a single string.

Should we create a funcs repo in the chezmoi organization?

Hmm, yes. I'll create a templatefuncs repo with some initial ideas.

[bradenhilton]

Note it's also possible to extend template functions so that they operate on lists as well as isolated values.

I think we should just do this. I'd argue that {{ $list | quote }} implicitly iterating through the list and quoting each element is expected behavior (as opposed to something like implicitly joining the elements, then quoting) and doesn't increase the number of template functions.

[twpayne]

So, sadly, thanks to Masterminds/sprig's inherent brokenness, this isn't as immediately fixable as I had hoped. Some specific problems:

• Masterminds/sprig's quote function already accepts lists ... and quotes them:

  $ chezmoi execute-template '{{ list "a" "b" | quote }}'
  "[a b]"

  so changing quote's behavior for lists will be a backwards-incompatible change.
• Masterminds/sprig just seems more and more broken the more I look at it. For example, here's the implementation of squote. It's not even slightly correct: strings with a single quote in them are not correctly escaped 🤯 .

So, replacing the template library is definitely something for chezmoi v3, as trying to maintain backwards compatibility with all of Minorminds stupidity is just not possible.

[bradenhilton]

Also, just to clarify as I didn't do it before...

I have to ask, though: given that text/template already supports range/end actions, how much are these extra functions really needed?

You're absolutely right, especially if the solution was to add *Each permutations of each function to the funcMap, but if we control the implementations of the functions this is a non-issue, as we can make them list-aware. This has some advantages over range/end:

1. Improved template readability. range/end require whitespace management. Often I want to do something like this:

   {{ range $list }}
       {{ . | quote }}
   {{ end }}
   

   for readability, but the whitespace is included in the output, so it must be removed with the control characters. This is one of the simplest examples I could come up with and it's still not easy to reason about at a glance in my opinion.

   Also, it's not like this approach would become unusable, it would still work perfectly as a backup or a preference depending on the user.

   I'm not sure if I can articulate this properly, but this approach is also entirely focused on the output, it doesn't affect the $list variable in any way, so it's difficult to pipe the result to something else. Speaking of which...

2. Improved pipelining. This removes the need for temporary variables/variable re-assignment/multi-line actions etc., which should allow something like the following:

   {{ $numList | add1 | quote }}  # here I am assuming that the functions work correctly with lists
   

[twpayne]

https://github.com/chezmoi/templatefuncs contains some initial replacement template functions. It's a work in progress.

[twpayne]

I asked on golang-nuts if there are existing alternatives to Minorminds/sprig.

[halostatue]

I was looking at gomplate the other day and it seems to me that it has a few nice things about it and may be a viable replacement for sprig in a number of places; it has other features that aren't as applicable to Chezmoi (but many of them would be, I think).

The biggest thing that I liked about it is that it exposes objects rather than functions (although some functions are common enough to expose aliases), so for v3 (#2673) it would be possible to have onepassword.Details instead of onepasswordDetails &c.

[bradenhilton]

@halostatue I'd personally rather take inspiration from gomplate than integrate it into chezmoi.

If gomplate went the way of sprig, we would just find ourselves in this situation again. Creating our own bespoke function library allows us to fix and extend things quickly, rather than having to patch in our own fixes and create discrepancies like the one mentioned recently in #3335:

On a related note, toPrettyJson is listed on chezmoi's website, which suggests that you're rolling your own implementation instead of just using sprig's. If that suggestion is false then I recommend removing that page.

Using objects just isn't necessary for some use cases. Would we really benefit from having something like strings.ToUpper instead of toUpper? The aliases would just create clutter in the FuncMap. You also end up with awkward names like conv.ToString and coll.Dict to avoid having the names be too verbose.

[halostatue]

For some operations, sure having both the alias and the full version would be a waste. On the other hand, collecting password manager functionality into objects with functions available would IMO be much cleaner.

Providing migration steps for this would be vital (e.g., reimplement the 1Password functionality as onepassword.* or op.*, but put the aliases in place; start warning about the use of those aliases in a future release, and remove them at Chezmoi 3).

The warning phase could even be done as "light" (the use of a deprecated function/alias warns once) and "heavy" (each use of a deprecated function/alias warns).

This sort of approach would allow us to think about what functions should be quickly available (toString, toUpper, toJson, fromJson, etc.) and what functions should be grouped together.

I may put together a list of the functions available in chezmoi via sprig or direct and suggest some sort of organization.

[bradenhilton]

Generally I agree, but I think we need to get https://github.com/chezmoi/templatefuncs in a much more complete state before we even think about that.

I think v3 should be a clean break, and we should be as noisy as would be reasonable about that leading up to its release (once the new features are nearing completion).

[twpayne]

So, I propose to make this change in an incremental way, without needing chezmoi v3, with the following strategy:

1. Add a template.functions config variable which is a list of strings in priority order.
2. The list element github.com/masterminds/sprig ("sprig") corresponds to the existing minorminds/sprig functions and the list element github.com/chezmoi/templatefuncs corresponds to https://github.com/chezmoi/templatefuncs ("templatefuncs").
3. The default value of template.functions will be github.com/masterminds/sprig.
4. Users can add templatefuncs to the end of the list, but sprig functions will take priority.
5. When the user has updated their templates to use templatefuncs they can move templatefuncs to the head of the list.
6. When the user no longer relies on any sprig functions they can remove sprig from the list.
7. In chezmoi v3 we remove the sprig option.

This should allow an orderly transition from sprig to templatefuncs, and of course the user can use chezmoi diff and chezmoi's error reporting to catch any problems along the way. As template.functions will be set in the user's configuration file, they can even use different template functions on different systems (if they are mad enough).

@bradenhilton thoughts?

[halostatue]

This seems like a reasonable approach to me.

[bradenhilton]

@twpayne How do you propose we handle the current chezmoi-specific functions, especially functions such as ioreg and our overridden sprig functions? My understanding was that ioreg would be removed, and the rest would eventually be rolled into templatefuncs when ready, but allowing templatefuncs now complicates that slightly.

I do think incremental adoption is a good idea.

[2-4601]

Just came across this fork of sprig: https://github.com/go-sprout/sprout

Here's an issue with some discussion on roadmap: go-sprout/sprout#1
More discussion on improvements here: https://github.com/orgs/go-sprout/discussions

I do not know if sprout aims to solve the problems raised in the first post of this issue. This is just a FYI :)