Review feedback.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
grafana · Nov 5, 2020 · 8c55441 · 8c55441
1 parent 2fe24d8
commit 8c55441
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 34 deletions.
diff --git a/pkg/logql/log/fmt.go b/pkg/logql/log/fmt.go
@@ -25,25 +25,27 @@ var (
 		"TrimSuffix": strings.TrimSuffix,
 		"TrimSpace":  strings.TrimSpace,
 
-		// new function ported from https://github.com/Masterminds/sprig/
-		"lower":     strings.ToLower,
-		"upper":     strings.ToUpper,
-		"title":     strings.Title,
-		"trunc":     trunc,
-		"substr":    substring,
-		"contains":  func(substr string, str string) bool { return strings.Contains(str, substr) },
-		"hasPrefix": func(substr string, str string) bool { return strings.HasPrefix(str, substr) },
-		"hasSuffix": func(substr string, str string) bool { return strings.HasSuffix(str, substr) },
-		"indent":    indent,
-		"nindent":   nindent,
-		"replace":   replace,
-		"repeat":    func(count int, str string) string { return strings.Repeat(str, count) },
-		"trim":      strings.TrimSpace,
-		// Switch order so that "$foo" | trimall "$"
-		"trimAll":    func(a, b string) string { return strings.Trim(b, a) },
-		"trimSuffix": func(a, b string) string { return strings.TrimSuffix(b, a) },
-		"trimPrefix": func(a, b string) string { return strings.TrimPrefix(b, a) },
-
+		// New function ported from https://github.com/Masterminds/sprig/
+		// Those function takes the string as the last parameter, allowing pipe chaining.
+		// Example: .mylabel | lower | substring 0 5
+		"lower":      strings.ToLower,
+		"upper":      strings.ToUpper,
+		"title":      strings.Title,
+		"trunc":      trunc,
+		"substr":     substring,
+		"contains":   contains,
+		"hasPrefix":  hasPrefix,
+		"hasSuffix":  hasSuffix,
+		"indent":     indent,
+		"nindent":    nindent,
+		"replace":    replace,
+		"repeat":     repeat,
+		"trim":       strings.TrimSpace,
+		"trimAll":    trimAll,
+		"trimSuffix": trimSuffix,
+		"trimPrefix": trimPrefix,
+
+		// regex functions
 		"regexReplaceAll": func(regex string, s string, repl string) string {
 			r := regexp.MustCompile(regex)
 			return r.ReplaceAllString(s, repl)
@@ -186,11 +188,13 @@ func (lf *LabelsFormatter) Process(l []byte, lbs *LabelsBuilder) ([]byte, bool)
 }
 
 func trunc(c int, s string) string {
-	if c < 0 && len(s)+c > 0 {
-		return s[len(s)+c:]
+	runes := []rune(s)
+	l := len(s)
+	if c < 0 && l+c > 0 {
+		return string(runes[l+c:])
 	}
-	if c >= 0 && len(s) > c {
-		return s[:c]
+	if c >= 0 && l > c {
+		return string(runes[:c])
 	}
 	return s
 }
@@ -203,24 +207,40 @@ func trunc(c int, s string) string {
 //
 // Otherwise, this calls string[start, end].
 func substring(start, end int, s string) string {
+	runes := []rune(s)
+	l := len(runes)
+	if end > l {
+		end = l
+	}
+	if start > l {
+		start = l
+	}
 	if start < 0 {
-		return s[:end]
+		if end < 0 {
+			return ""
+		}
+		return string(runes[:end])
 	}
-	if end < 0 || end > len(s) {
-		return s[start:]
+	if end < 0 {
+		return string(runes[start:])
 	}
-	return s[start:end]
-}
-
-func replace(old, new, src string) string {
-	return strings.Replace(src, old, new, -1)
+	if start > end {
+		return ""
+	}
+	return string(runes[start:end])
 }
 
+func contains(substr string, str string) bool  { return strings.Contains(str, substr) }
+func hasPrefix(substr string, str string) bool { return strings.HasPrefix(str, substr) }
+func hasSuffix(substr string, str string) bool { return strings.HasSuffix(str, substr) }
+func repeat(count int, str string) string      { return strings.Repeat(str, count) }
+func replace(old, new, src string) string      { return strings.Replace(src, old, new, -1) }
+func trimAll(a, b string) string               { return strings.Trim(b, a) }
+func trimSuffix(a, b string) string            { return strings.TrimSuffix(b, a) }
+func trimPrefix(a, b string) string            { return strings.TrimPrefix(b, a) }
 func indent(spaces int, v string) string {
 	pad := strings.Repeat(" ", spaces)
 	return pad + strings.Replace(v, "\n", "\n"+pad, -1)
 }
 
-func nindent(spaces int, v string) string {
-	return "\n" + indent(spaces, v)
-}
+func nindent(spaces int, v string) string { return "\n" + indent(spaces, v) }
diff --git a/pkg/logql/log/fmt_test.go b/pkg/logql/log/fmt_test.go
@@ -240,3 +240,52 @@ func Test_validate(t *testing.T) {
 		})
 	}
 }
+
+func Test_trunc(t *testing.T) {
+	tests := []struct {
+		s    string
+		c    int
+		want string
+	}{
+		{"Hello, 世界", -1, "界"},
+		{"Hello, 世界", 1, "H"},
+		{"Hello, 世界", 0, ""},
+		{"Hello, 世界", 20, "Hello, 世界"},
+		{"Hello, 世界", -20, "Hello, 世界"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.s, func(t *testing.T) {
+			if got := trunc(tt.c, tt.s); got != tt.want {
+				t.Errorf("trunc() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_substring(t *testing.T) {
+
+	tests := []struct {
+		start int
+		end   int
+		s     string
+		want  string
+	}{
+		{1, 8, "Hello, 世界", "ello, 世"},
+		{-10, 8, "Hello, 世界", "Hello, 世"},
+		{1, 10, "Hello, 世界", "ello, 世界"},
+		{-1, 10, "Hello, 世界", "Hello, 世界"},
+		{-1, 1, "Hello, 世界", "H"},
+		{-1, -1, "Hello, 世界", ""},
+		{20, -1, "Hello, 世界", ""},
+		{1, 1, "Hello, 世界", ""},
+		{5, 1, "Hello, 世界", ""},
+		{3, -1, "Hello, 世界", "lo, 世界"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.s, func(t *testing.T) {
+			if got := substring(tt.start, tt.end, tt.s); got != tt.want {
+				t.Errorf("substring() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}