vamolessa / pepper

simple and opinionated modal code editor for your terminal
https://vamolessa.github.io/pepper/
372 stars 17 forks source link

Pepper doesn't send trailing newlines to the LSP server #70

Closed praschke closed 1 year ago

praschke commented 1 year ago

This assumption means pepper fails to open files ending with a different character, and also contaminates lsp-format by neglecting to send final newlines in textDocument/didChange events, which in my case causes zls to add a second newline to the end of the file.

vamolessa commented 1 year ago

Would you mind explaining the issue a little bit further? I've just tested opening a file that does not have a trailing \n and it could open it no problem. Feel free to send a test file which reproduces the issue. Thanks!

praschke commented 1 year ago

i accidentally introduced null characters into the file i was testing with, which explains the failure to open. sorry for the confusion.

there is no distinction in the user interface between a file ending with a newline or not, which i suppose is a different issue. this is just relevant to the lsp then.

an example file:

~/w/z/deimos $ xxd file.zig 
00000000: 636f 6e73 7420 7374 6420 3d20 4069 6d70  const std = @imp
00000010: 6f72 7428 2273 7464 2229 3b0a 0a70 7562  ort("std");..pub
00000020: 2066 6e20 6275 696c 6428 623a 202a 7374   fn build(b: *st
00000030: 642e 6275 696c 642e 4275 696c 6465 7229  d.build.Builder)
00000040: 2076 6f69 6420 7b0a 0a7d 0a               void {..}.

the lsp log:

lsp server 'zls' started

lsp: send notification
method: 'initialized'
params:
{}

lsp: send notification
method: 'textDocument/didOpen'
params:
{"textDocument":{"uri":"file:////home/stel/w/z/deimos/file.zig","languageId":"zig","version":1,"text":"const std = @import(\"std\");\n\npub fn build(b: *std.build.Builder) void {\n\n}"}}

lsp: receive notification
method: 'textDocument/publishDiagnostics'
params:
{"uri":"file:////home/stel/w/z/deimos/file.zig","diagnostics":[{"range":{"start":{"line":2,"character":13},"end":{"line":2,"character":14}},"severity":1,"code":"ast_check","source":"zls","message":"unused function parameter","relatedInformation":null}]}

lsp: send request
method: 'textDocument/formatting'
params:
{"textDocument":{"uri":"file:////home/stel/w/z/deimos/file.zig"},"options":{"tabSize":4,"insertSpaces":true,"trimTrailingWhitespace":true,"trimFinalNewlines":true}}

lsp: receive response
id: 2
method: 'textDocument/formatting'
result:
[{"range":{"start":{"line":0,"character":0},"end":{"line":4,"character":1}},"newText":"const std = @import(\"std\");\n\npub fn build(b: *std.build.Builder) void {}\n"}]

lsp: send notification
method: 'textDocument/didChange'
params:
{"textDocument":{"uri":"file:////home/stel/w/z/deimos/file.zig","version":2},"contentChanges":[{"text":"const std = @import(\"std\");\n\npub fn build(b: *std.build.Builder) void {}\n"}]}

the file after saving:

~/w/z/deimos $ xxd file.zig 
00000000: 636f 6e73 7420 7374 6420 3d20 4069 6d70  const std = @imp
00000010: 6f72 7428 2273 7464 2229 3b0a 0a70 7562  ort("std");..pub
00000020: 2066 6e20 6275 696c 6428 623a 202a 7374   fn build(b: *st
00000030: 642e 6275 696c 642e 4275 696c 6465 7229  d.build.Builder)
00000040: 2076 6f69 6420 7b7d 0a0a                  void {}..

pepper doesn't send the final newline of the file to the lsp server zls in didOpen and didChange. zls tells pepper to put a newline at the end of the file (actually before the final newline). this causes an extra newline to appear.

the relevant lines seem to be let text = client.json.fmt_string(format_args!("{}", buffer.content()));, either the buffer content doesn't actually contain the final newline or it is being dropped somewhere in transit.

praschke commented 1 year ago

BufferContent is printed with newlines between each BufferLine, and not on the outside. this is just as needed to represent files without trailing newlines. however, there is a missing BufferLine at the end of files with trailing newlines.

pepper::buffer::BufferLine ("    }"),
pepper::buffer::BufferLine ("}"),
pepper::buffer::BufferLine ("")

this is what i see in gdb from a file with two trailing newlines. it's what i would expect to see from a buffer with one trailing newline.

pepper::buffer::BufferLine ("    }"),
pepper::buffer::BufferLine ("}")

this is what i see from a file with one trailing newline, and from a file with no trailing newline. i would expect to see this only from a buffer with no trailing newline.

this looks like the source of the problem. popping newlines off of lines then calling read_line forgets whether there is an empty line between the last newline and EOF/whether the last character before EOF is a newline.

vamolessa commented 1 year ago

Thanks for the thoroughly detailed explanation! I've just merged your PR. Hopefully that will fix your issues. It should be out with the 0.31.0 release.

Will close now. Please feel free to reopen should you find further issues.