Skip to content

Commit 5b89670

Browse files
authored
Use a general Eval function for expressions in templates. (#23927)
One of the proposals in #23328 This PR introduces a simple expression calculator (templates/eval/eval.go), it can do basic expression calculations. Many untested template helper functions like `Mul` `Add` can be replaced by this new approach. Then these `Add` / `Mul` / `percentage` / `Subtract` / `DiffStatsWidth` could all use this `Eval`. And it provides enhancements for Golang templates, and improves readability. Some examples: ---- * Before: `{{Add (Mul $glyph.Row 12) 12}}` * After: `{{Eval $glyph.Row "*" 12 "+" 12}}` ---- * Before: `{{if lt (Add $i 1) (len $.Topics)}}` * After: `{{if Eval $i "+" 1 "<" (len $.Topics)}}` ## FAQ ### Why not use an existing expression package? We need a highly customized expression engine: * do the calculation on the fly, without pre-compiling * deal with int/int64/float64 types, to make the result could be used in Golang template. * make the syntax could be used in the Golang template directly * do not introduce too much complex or strange syntax, we just need a simple calculator. * it needs to strictly follow Golang template's behavior, for example, Golang template treats all non-zero values as truth, but many 3rd packages don't do so. ### What's the benefit? * Developers don't need to add more `Add`/`Mul`/`Sub`-like functions, they were getting more and more. Now, only one `Eval` is enough for all cases. * The new code reads better than old `{{Add (Mul $glyph.Row 12) 12}}`, the old one isn't familiar to most procedural programming developers (eg, the Golang expression syntax). * The `Eval` is fully covered by tests, many old `Add`/`Mul`-like functions were never tested. ### The performance? It doesn't use `reflect`, it doesn't need to parse or compile when used in Golang template, the performance is as fast as native Go template. ### Is it too complex? Could it be unstable? The expression calculator program is a common homework for computer science students, and it's widely used as a teaching and practicing purpose for developers. The algorithm is pretty well-known. The behavior can be clearly defined, it is stable.
1 parent ecf34fc commit 5b89670

File tree

15 files changed

+529
-157
lines changed

15 files changed

+529
-157
lines changed

modules/base/tool.go

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -141,55 +141,6 @@ func FileSize(s int64) string {
141141
return humanize.IBytes(uint64(s))
142142
}
143143

144-
// Subtract deals with subtraction of all types of number.
145-
func Subtract(left, right interface{}) interface{} {
146-
var rleft, rright int64
147-
var fleft, fright float64
148-
isInt := true
149-
switch v := left.(type) {
150-
case int:
151-
rleft = int64(v)
152-
case int8:
153-
rleft = int64(v)
154-
case int16:
155-
rleft = int64(v)
156-
case int32:
157-
rleft = int64(v)
158-
case int64:
159-
rleft = v
160-
case float32:
161-
fleft = float64(v)
162-
isInt = false
163-
case float64:
164-
fleft = v
165-
isInt = false
166-
}
167-
168-
switch v := right.(type) {
169-
case int:
170-
rright = int64(v)
171-
case int8:
172-
rright = int64(v)
173-
case int16:
174-
rright = int64(v)
175-
case int32:
176-
rright = int64(v)
177-
case int64:
178-
rright = v
179-
case float32:
180-
fright = float64(v)
181-
isInt = false
182-
case float64:
183-
fright = v
184-
isInt = false
185-
}
186-
187-
if isInt {
188-
return rleft - rright
189-
}
190-
return fleft + float64(rleft) - (fright + float64(rright))
191-
}
192-
193144
// EllipsisString returns a truncated short string,
194145
// it appends '...' in the end of the length of string is too large.
195146
func EllipsisString(str string, length int) string {

modules/base/tool_test.go

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -114,45 +114,6 @@ func TestFileSize(t *testing.T) {
114114
assert.Equal(t, "2.0 EiB", FileSize(size))
115115
}
116116

117-
func TestSubtract(t *testing.T) {
118-
toFloat64 := func(n interface{}) float64 {
119-
switch v := n.(type) {
120-
case int:
121-
return float64(v)
122-
case int8:
123-
return float64(v)
124-
case int16:
125-
return float64(v)
126-
case int32:
127-
return float64(v)
128-
case int64:
129-
return float64(v)
130-
case float32:
131-
return float64(v)
132-
case float64:
133-
return v
134-
default:
135-
return 0.0
136-
}
137-
}
138-
values := []interface{}{
139-
int(-3),
140-
int8(14),
141-
int16(81),
142-
int32(-156),
143-
int64(1528),
144-
float32(3.5),
145-
float64(-15.348),
146-
}
147-
for _, left := range values {
148-
for _, right := range values {
149-
expected := toFloat64(left) - toFloat64(right)
150-
sub := Subtract(left, right)
151-
assert.InDelta(t, expected, sub, 1e-3)
152-
}
153-
}
154-
}
155-
156117
func TestEllipsisString(t *testing.T) {
157118
assert.Equal(t, "...", EllipsisString("foobar", 0))
158119
assert.Equal(t, "...", EllipsisString("foobar", 1))

0 commit comments

Comments
 (0)