Correctly map between UTF-8 and UTF-16 positions

[aochagavia]

Fixes #202

[aochagavia]

Current concerns:

• ColIndex is always used together with LineIndex. Though I had to write quite some boilerplate to keep them separate, in the end I would rather make ColIndex a field of LineIndex. Any objections to this?
• It would be great to have integration tests for this, but I have no idea where they should go. Any suggestions are welcome.

[aochagavia]

Note: the LSP supports three line ending styles: \r, \n and \r\n. The current implementation supports the last two, but not \r. This should be fixed at some point.

[kjeremy]

• ColIndex is always used together with LineIndex. Though I had to write quite some boilerplate to keep them separate, in the end I would rather make ColIndex a field of LineIndex. Any objections to this?

Yeah that's a lot of duplicated boilerplate. I would rather have one "file_line_index" type method instead of both file_line_index and file_utf16_line_index.

[matklad]

Yeah, let's make utf16_lines: FxHashMap<u32, Vec<Utf16Char>>, a field of LineIndex!

[matklad]

I think this loop could work better as

for line_idx, line in text.lines().enumerate() {
}

That way, we don't need to worry about lines interfering.

[matklad]

Hm, this LineCol -> LineCol API seems a bit error prone, because it uses the same type for different units of measure. This can lead to errors: just yesterday my bank showed me the amount of money on my account in rubles, while using euro as a currency sign :D

I think a lower-level API might be safer:

pub fn col_as_utf16(&self, line_col: LineCol) -> usize {...}

Note that, by definition, TextUnit is always a utf_8 length, so using it for utf-16 is not correct.

[matklad]

There's assert_eq! macro

[aochagavia]

bors r+

[matklad]

bors r-

[bors]

Canceled

[matklad]

I think it's important to mark somehow that col is utf16, it's not obvouls from looking at the type definition. I think a doc comment would be fine, but probably just naming field col_utf16 works best?

[matklad]

Otherwise, LGTM! 👍

[aochagavia]

@matklad In the values of the utf16_lines hashmap we could use SmallVec instead of Vec to store instances of Utf16Char. We don't expect too much UTF16 chars per line, so we might as well store them inline. Do you think it is worth it or should we keep the Vec?

[matklad]

@aochagavia I was thinking about SmallVec, but I think that's a premature optimization: the number of lines with UTF-16 is small, so it should not matter if storage for a particular line is big.

[aochagavia]

Just pushed a commit to update col to col_utf16

[aochagavia]

bors r+

[bors]

Canceled

[aochagavia]

bors r+

[bors]

Build succeeded

• continuous-integration/travis-ci/push