Skip to content

Commit

Permalink
fix: bug with newlines being added to reinserted text in Neovim
Browse files Browse the repository at this point in the history
  • Loading branch information
a-h committed Mar 5, 2023
1 parent a76a114 commit b1973fd
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 37 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ dist/
cmd/templ/templ
cmd/templ/lspcmd/*log.txt
.DS_Store
coverage.out
75 changes: 40 additions & 35 deletions cmd/templ/lspcmd/proxy/documentcontents.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package proxy

import (
"errors"
"fmt"
"strings"
"sync"
Expand Down Expand Up @@ -70,10 +69,7 @@ func (dc *DocumentContents) Apply(uri string, changes []lsp.TextDocumentContentC
return
}
for _, change := range changes {
err = d.Overwrite(change.Range, change.Text)
if err != nil {
return
}
d.Overwrite(change.Range, change.Text)
}
return
}
Expand All @@ -99,28 +95,10 @@ func (d *Document) isRangeOfDocument(r lsp.Range) bool {
return startLine == 0 && startChar == 0 && endLine == len(d.Lines)-1 && endChar == len(d.Lines[len(d.Lines)-1])-1
}

func (d *Document) isOutsideDocumentRange(r lsp.Range) bool {
startLine, startChar := int(r.Start.Line), int(r.Start.Character)
endLine, endChar := int(r.End.Line), int(r.End.Character)
if startLine < 0 || startChar < 0 || endChar < 0 {
return true
}
startLineMaxCharIndex := len(d.Lines[startLine])
if r.Start.Character > uint32(startLineMaxCharIndex) {
return true
}
if endLine > len(d.Lines)-1 {
return true
}
endLineMaxCharIndex := len(d.Lines[endLine])
if r.End.Character > uint32(endLineMaxCharIndex) {
return true
}
return false
}

// As per https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#range
// If you want to specify a range that contains a line including the line ending character(s) then use an end position denoting the start of the next line.
func (d *Document) isWholeLineRange(r lsp.Range) bool {
return r.Start.Character == 0 && r.End.Character == 0
return r.Start.Character == 0 && r.End.Character == 0 && r.End.Line != r.Start.Line
}

func (d *Document) remove(i, j int) {
Expand All @@ -131,22 +109,41 @@ func (d *Document) insert(i int, withLines []string) {
d.Lines = append(d.Lines[:i], append(withLines, d.Lines[i:]...)...)
}

var ErrOutsideDocumentRange = errors.New("range is outside of document bounds")
func (d *Document) normaliseRange(r *lsp.Range) {
if r.Start.Line > uint32(len(d.Lines))-1 {
r.Start.Line = uint32(len(d.Lines)) - 1
}
if r.End.Line > uint32(len(d.Lines))-1 {
r.End.Line = uint32(len(d.Lines)) - 1
}
startLine := d.Lines[r.Start.Line]
startLineMaxCharIndex := len(startLine)
if r.Start.Character > uint32(startLineMaxCharIndex) {
r.Start.Character = uint32(startLineMaxCharIndex)
}
endLine := d.Lines[r.End.Line]
endLineMaxCharIndex := len(endLine)
if r.End.Character > uint32(endLineMaxCharIndex) {
r.End.Character = uint32(endLineMaxCharIndex)
}
}

func (d *Document) Overwrite(r *lsp.Range, with string) error {
if r == nil || d.isEmptyRange(*r) || d.isRangeOfDocument(*r) {
func (d *Document) Overwrite(r *lsp.Range, with string) {
if r == nil || d.isEmptyRange(*r) || len(d.Lines) == 0 {
d.Lines = strings.Split(with, "\n")
return nil
return
}
if d.isOutsideDocumentRange(*r) {
return ErrOutsideDocumentRange
d.normaliseRange(r)
if d.isRangeOfDocument(*r) {
d.Lines = strings.Split(with, "\n")
return
}
if d.isWholeLineRange(*r) {
d.remove(int(r.Start.Line), int(r.End.Line))
if with != "" {
d.insert(int(r.Start.Line), strings.Split(with, "\n"))
}
return nil
return
}
withLines := strings.Split(with, "\n")
if r.Start.Character > 0 {
Expand All @@ -157,9 +154,17 @@ func (d *Document) Overwrite(r *lsp.Range, with string) error {
suffix := d.Lines[r.End.Line][r.End.Character:]
withLines[len(withLines)-1] = withLines[len(withLines)-1] + suffix
}
d.remove(int(r.Start.Line), int(r.End.Line+1))
if r.End.Line > r.Start.Line && r.End.Character == 0 {
// Neovim unexpectedly adds a newline when re-inserting content (dd, followed by u for undo).
if last := withLines[len(withLines)-1]; last == "" {
withLines = withLines[0 : len(withLines)-1]
}
d.remove(int(r.Start.Line), int(r.End.Line))
} else {
d.remove(int(r.Start.Line), int(r.End.Line+1))
}
d.insert(int(r.Start.Line), withLines)
return nil
return
}

func (d *Document) String() string {
Expand Down
149 changes: 147 additions & 2 deletions cmd/templ/lspcmd/proxy/documentcontents_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package proxy
import (
"testing"

"github.com/google/go-cmp/cmp"
lsp "github.com/a-h/protocol"
"github.com/google/go-cmp/cmp"
)

func TestDocument(t *testing.T) {
Expand Down Expand Up @@ -130,7 +130,76 @@ func TestDocument(t *testing.T) {
expected: "Line one test\nNew Line 2\nNew line three",
},
{
name: "Add new line with indent",
name: "Add new line to end of single line",
start: `a`,
operations: []func(d *Document){
func(d *Document) {
d.Overwrite(&lsp.Range{
Start: lsp.Position{
Line: 0,
Character: 1,
},
End: lsp.Position{
Line: 1,
Character: 0,
},
}, "\nb")
},
},
expected: "a\nb",
},
{
name: "Exceeding the col and line count rounds down to the end of the file",
start: `a`,
operations: []func(d *Document){
func(d *Document) {
d.Overwrite(&lsp.Range{
Start: lsp.Position{
Line: 200,
Character: 600,
},
End: lsp.Position{
Line: 300,
Character: 1200,
},
}, "\nb")
},
},
expected: "a\nb",
},
{
name: "Can remove a line and add it back from the end of the previous line",
start: "a\nb\nc",
operations: []func(d *Document){
func(d *Document) {
// Delete.
d.Overwrite(&lsp.Range{
Start: lsp.Position{
Line: 1,
Character: 0,
},
End: lsp.Position{
Line: 2,
Character: 0,
},
}, "")
// Put it back.
d.Overwrite(&lsp.Range{
Start: lsp.Position{
Line: 0,
Character: 1,
},
End: lsp.Position{
Line: 1,
Character: 0,
},
}, "\nb")
},
},
expected: "a\nb\nc",
},
{
name: "Add new line with indent to the end of the line",
// Based on log entry.
// {"level":"info","ts":"2022-06-04T20:55:15+01:00","caller":"proxy/server.go:391","msg":"client -> server: DidChange","params":{"textDocument":{"uri":"file:///Users/adrian/github.com/a-h/templ/generator/test-call/template.templ","version":2},"contentChanges":[{"range":{"start":{"line":4,"character":21},"end":{"line":4,"character":21}},"text":"\n\t\t"}]}}
start: `package testcall
Expand Down Expand Up @@ -163,6 +232,82 @@ templ personTemplate(p person) {
</div>
}
`,
},
{
name: "Recreate error smaller",
// Based on log entry.
// {"level":"info","ts":"2022-06-04T20:55:15+01:00","caller":"proxy/server.go:391","msg":"client -> server: DidChange","params":{"textDocument":{"uri":"file:///Users/adrian/github.com/a-h/templ/generator/test-call/template.templ","version":2},"contentChanges":[{"range":{"start":{"line":4,"character":21},"end":{"line":4,"character":21}},"text":"\n\t\t"}]}}
start: "line1\n\t\tline2\nline3",
operations: []func(d *Document){
func(d *Document) {
// Remove \t\tline2
d.Overwrite(&lsp.Range{
Start: lsp.Position{
Line: 1,
Character: 0,
},
End: lsp.Position{
Line: 2,
Character: 0,
},
}, "")
// Put it back.
d.Overwrite(&lsp.Range{
Start: lsp.Position{
Line: 0,
Character: 5,
},
End: lsp.Position{
Line: 1,
Character: 0,
},
},
"\n\t\tline2\n")
},
},
expected: "line1\n\t\tline2\nline3",
},
{
name: "Recreate error",
// Based on log entry.
// {"level":"info","ts":"2022-06-04T20:55:15+01:00","caller":"proxy/server.go:391","msg":"client -> server: DidChange","params":{"textDocument":{"uri":"file:///Users/adrian/github.com/a-h/templ/generator/test-call/template.templ","version":2},"contentChanges":[{"range":{"start":{"line":4,"character":21},"end":{"line":4,"character":21}},"text":"\n\t\t"}]}}
start: ` <footer data-testid="footerTemplate">
<div>&copy; { fmt.Sprintf("%d", time.Now().Year()) }</div>
</footer>
}
`,
operations: []func(d *Document){
func(d *Document) {
// Remove <div>&copy; { fmt.Sprintf("%d", time.Now().Year()) }</div>
d.Overwrite(&lsp.Range{
Start: lsp.Position{
Line: 1,
Character: 0,
},
End: lsp.Position{
Line: 2,
Character: 0,
},
}, "")
// Put it back.
d.Overwrite(&lsp.Range{
Start: lsp.Position{
Line: 0,
Character: 38,
},
End: lsp.Position{
Line: 1,
Character: 0,
},
},
"\n\t\t<div>&copy; { fmt.Sprintf(\"%d\", time.Now().Year()) }</div>\n")
},
},
expected: ` <footer data-testid="footerTemplate">
<div>&copy; { fmt.Sprintf("%d", time.Now().Year()) }</div>
</footer>
}
`,
},
}
Expand Down

0 comments on commit b1973fd

Please sign in to comment.