Q/codewhisperer does not declare its position encoding

[justinmk3]

Problem

It seems like codewhisperer server expects utf-8 encoding,

language-servers/server/aws-lsp-codewhisperer/src/language-server/chat/contexts/utils.test.ts

Line 13 in 1c38b0c

const data = await fs.readFile(file, { encoding: 'utf8' });

but the default LSP encoding is utf-16.

Expected behavior

• Document the expected position encoding in the readme
• Declare the expected position encoding from the server settings/capabilities, so that clients don't have to guess it.
• (Optional) Declare utf-8 as the encoding expected by all the lsp servers in this repo? Mention this in the top-level docs, wherever those are...

[viktorsaws]

The codepath you're referring to is a file content create for unit test, it doesn't indicate which encoding is actually used by server.

Do you observe specific issues that you think are caused by incorrect handling of encoding on the server?

[justinmk3]

it doesn't indicate which encoding is actually used by server.

What is the encoding used by the server? My request is (at least):

• document that
• return it in the server config/init result (standard LSP method)

p.s. why is the test using a different encoding?

Do you observe specific issues that you think are caused by incorrect handling of encoding on the server?

Clients must know the correct encoding to use, in order to compute offsets ("position encoding").

[viktorsaws]

I'll look into documenting and encoding declaration by server.

p.s. why is the test using a different encoding?

Looks like that test is irrelevant, as it some TextDocument with random content, which just happen to include utf-8. It is as well could have been random text, it's doesn't indicate anything. Also looks like test was contributed by developer of Q Chat over LSP

[viktorsaws]

Reading LSP specs here, it looks like server defaults to UTF-16 encoding if positionEncodings is not returned, so clients should assume it is UTF16:

If the string value utf-16 is missing from the client’s capability general.positionEncodings servers can safely assume that the client supports UTF-16. If the server omits the position encoding in its initialize result the encoding defaults to the string value utf-16. Implementation considerations: since the conversion from one encoding into another requires the content of the file / line the conversion is best done where the file is read which is usually on the server side.

I don't think we switch to UTF8 for LSP exchanges anywhere, and I don't quite understand why documenting this is necessary, given this defaults per LSP specs.

Did you observe any issues with server computing offsets incorrectly?

[justinmk3]

Did you observe any issues with server computing offsets incorrectly?

Sending positions in a Q chat request often get results about a different part of the file. I'm not certain that the root cause of that issue is a position encoding problem (sounds like not), but having a reference (and a server result) would have saved time.

I don't quite understand why documenting this is necessary, given this defaults per LSP specs.

It's very inexpensive (one line in the existing doc) and helps consumers by removing ambiguity. Even if it's not technically necessary.

[viktorsaws]

Gotcha, thanks.

Sending positions in a Q chat request often get results about a different part of the file.

This is interesting, we'd need to investigate if there's an issue in Q Chat.