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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd99b2a976a37: [clangd][remote] Add Windows paths support (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

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

ArcsinX added a comment. In D89529#2338794 , @kbobyrev wrote: > Thank you for the patch! Ready to land it now. Thank you for review! > Please update the patch message before commiting though: > >> Without this patch 7 marshalling tests fails on Windows.

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

kbobyrev accepted this revision. kbobyrev added a comment. This revision is now accepted and ready to land. Thank you for the patch! Ready to land it now. Please update the patch message before commiting though: > Without this patch 7 marshalling tests fails on Windows. > This patch contains the

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

ArcsinX updated this revision to Diff 299034. ArcsinX added a comment. Comments fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89529/new/ https://reviews.llvm.org/D89529 Files: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.c

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

kbobyrev requested changes to this revision. kbobyrev added a comment. This revision now requires changes to proceed. Also, please hit "Done" on the comments you resolved so that it's easier to track which ones are fixed, otherwise it's hard to follow :( The patch looks good now, almost ready to

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

ArcsinX marked 2 inline comments as done. ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:59 +RemoteIndexRoot, llvm::sys::path::Style::windows); +llvm::StringRef Path(*this->RemoteIndexRoot); +if (!Path.

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

ArcsinX updated this revision to Diff 299017. ArcsinX added a comment. auto => StringRef add comment for `llvm::sys::path::native()` call Result.str().str() => std::string(Result) add back removed test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8

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

kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:59 +RemoteIndexRoot, llvm::sys::path::Style::windows); +llvm::StringRef Path(*this->RemoteIndexRoot); +if (!Path.endswith(PosixSeparator)) ---

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

ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp:59 +RemoteIndexRoot, llvm::sys::path::Style::windows); +llvm::StringRef Path(*this->RemoteIndexRoot); +if (!Path.endswith(PosixSeparator))

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

ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp:242 // Add only valid headers. - Header.IncludeHeader = Strings.save( - URI::createFile("/usr/local/user/home/project/Header.h").toString()); kbobyr

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

kbobyrev requested changes to this revision. kbobyrev added a comment. This revision now requires changes to proceed. In D89529#2337197 , @ArcsinX wrote: > In D89529#2334517 , @kbobyrev wrote: > >> The solution woul

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

ArcsinX added a comment. In D89529#2334517 , @kbobyrev wrote: > The solution would be to just convert to the POSIX path instead of asserting > it. I have updated the patch according to this advice. It seems to me it looks more consistent now, thank you.

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

ArcsinX updated this revision to Diff 298869. ArcsinX added a comment. Convert `RemoteIndexRoot` and `LocalIndexRoot` to the POSIX style. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89529/new/ https://reviews.llvm.org/D89529 Files: clang-tools

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

ArcsinX added a comment. In D89529#2334517 , @kbobyrev wrote: > In D89529#2334475 , @ArcsinX wrote: > >> In D89529#2334407 , @kbobyrev wrote: >> >>> Hi, thanks for flagging t

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

kbobyrev added a comment. In D89529#2334475 , @ArcsinX wrote: > In D89529#2334407 , @kbobyrev wrote: > >> Hi, thanks for flagging the issue! Could you please give more information >> where exactly the tests fail (e

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

ArcsinX added a comment. Logs: D:\work\llvm-project\build>ninja check-clangd [6/7] Running the Clangd regression tests llvm-lit.py: D:\work\llvm-project\build\bin\..\..\llvm\utils\lit\lit\llvm\config.py:347: note: using clang: d:\work\llvm-project\build\bin\clang.exe -- Testing: 905 tes

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

ArcsinX added a comment. In D89529#2334407 , @kbobyrev wrote: > 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

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

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

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

ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman. Herald added a project: clang. ArcsinX requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Without this patch 7 marshalling tests fails on Windows. This patch conta