search

haskell / haskell-language-server

Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.
Apache License 2.0
2.61k stars 351 forks source link

Test Rope.getLine #4303

Open jhrcek opened 2 weeks ago

jhrcek commented 2 weeks ago

Just testing out newly introduced utility from text-rope

michaelpj commented 2 weeks ago

We also do this a bunch in lsp, I think.

jhrcek commented 2 weeks ago

We also do this a bunch in lsp, I think.

@michaelpj Hmm, in lsp there's some extractLine in LSP which returns a Rope and does further manipulation on that Rope, which means that the new function I introduced is probably useless. Or do you think that the function I introduced should return a Rope?

michaelpj commented 2 weeks ago

Hmm, that's a little annoying. We return a Rope so we can use the mixed encoding indexing which text-rope provides, but conceptually it's just a one-line Text. I don't know if there are comparable mixed-indexing functions for plain Text? If so we can switch to use your getLine.

michaelpj commented 2 weeks ago

Note that it's quite likely that functions in HLS also need this facility. Remember that much of HLS incorrectly assumes that it can use LSP positions as GHC positions or as indexes into Text. So e.g. this line is wrong: it needs to drop c UTF-16 code units, whereas Text.drop drops code units.

So HLS might also prefer to get a Rope. Or it should use rangeLinesFromVFS from lsp, which does this correctly.

jhrcek commented 2 weeks ago

@michaelpj I opened a followup PR in text-rope that makes getLine "stay in the Rope world". Would it make more sense to you like that? https://github.com/Bodigrim/text-rope/pull/7