From dfff4ebf806063a87d5bd961227adf6237dcd762 Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Fri, 8 Dec 2023 15:03:40 -0700 Subject: [PATCH 1/7] drop trailing newline --- contentprovider.go | 7 ++++++- contentprovider_test.go | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/contentprovider.go b/contentprovider.go index 802f925d7..54cf2c9bb 100644 --- a/contentprovider.go +++ b/contentprovider.go @@ -454,6 +454,12 @@ func (nls newlines) getLines(data []byte, low, high int) []byte { lowStart, _ := nls.lineBounds(low) _, highEnd := nls.lineBounds(high - 1) + // Drop any trailing newline + if highEnd == uint32(len(data)) && len(data) > 0 && data[len(data)-1] == '\n' { + highEnd = highEnd - 1 + lowStart = min(lowStart, highEnd) + } + return data[lowStart:highEnd] } @@ -681,7 +687,6 @@ func sectionSlice(data []byte, sec DocumentSection) []byte { return data[sec.Start:sec.End] } - // scoreSymbolKind boosts a match based on the combination of language, symbol // and kind. The language string comes from go-enry, the symbol and kind from // ctags. diff --git a/contentprovider_test.go b/contentprovider_test.go index 59294495a..deb31b090 100644 --- a/contentprovider_test.go +++ b/contentprovider_test.go @@ -32,7 +32,8 @@ func TestGetLines(t *testing.T) { for _, content := range contents { t.Run("", func(t *testing.T) { newLines := getNewlines(content) - lines := bytes.Split(content, []byte{'\n'}) // TODO does split group consecutive sep? + // Trim the last newline before splitting because a trailing newline does not constitute a new line + lines := bytes.Split(bytes.TrimSuffix(content, []byte{'\n'}), []byte{'\n'}) wantGetLines := func(low, high int) []byte { low-- high-- From 76aafd643c2628a02924a36522226587a3881a63 Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Fri, 8 Dec 2023 15:19:30 -0700 Subject: [PATCH 2/7] add explanatory comment --- contentprovider.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/contentprovider.go b/contentprovider.go index 54cf2c9bb..85b3710ad 100644 --- a/contentprovider.go +++ b/contentprovider.go @@ -454,7 +454,13 @@ func (nls newlines) getLines(data []byte, low, high int) []byte { lowStart, _ := nls.lineBounds(low) _, highEnd := nls.lineBounds(high - 1) - // Drop any trailing newline + // Drop any trailing newline. Editors do not treat a trailing newline as + // the start of a new line, so we should not either. lineBounds clamps to + // len(data) when an out-of-bounds line is requested. + // + // As an example, if we request lines 1-5 from a file with contents + // `one\ntwo\nthree\n`, we should return `one\ntwo\nthree` because those are + // the three "lines" in the file, separated by newlines. if highEnd == uint32(len(data)) && len(data) > 0 && data[len(data)-1] == '\n' { highEnd = highEnd - 1 lowStart = min(lowStart, highEnd) From d12ed3366e861bec6567e0d8cb8459e7f442581f Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Fri, 8 Dec 2023 15:20:10 -0700 Subject: [PATCH 3/7] update go version for fuzz testing --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8f93baf9a..019265cb6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,6 +28,7 @@ jobs: packages: 'github.com/sourcegraph/zoekt' # This is the package where the Protobuf round trip tests are defined fuzz-time: 30s fuzz-minimize-time: 1m + go-version: '1.21' shellcheck: name: shellcheck From 6ac8e22dee22f4bb6f4e3e35bc3a245cbf5985cc Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Fri, 8 Dec 2023 15:23:14 -0700 Subject: [PATCH 4/7] use pinned version --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 019265cb6..026dfca03 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,7 @@ jobs: name: fuzz test runs-on: ubuntu-latest steps: - - uses: jidicula/go-fuzz-action@4f24eed45b25214f31a9fe035ca68ea2c88c6a13 # v1.2.0 + - uses: jidicula/go-fuzz-action@0206b61afc603b665297621fa5e691b1447a5e57 # to make go version configurable with: packages: 'github.com/sourcegraph/zoekt' # This is the package where the Protobuf round trip tests are defined fuzz-time: 30s From 5c2e4fc40fa2e147f3fe97f35558f4044cebacaa Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Fri, 8 Dec 2023 15:25:16 -0700 Subject: [PATCH 5/7] add explanation --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 026dfca03..75038b07d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,10 @@ jobs: name: fuzz test runs-on: ubuntu-latest steps: - - uses: jidicula/go-fuzz-action@0206b61afc603b665297621fa5e691b1447a5e57 # to make go version configurable + # Pinned a commit to make go version configurable. + # This should be safe to upgrade once this commit is in a released version: + # https://github.com/jidicula/go-fuzz-action/commit/23cc553941669144159507e2cccdbb4afc5b3076 + - uses: jidicula/go-fuzz-action@0206b61afc603b665297621fa5e691b1447a5e57 with: packages: 'github.com/sourcegraph/zoekt' # This is the package where the Protobuf round trip tests are defined fuzz-time: 30s From 083cb677f0cfaaf98024d42d2c953e56f982a65e Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Fri, 8 Dec 2023 15:38:52 -0700 Subject: [PATCH 6/7] update e2e_test --- web/e2e_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/web/e2e_test.go b/web/e2e_test.go index be00b61d6..01ca5999c 100644 --- a/web/e2e_test.go +++ b/web/e2e_test.go @@ -560,7 +560,10 @@ func TestContextLines(t *testing.T) { // Returns 3 instead of 4 new line characters since we swallow // the last new line in Before, Fragments and After. Before: "\n\n\n", - After: "\n\n\n", + // Returns 2 instead of 3 new line characters since a + // trailing newline at the end of the file does not + // constitue a new line. + After: "\n\n", }, }, }, From cc6096f7877ede336d458c90d1c56f1a65f84c2c Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Mon, 11 Dec 2023 08:33:42 -0700 Subject: [PATCH 7/7] Update contentprovider.go Co-authored-by: Keegan Carruthers-Smith --- contentprovider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contentprovider.go b/contentprovider.go index 85b3710ad..b1fe0ab81 100644 --- a/contentprovider.go +++ b/contentprovider.go @@ -461,7 +461,7 @@ func (nls newlines) getLines(data []byte, low, high int) []byte { // As an example, if we request lines 1-5 from a file with contents // `one\ntwo\nthree\n`, we should return `one\ntwo\nthree` because those are // the three "lines" in the file, separated by newlines. - if highEnd == uint32(len(data)) && len(data) > 0 && data[len(data)-1] == '\n' { + if highEnd == uint32(len(data)) && bytes.HasSuffix(data, []byte{'\n'}) { highEnd = highEnd - 1 lowStart = min(lowStart, highEnd) }