Add split and join to templates #265
Conversation
|
OK, added In your hands! |
| "join": join, | ||
| "replace": replace, | ||
| "regexReplace": regexReplace, | ||
| "regexpReplace": regexReplace, |
There was a problem hiding this comment.
Why the alias? If we were going to have more than one regex option at all I'd rather we match what consul-template did with having both a regexMatch and regexReplaceAll.
If we don't want to implement the regexMatch bool (which doesn't seem unreasonable), then having the name be regexReplaceAll has the added advantage of giving us room to have a "replace n" option down the road.
There was a problem hiding this comment.
Why the alias?
Um... just because? In all honesty, my first run of tests failed because I made that mistake! Seemed easiest to fix it by aliasing.
I don't care much one way or the other. If we want to use template's replaceAll, regexMatch, regexReplaceAll, I am fine with that. Whatever you want, just let me know, happy to modify, rerun tests and push out.
There was a problem hiding this comment.
Let's match consul-template, it'll be easier for end users.
There was a problem hiding this comment.
FYI, consul template uses replaceAll instead of replace, so if we are going to match, that one too?
|
Just had the one comment about the field name but other than that LGTM. |
…e, remove regexpReplace
|
Changed the field names as requested. Let's leave |
|
LGTM. Will merge once Travis gives us the 👍 |
I will try to add the
replace*ones after, but here is a good start. Obviously includes a test line as well. I like how I just tested both in one line. :-)Per the request for PR from @tgross at #261 (comment)