Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ChunkMatches: drop trailing newline #709

Merged
merged 7 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,15 @@ jobs:
name: fuzz test
runs-on: ubuntu-latest
steps:
- uses: jidicula/go-fuzz-action@4f24eed45b25214f31a9fe035ca68ea2c88c6a13 # v1.2.0
# 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
fuzz-minimize-time: 1m
go-version: '1.21'
Comment on lines +26 to +34
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed to update this so the build doesn't fail when using new go 1.21 features.


shellcheck:
name: shellcheck
Expand Down
13 changes: 12 additions & 1 deletion contentprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,18 @@ func (nls newlines) getLines(data []byte, low, high int) []byte {
lowStart, _ := nls.lineBounds(low)
_, highEnd := nls.lineBounds(high - 1)

// 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)) && bytes.HasSuffix(data, []byte{'\n'}) {
highEnd = highEnd - 1
lowStart = min(lowStart, highEnd)
}

return data[lowStart:highEnd]
}

Expand Down Expand Up @@ -681,7 +693,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.
Expand Down
3 changes: 2 additions & 1 deletion contentprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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'})
Comment on lines +35 to +36
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g. one\ntwo\nthree\n is three lines, not four. Same as one\ntwo\nthree

wantGetLines := func(low, high int) []byte {
low--
high--
Expand Down
5 changes: 4 additions & 1 deletion web/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
},
Expand Down
Loading