Add support for JSON responses in the webserver

[kisabaka]

This PR allows using the webserver to serve JSON results so it can be used as an API backend in a small deployment. In addition to that, it also returns context lines to make it easy to build a simple client around the webserver.

Issue

google/zoekt#126

Rationale

The zoekt webserver makes it easy to launch and test a local instance. However, it cannot be used as an API backend for custom web clients since it only returns results in server rendered HTML pages. Additionally, introducing context lines makes it easy to write clients that behave differently from the default webserver.

Tests

Tested locally with the webserver, and passing ?format=json&ctx=2. All existing tests pass

[keegancsmith]

@kisabaka thanks will take a look soon. I just pushed a merge of gerrit/master to our master. Can you rebase your changes ontop of our current master so this PR only contains your changes?

[kisabaka]

@kisabaka thanks will take a look soon. I just pushed a merge of gerrit/master to our master. Can you rebase your changes ontop of our current master so this PR only contains your changes?

Done!

@keegancsmith let me know if I should break this up in 2 PRs: Adding context lines and adding JSON API support.

[keegancsmith]

Thank you for contributing. Can you include some tests that the feature works / we won't regress?

[kisabaka]

Can you include some tests that the feature works / we won't regress?
I've added some web e2e tests. Please let me know if you want more tests and where will be the proper place to put them.

@keegancsmith PTAL. Thanks for the review

[keegancsmith]

a suggested simplification in the code inline, but close to merging :) thank you

[kisabaka]

@keegancsmith PTAL. Changed the new fields to be string instead of []bytes

[keegancsmith]

Hey, sorry some more inline feedback. I'll review quickly! In fact stuff is quite minor here, so if you are happy with it I'm happy to follow up by pushing to this branch if I have any more minor feedback so we can get this landed :)

[keegancsmith]

I wasn't convinced this didn't have a panic in it, so added a simple test. Now, I did adjust the getLines implementation so it would be 1-based, but we should make the helper robust so this would apply even though we have some invariants on what is possible for num.

diff --git a/contentprovider.go b/contentprovider.go
index 2442e3f..0b28fe6 100644
--- a/contentprovider.go
+++ b/contentprovider.go
@@ -213,15 +213,10 @@ func (p *contentProvider) fillContentMatches(ms []*candidateMatch, numContextLin
 		finalMatch.Line = data[lineStart:lineEnd]
 
 		if numContextLines > 0 {
-			// Why "-2"?
-			// "num" is 1-based, and "p.newlines()" returns an array of "\n" positions in the doc.
-			// If we do "num-1" then high will be the index of the "\n" where the matched line ends
-			// which is not what we want.
-			// We want the index of the "\n" where the matched line begins so we "num-2" to get that.
 			finalMatch.Before = getLines(
-				data, p.newlines(), num-numContextLines-2, num-2)
+				data, p.newlines(), num-numContextLines, num)
 			finalMatch.After = getLines(
-				data, p.newlines(), num-1, num+numContextLines-1)
+				data, p.newlines(), num+1, num+1+numContextLines)
 		}
 
 		for _, m := range lineCands {
@@ -246,7 +241,14 @@ func (p *contentProvider) fillContentMatches(ms []*candidateMatch, numContextLin
 	return result
 }
 
+// getLines returns a slice of data containing the lines [low, high). low is
+// 1-based and inclusive. high is exclusive.
 func getLines(data []byte, newLines []uint32, low, high int) []byte {
+	// newlines[0] is the start of the 2nd line. So adjust low and high to be
+	// based on newLines.
+	low -= 2
+	high -= 2
+
 	if high < 0 || low >= len(newLines) || len(newLines) == 0 {
 		return nil
 	}
diff --git a/contentprovider_test.go b/contentprovider_test.go
new file mode 100644
index 0000000..cfe4928
--- /dev/null
+++ b/contentprovider_test.go
@@ -0,0 +1,51 @@
+package zoekt
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestGetLines(t *testing.T) {
+	data := []byte(`one
+two
+three
+four`)
+
+	var newLines []uint32
+	for i, c := range data {
+		if c == '\n' {
+			newLines = append(newLines, uint32(i))
+		}
+	}
+
+	lines := bytes.Split(data, []byte{'\n'}) // TODO does split group consecutive sep?
+	wantGetLines := func(low, high int) []byte {
+		low--
+		high--
+		if low < 0 {
+			low = 0
+		}
+		if low >= len(lines) {
+			return nil
+		}
+		if high <= 0 {
+			return nil
+		}
+		if high > len(lines) {
+			high = len(lines)
+		}
+		return bytes.Join(lines[low:high], []byte{'\n'})
+	}
+
+	for low := -1; low <= len(lines)+2; low++ {
+		for high := low; high <= len(lines)+2; high++ {
+			want := wantGetLines(low, high)
+			got := getLines(data, newLines, low, high)
+			if d := cmp.Diff(string(want), string(got)); d != "" {
+				t.Fatal(d)
+			}
+		}
+	}
+}

[kisabaka]

@keegancsmith thanks for the contentprovider_test.go. I've added it

[keegancsmith]

one very minor bug inline, but let me land this now and I'll follow up with a commit to fix it rather than waiting for a full review cycle :)