Repository: mbrt/gmailctl (go)

Issue:
Easier-to-parse diff
Currently gmailctl diff can sometimes show output like this:

```
- to: {long@gmail.com list@gmail.com of@gmail.com aliases@gmail.com that@gmail.com apply@gmail.com to@gmail.com this@gmail.com rule@gmail.com}
+ to: {long@gmail.com list@gmail.com of@gmail.com aliases@gmail.com that@gmail.com apply@gmail.com this@gmail.com rule@gmali.com}
```

...it's not fun figuring out what's missing there.

The cmp package does what I mean, I think: https://go.dev/play/p/E5RwmDYIsvX -

```
  main.WithSlices{
  	To: []string{
  		... // 4 identical elements
  		"that@gmail.com",
  		"apply@gmail.com",
- 		"to@gmail.com",
  		"this@gmail.com",
  		"rule@gmail.com",
  	},
  }
```

Pull request description:
Indent filter criteria
Makes for easier-to-read diffs.

Close #221.

Patch:
diff --git a/internal/engine/filter/filter.go b/internal/engine/filter/filter.go
index ffc585e..3ee757c 100644
--- a/internal/engine/filter/filter.go
+++ b/internal/engine/filter/filter.go
@@ -1,7 +1,9 @@
 package filter
 
 import (
+	"bytes"
 	"fmt"
+	"io"
 	"strings"
 
 	"github.com/mbrt/gmailctl/internal/engine/gmail"
@@ -50,7 +52,8 @@ func (f Filter) String() string {
 	w.WriteParam("from", f.Criteria.From)
 	w.WriteParam("to", f.Criteria.To)
 	w.WriteParam("subject", f.Criteria.Subject)
-	w.WriteParam("query", f.Criteria.Query)
+
+	w.WriteParam("query", indent(f.Criteria.Query, 2))
 
 	w.WriteString("  Actions:\n")
 	w.WriteBool("archive", f.Action.Archive)
@@ -67,6 +70,93 @@ func (f Filter) String() string {
 	return w.String()
 }
 
+func indent(query string, level int) string {
+	var indented bytes.Buffer
+	if !indentInternal(strings.NewReader(query), &indented, level+1) {
+		return query
+	}
+	return "\n" + strings.TrimRight(indented.String(), "\n ")
+}
+
+func indentInternal(queryReader io.RuneReader, out *bytes.Buffer, level int) bool {
+	type parseState int
+	const (
+		other parseState = iota
+		skipSpaces
+		inQuotes
+	)
+
+	for i := 0; i < level; i++ {
+		out.Write([]byte("  "))
+	}
+
+	indentationWasNeeded := false
+	writeIndentation := func(n int) {
+		out.WriteByte('\n')
+		for i := 0; i < n; i++ {
+			out.Write([]byte("  "))
+		}
+		indentationWasNeeded = true
+	}
+
+	state := skipSpaces
+
+	for {
+		r, _, err := queryReader.ReadRune()
+		if err != nil {
+			break
+		}
+		switch state {
+		case inQuotes:
+			out.WriteRune(r)
+			if r == '"' {
+				state = other
+			}
+		case skipSpaces, other:
+			switch r {
+			case ' ':
+				if state == skipSpaces {
+					continue
+				}
+				writeIndentation(level)
+
+			case '{', '(':
+				out.WriteRune(r)
+
+				level++
+				writeIndentation(level)
+
+				state = skipSpaces
+
+			case '}', ')':
+				writeIndentation(level - 1)
+				out.WriteRune(r)
+
+				level--
+				writeIndentation(level)
+
+				state = skipSpaces
+
+			case ':':
+				out.WriteByte(':')
+				state = skipSpaces
+
+			case '"':
+				state = inQuotes
+				out.WriteByte('"')
+
+			default:
+				if state != inQuotes {
+					state = other
+				}
+				out.WriteRune(r)
+			}
+		}
+	}
+
+	return indentationWasNeeded
+}
+
 // HasLabel returns true if the given label is used by the filter.
 func (f Filter) HasLabel(name string) bool {
 	return f.Action.AddLabel == name


Test patch:
diff --git a/internal/engine/filter/diff_test.go b/internal/engine/filter/diff_test.go
index d7fc0f5..68611d8 100644
--- a/internal/engine/filter/diff_test.go
+++ b/internal/engine/filter/diff_test.go
@@ -43,7 +43,8 @@ func TestDiffOutput(t *testing.T) {
 		{
 			ID: "abcdefg",
 			Criteria: Criteria{
-				From: "someone@gmail.com",
+				From:  "someone@gmail.com",
+				Query: "(a b) subject:(foo bar)",
 			},
 			Action: Actions{
 				MarkRead: true,
@@ -54,7 +55,8 @@ func TestDiffOutput(t *testing.T) {
 	new := Filters{
 		{
 			Criteria: Criteria{
-				From: "{someone@gmail.com else@gmail.com}",
+				From:  "{someone@gmail.com else@gmail.com}",
+				Query: "(a c) subject:(foo baz)",
 			},
 			Action: Actions{
 				MarkRead: true,
@@ -69,10 +71,21 @@ func TestDiffOutput(t *testing.T) {
 	expected := `
 --- Current
 +++ TO BE APPLIED
-@@ -1,6 +1,6 @@
+@@ -1,15 +1,15 @@
  * Criteria:
 -    from: someone@gmail.com
 +    from: {someone@gmail.com else@gmail.com}
+     query: 
+       (
+         a
+-        b
++        c
+       )
+       subject:(
+         foo
+-        bar
++        baz
+       )
    Actions:
      mark as read
      categorize as: personal`
@@ -322,3 +335,45 @@ func TestDuplicate(t *testing.T) {
 	// Only one of the two identical filters is present
 	assert.Equal(t, new[1:], fd.Added)
 }
+
+func TestIndent(t *testing.T) {
+	testCases := []struct{ name, query, want string }{
+		{"no_newline_necessary", `from:"foo bar"`, `from:"foo bar"`},
+		{"quotes", `from:"foo bar" "another foo bar" "good thing gmail doesn't support escaping" "re: something"`, `
+  from:"foo bar"
+  "another foo bar"
+  "good thing gmail doesn't support escaping"
+  "re: something"`},
+		{"parens", `(a b) subject:(foo bar "re: something")`, `
+  (
+    a
+    b
+  )
+  subject:(
+    foo
+    bar
+    "re: something"
+  )`},
+		{"negations_and_braces", `-(x {y -z} l)`, `
+  -(
+    x
+    {
+      y
+      -z
+    }
+    l
+  )`},
+		{"unicode", `日(本)語`, `
+  日(
+    本
+  )
+  語`},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := indent(tc.query, 0)
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}
diff --git a/testdata/valid/02-add-one.diff b/testdata/valid/02-add-one.diff
index 7a0e563..4f0a40d 100644
--- a/testdata/valid/02-add-one.diff
+++ b/testdata/valid/02-add-one.diff
@@ -1,7 +1,7 @@
 Filters:
 --- Current
 +++ TO BE APPLIED
-@@ -1 +1,119 @@
+@@ -1 +1,123 @@
 +* Criteria:
 +    query: replyto:replyer@gmail.com
 +  Actions:
@@ -59,7 +59,9 @@ Filters:
 +    apply label: label2
 +
 +* Criteria:
-+    query: cc:peeker@yahoo.com -subject:"a subject"
++    query: 
++      cc:peeker@yahoo.com
++      -subject:"a subject"
 +  Actions:
 +    archive
 +    mark as important
@@ -83,7 +85,9 @@ Filters:
 +    forward to: forward-address@gmail.com
 +
 +* Criteria:
-+    query: cc:peeker@yahoo.com -subject:"a subject"
++    query: 
++      cc:peeker@yahoo.com
++      -subject:"a subject"
 +  Actions:
 +    apply label: label2
 +
diff --git a/testdata/valid/03-labels.diff b/testdata/valid/03-labels.diff
index 875017d..873ae37 100644
--- a/testdata/valid/03-labels.diff
+++ b/testdata/valid/03-labels.diff
@@ -1,7 +1,7 @@
 Filters:
 --- Current
 +++ TO BE APPLIED
-@@ -5,28 +5,26 @@
+@@ -7,28 +7,26 @@
      mark as important
      never mark as spam
      mark as read
@@ -31,7 +31,7 @@ Filters:
  * Criteria:
      query: replyto:replyer@gmail.com
    Actions:
-@@ -34,11 +32,10 @@
+@@ -36,11 +34,10 @@
      mark as important
      never mark as spam
      mark as read
@@ -43,7 +43,7 @@ Filters:
  * Criteria:
      query: bcc:bccer@gmail.com
    Actions:
-@@ -46,11 +43,10 @@
+@@ -48,11 +45,10 @@
      mark as important
      never mark as spam
      mark as read
@@ -55,7 +55,7 @@ Filters:
  * Criteria:
      from: someone@gmail.com
    Actions:
-@@ -58,11 +54,10 @@
+@@ -60,11 +56,10 @@
      mark as important
      never mark as spam
      mark as read
@@ -67,7 +67,7 @@ Filters:
  * Criteria:
      query: is:muted
    Actions:
-@@ -70,11 +65,10 @@
+@@ -72,11 +67,10 @@
      mark as important
      never mark as spam
      mark as read
@@ -79,7 +79,7 @@ Filters:
  * Criteria:
      to: someone-else@gmail.com
    Actions:
-@@ -82,43 +76,7 @@
+@@ -84,45 +78,7 @@
      mark as important
      never mark as spam
      mark as read
@@ -109,7 +109,9 @@ Filters:
 -    apply label: label2
 -
 -* Criteria:
--    query: cc:peeker@yahoo.com -subject:"a subject"
+-    query: 
+-      cc:peeker@yahoo.com
+-      -subject:"a subject"
 -  Actions:
 -    apply label: label2
 -
diff --git a/testdata/valid/05-bigdiff.diff b/testdata/valid/05-bigdiff.diff
index eb3b995..0db6cb1 100644
--- a/testdata/valid/05-bigdiff.diff
+++ b/testdata/valid/05-bigdiff.diff
@@ -1,28 +1,43 @@
 Filters:
 --- Current
 +++ TO BE APPLIED
-@@ -1,82 +1,70 @@
+@@ -1,84 +1,95 @@
  * Criteria:
 -    to: someone-else@gmail.com
-+    query: list:foobaz.mail.com -"action needed"
-+  Actions:
-+    delete
-+
-+* Criteria:
-+    query: list:{list3 list1 list4 list6} -to:none@gmail.com
++    from: spammer2
    Actions:
-     archive
+-    archive
 -    mark as important
 -    never mark as spam
 -    mark as read
 -    star
 -    categorize as: social
 -    forward to: forward-address@gmail.com
-+    categorize as: personal
-+    apply label: maillist
++    delete
  
  * Criteria:
 -    query: is:muted
++    query: "buy this thing"
+   Actions:
+-    archive
+-    mark as important
+-    never mark as spam
+-    mark as read
+-    star
+-    categorize as: social
+-    forward to: forward-address@gmail.com
++    delete
+ 
+ * Criteria:
+     query: 
+-      cc:peeker@yahoo.com
+-      -subject:"a subject"
++      list:foobaz.mail.com
++      -"action needed"
++  Actions:
++    delete
++
++* Criteria:
 +    to: alias@gmail.com
 +  Actions:
 +    categorize as: promotions
@@ -50,20 +65,66 @@ Filters:
  
  * Criteria:
 -    from: someone@gmail.com
-+    query: list:{list3 list1 list4 list6} -to:none@gmail.com
++    to: pippo+spammy@gmail.com
++  Actions:
++    delete
++
++* Criteria:
++    query: bcc:aaaa@gmail.com
++  Actions:
++    categorize as: updates
++
++* Criteria:
++    query: 
++      list:{
++        list3
++        list1
++        list4
++        list6
++      }
++      -to:none@gmail.com
++  Actions:
++    apply label: differentlabel
++
++* Criteria:
++    from: spammer1
++    subject: "spam mail"
++    query: 
++      cc:foo@baz.com
++      bcc:bar@baz.com
++  Actions:
++    delete
++
++* Criteria:
++    query: 
++      list:{
++        list3
++        list1
++        list4
++        list6
++      }
++      -to:none@gmail.com
    Actions:
--    archive
+     archive
 -    mark as important
 -    never mark as spam
 -    mark as read
 -    star
 -    categorize as: social
 -    forward to: forward-address@gmail.com
-+    apply label: thirdlabel
++    categorize as: personal
++    apply label: maillist
  
  * Criteria:
 -    query: bcc:bccer@gmail.com
-+    to: pippo+spammy@gmail.com
++    query: 
++      list:{
++        list3
++        list1
++        list4
++        list6
++      }
++      -to:none@gmail.com
    Actions:
 -    archive
 -    mark as important
@@ -72,12 +133,11 @@ Filters:
 -    star
 -    categorize as: social
 -    forward to: forward-address@gmail.com
-+    delete
++    apply label: thirdlabel
  
- * Criteria:
+-* Criteria:
 -    query: replyto:replyer@gmail.com
-+    query: bcc:aaaa@gmail.com
-   Actions:
+-  Actions:
 -    archive
 -    mark as important
 -    never mark as spam
@@ -85,12 +145,10 @@ Filters:
 -    star
 -    categorize as: social
 -    forward to: forward-address@gmail.com
-+    categorize as: updates
- 
- * Criteria:
+-
+-* Criteria:
 -    query: "something in the body"
-+    from: spammer2
-   Actions:
+-  Actions:
 -    archive
 -    mark as important
 -    never mark as spam
@@ -98,35 +156,12 @@ Filters:
 -    star
 -    categorize as: social
 -    forward to: forward-address@gmail.com
-+    delete
- 
- * Criteria:
+-
+-* Criteria:
 -    query: list:maillist@google.com
-+    query: "buy this thing"
-   Actions:
+-  Actions:
 -    never mark as important
-+    delete
- 
- * Criteria:
--    query: cc:peeker@yahoo.com -subject:"a subject"
-+    query: list:{list3 list1 list4 list6} -to:none@gmail.com
-   Actions:
--    archive
--    mark as important
--    never mark as spam
--    mark as read
--    star
--    categorize as: social
--    forward to: forward-address@gmail.com
-+    apply label: differentlabel
- 
-+* Criteria:
-+    from: spammer1
-+    subject: "spam mail"
-+    query: cc:foo@baz.com bcc:bar@baz.com
-+  Actions:
-+    delete
-+
+-
 
 Labels:
 --- Current
diff --git a/testdata/valid/06-bigsplit.diff b/testdata/valid/06-bigsplit.diff
index fdc8fe9..7171e10 100644
--- a/testdata/valid/06-bigsplit.diff
+++ b/testdata/valid/06-bigsplit.diff
@@ -1,46 +1,117 @@
 Filters:
 --- Current
 +++ TO BE APPLIED
-@@ -1,70 +1,15 @@
+@@ -1,95 +1,72 @@
  * Criteria:
--    query: bcc:aaaa@gmail.com
-+    query: list:{list40 list41 list42 list43 list44 list45 list46 list47 list48 list49 list50}
-   Actions:
--    categorize as: updates
-+    archive
- 
- * Criteria:
--    query: list:{list3 list1 list4 list6} -to:none@gmail.com
-+    query: list:{list20 list21 list22 list23 list24 list25 list26 list27 list28 list29 list30 list31 list32 list33 list34 list35 list36 list37 list38 list39}
+     query: 
+       list:{
+-        list3
+-        list1
+-        list4
+-        list6
++        list40
++        list41
++        list42
++        list43
++        list44
++        list45
++        list46
++        list47
++        list48
++        list49
++        list50
+       }
+-      -to:none@gmail.com
    Actions:
      archive
 -    categorize as: personal
 -    apply label: maillist
  
  * Criteria:
--    from: spammer2
+     query: 
+       list:{
+-        list3
+-        list1
+-        list4
+-        list6
++        list20
++        list21
++        list22
++        list23
++        list24
++        list25
++        list26
++        list27
++        list28
++        list29
++        list30
++        list31
++        list32
++        list33
++        list34
++        list35
++        list36
++        list37
++        list38
++        list39
+       }
+-      -to:none@gmail.com
+   Actions:
+-    apply label: differentlabel
++    archive
+ 
+ * Criteria:
+     query: 
+       list:{
++        list0
++        list1
++        list2
+         list3
+-        list1
+         list4
++        list5
+         list6
++        list7
++        list8
++        list9
++        list10
++        list11
++        list12
++        list13
++        list14
++        list15
++        list16
++        list17
++        list18
++        list19
+       }
+-      -to:none@gmail.com
 -  Actions:
--    delete
+-    apply label: thirdlabel
 -
 -* Criteria:
--    query: "buy this thing"
+-    from: spammer2
 -  Actions:
 -    delete
 -
 -* Criteria:
--    query: list:{list3 list1 list4 list6} -to:none@gmail.com
+-    query: "buy this thing"
 -  Actions:
--    apply label: differentlabel
+-    delete
 -
 -* Criteria:
 -    from: spammer1
 -    subject: "spam mail"
--    query: cc:foo@baz.com bcc:bar@baz.com
+-    query: 
+-      cc:foo@baz.com
+-      bcc:bar@baz.com
 -  Actions:
 -    delete
 -
 -* Criteria:
--    query: list:foobaz.mail.com -"action needed"
+-    query: 
+-      list:foobaz.mail.com
+-      -"action needed"
 -  Actions:
 -    delete
 -
@@ -60,19 +131,18 @@ Filters:
 -    from: notfriend@gmail.com
 -    subject: "hey there"
 -    query: -to:none@gmail.com
-+    query: list:{list0 list1 list2 list3 list4 list5 list6 list7 list8 list9 list10 list11 list12 list13 list14 list15 list16 list17 list18 list19}
    Actions:
      archive
 -    star
 -    categorize as: forums
  
 -* Criteria:
--    query: list:{list3 list1 list4 list6} -to:none@gmail.com
+-    to: pippo+spammy@gmail.com
 -  Actions:
--    apply label: thirdlabel
+-    delete
 -
 -* Criteria:
--    to: pippo+spammy@gmail.com
+-    query: bcc:aaaa@gmail.com
 -  Actions:
--    delete
+-    categorize as: updates
 -
diff --git a/testdata/valid/07-deleteall.diff b/testdata/valid/07-deleteall.diff
index 8a8b47b..ee0a4f4 100644
--- a/testdata/valid/07-deleteall.diff
+++ b/testdata/valid/07-deleteall.diff
@@ -1,19 +1,76 @@
 Filters:
 --- Current
 +++ TO BE APPLIED
-@@ -1,15 +1 @@
+@@ -1,72 +1 @@
 -* Criteria:
--    query: list:{list40 list41 list42 list43 list44 list45 list46 list47 list48 list49 list50}
+-    query: 
+-      list:{
+-        list40
+-        list41
+-        list42
+-        list43
+-        list44
+-        list45
+-        list46
+-        list47
+-        list48
+-        list49
+-        list50
+-      }
 -  Actions:
 -    archive
  
 -* Criteria:
--    query: list:{list20 list21 list22 list23 list24 list25 list26 list27 list28 list29 list30 list31 list32 list33 list34 list35 list36 list37 list38 list39}
+-    query: 
+-      list:{
+-        list20
+-        list21
+-        list22
+-        list23
+-        list24
+-        list25
+-        list26
+-        list27
+-        list28
+-        list29
+-        list30
+-        list31
+-        list32
+-        list33
+-        list34
+-        list35
+-        list36
+-        list37
+-        list38
+-        list39
+-      }
 -  Actions:
 -    archive
 -
 -* Criteria:
--    query: list:{list0 list1 list2 list3 list4 list5 list6 list7 list8 list9 list10 list11 list12 list13 list14 list15 list16 list17 list18 list19}
+-    query: 
+-      list:{
+-        list0
+-        list1
+-        list2
+-        list3
+-        list4
+-        list5
+-        list6
+-        list7
+-        list8
+-        list9
+-        list10
+-        list11
+-        list12
+-        list13
+-        list14
+-        list15
+-        list16
+-        list17
+-        list18
+-        list19
+-      }
 -  Actions:
 -    archive
 -