[clangd-dev] DraftStore::updateDraft compares lsp's rangeLength in UTF16 code units against the computed rangeLength in bytes

[Sam McCall via clangd-dev]

On Mon, Oct 22, 2018 at 1:32 PM Daan De Meyer <daan.j.demeyer at gmail.com>
wrote:

>
>> My only concern is that we'll break clients that make the equal and
>> opposite mistake (sending byte lengths instead of UTF-16 lengths).
>> Maybe we should also downgrade this error to a log message, since we can
>> presumably trust the length from the string. What do you think?
>>
>
> I think the check is useful just to verify if the client and server are on
> the same page so I'm partial to option 2 as well. Downgrading to a log
> message avoids outright breaking existing clients so that seems like the
> way to go. As soon as Github is back up I'll make an issue asking to
> clarify the unit of rangeLength in the lsp repository as well.
>
Thanks!

Assuming we go with option 2 (including downgrading to an error message), I
> have two more questions:
>
> 1. Which header should I put the utf16Len function in (from
> SourceCode.cpp)? I tried searching for an existing header with UTF
> functions but I didn't immediately find one.
>
I think SourceCode.h is a reasonable fit within clangd. (There are some UTF
libraries in llvm/support, but it's not clear this belongs there and it'd
be a bunch of work).

Consider renaming it something like `lspLength()` or so and keeping the
bits about UTF-16 in the function comment.
That's kind of a low-level detail.


> 2. How do I go about downgrading to a log message? Do I just write to
> errs()?
>
We use the log() functions in clangd/Logger.h. (Abstracts the destination,
handles locking, does formatting).

This one is scary+rare enough it should probably be an error log.
The functions use formatv syntax, so something like:
  elog("bad TextDocumentContentChangeEvent: rangeLength={0} but computed
length {1} for {2}",
       *change.rangeLength, Len, change.text);

Cheers, Sam

>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/clangd-dev/attachments/20181022/c7bbe8fd/attachment.html>