Repository: StackExchange/dnscontrol
Pull request #2508 (2023-08-11 22:00:25)

Strip ANSI color codes from notifications
Not very long ago, ansi color support was enabled in dnscontrol, to ease reviewing of changesets.

This doesn't play well with notifications, as they usually can't render these codes; slack for example outputs the escape codes, leading to ugly and hard to grasp messages. See #2499 for a few examples.

This changeset ensures that any message coming to a notifier will get stripped of ansi color codes, before passed on to the notifier.

That way, notifiers are then free to re-add colors in a way that is supported in the relevant platform.

<!--
Before you submit a pull request, please make sure you've run the following commands from the root directory.

go vet ./...
go fmt ./...
go generate ./...
go mod tidy
!-->



cafferata: Looks good and works as I would expect.
costasd: should be good now.

Patch:
diff --git a/pkg/notifications/notifications.go b/pkg/notifications/notifications.go
index 2a29490b..c454cfd7 100644
--- a/pkg/notifications/notifications.go
+++ b/pkg/notifications/notifications.go
@@ -1,5 +1,7 @@
 package notifications
 
+import "regexp"
+
 // Notifier is a type that can send a notification
 type Notifier interface {
 	// Notify will be called after a correction is performed.
@@ -14,6 +16,9 @@ type Notifier interface {
 // new notification types should add themselves to this array
 var initers = []func(map[string]string) Notifier{}
 
+// matches ansi color codes
+var ansiColorRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`)
+
 // Init will take the given config map (from creds.json notifications key) and create a single Notifier with
 // all notifications it has full config for.
 func Init(config map[string]string) Notifier {
@@ -29,9 +34,20 @@ func Init(config map[string]string) Notifier {
 
 type multiNotifier []Notifier
 
+// removes any ansi color codes from a given string
+func stripAnsiColors(colored string) string {
+	return ansiColorRegex.ReplaceAllString(colored, "")
+}
+
 func (m multiNotifier) Notify(domain, provider string, message string, err error, preview bool) {
+
+	// force-remove ansi colors that might come with the message from dnscontrol.
+	// These usually don't render well in notifiers, outputting escape codes.
+	// If a notifier wants to output colors, they should probably implement
+	// them natively.
+	nMsg := stripAnsiColors(message)
 	for _, n := range m {
-		n.Notify(domain, provider, message, err, preview)
+		n.Notify(domain, provider, nMsg, err, preview)
 	}
 }
 func (m multiNotifier) Done() {


Test patch:
diff --git a/pkg/notifications/notifications_test.go b/pkg/notifications/notifications_test.go
new file mode 100644
index 00000000..456302f6
--- /dev/null
+++ b/pkg/notifications/notifications_test.go
@@ -0,0 +1,25 @@
+package notifications
+
+import "testing"
+
+func Test_stripAnsiColorsValid(t *testing.T) {
+
+	coloredStr := "\x1b[0133myellow\x1b[0m" // 33 == yellow
+	nonColoredStr := "yellow"
+
+	s := stripAnsiColors(coloredStr)
+	if s != nonColoredStr {
+		t.Errorf("stripAnsiColors() stripped %q different from %q", coloredStr, nonColoredStr)
+	}
+}
+
+func Test_stripAnsiColorsInvalid(t *testing.T) {
+
+	coloredStr := "\x1b[01AAmyellow\x1b[0m" // AA not a real color
+	nonColoredStr := "yellow"
+
+	s := stripAnsiColors(coloredStr)
+	if s == nonColoredStr {
+		t.Errorf("stripAnsiColors() stripped %q should be different from %q", coloredStr, nonColoredStr)
+	}
+}