[PATCH] D89529: [clangd][remote] Add Windows paths support

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev requested changes to this revision.
kbobyrev added a comment.
This revision now requires changes to proceed.

Hi, thanks for flagging the issue! Could you please give more information where exactly the tests fail (e.g. logs with assertions)? This way I could understand where exactly the failure happens.

I can understand what you're trying to do but the patch introduces the behaviour which is unintended as of right now. The convention is to use POSIX paths everywhere in the Marshalling code since the end result will be a URI anyway. The patch changes it to Windows slashes which is orthogonal to this direction.

You're right about `llvm::Expected<clangd::FuzzyFindRequest> Marshaller::fromProtobuf` needing to convert to the native path though since there aren't any URIs in the end. Also, I think we should convert roots to the POSIX instead of asserting it.

I'm really curious what failures beyond `FuzzyFindRequest` are on Windows and I'd be happy to fix those but unfortunately I don't have a Windows machine to run the tests myself :(


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89529/new/

https://reviews.llvm.org/D89529