Skip to content

Conversation

@bomgar
Copy link
Collaborator

@bomgar bomgar commented Aug 31, 2025

Use replacer to run replace

@bomgar
Copy link
Collaborator Author

bomgar commented Aug 31, 2025

There is a lot going on. Especially alpha and $(1|2|3). Why did you choose simple regex instead of a template engine? It would be a lot easier to extend go templates (granted the current syntax is a lot nicer).

@mvllow
Copy link
Member

mvllow commented Aug 31, 2025

There is a lot going on. Especially alpha and $(1|2|3). Why did you choose simple regex instead of a template engine? It would be a lot easier to extend go templates (granted the current syntax is a lot nicer).

At least in JavaScript, the regex felt the easiest when we started the build tool. The surface area was smaller and it was quicker than a .includes. When moving Go, I was/am not aware of alternatives like the one you implemented. Do you have any benchmarks for this new method?

@bomgar
Copy link
Collaborator Author

bomgar commented Aug 31, 2025

I pushed a benchmark. The new version is slightly worse with very small templates. It gets faster the larger the template becomes.

I meant this as an alternative: https://pkg.go.dev/text/template
I know it is probably to late to change this stuff. I was just curious because I think features like blending would benefit from registering functions {{ blend .Pine .Base 20 }}

@mvllow
Copy link
Member

mvllow commented Aug 31, 2025

I don't mind changing things if it makes them better, the risk is fairly low here. I'm thinking about user ergonomics, though, and $pine/20 feels nice.

@bomgar
Copy link
Collaborator Author

bomgar commented Aug 31, 2025

I agree. The current syntax is a lot nicer. It is probably sufficient, too. As I said I was mostly curious. Go templates are powerful but the syntax is kind of ugly.

@mvllow mvllow changed the title perf: don't process entire string for every variable refactor(perf): don't process entire string for every variable Nov 16, 2025
@mvllow mvllow changed the title refactor(perf): don't process entire string for every variable perf: don't process entire string for every variable Nov 16, 2025
@mvllow
Copy link
Member

mvllow commented Nov 16, 2025

Rebased and force pushed. I will try to add more tests to this PR before merging.

@mvllow mvllow merged commit bf1592a into main Nov 17, 2025
2 checks passed
@mvllow mvllow deleted the replace-once branch November 17, 2025 21:53
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.

3 participants