[PATCH] D44484: [clangd] Use Contents from inputs in codeComplete and signatureHelp

simark created this revision. Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov, klimek. ClangdServer::{codeComplete,signatureHelp} both use the Contents from the draft manager. Since we want to move the draft manager from ClangdServer to ClangdLSPServer, this patch cha

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark added a comment. In https://reviews.llvm.org/D44408#1037327, @ilya-biryukov wrote: > In https://reviews.llvm.org/D44408#1036941, @simark wrote: > > > I don't see how to avoid adding the Contents parameter to the codeComplete > > and signatureHelp methods, since they needed to fetch the do

[PATCH] D44484: [clangd] Use Contents from inputs in codeComplete and signatureHelp

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE327550: [clangd] Use Contents from inputs in codeComplete and signatureHelp (authored by simark, committed by ). Changed prior to commit: https://reviews.llvm.org/D44484?vs=138417&id=138424#toc Repos

[PATCH] D44462: [clangd] Don't use DraftMgr in implementation of forceReparse.

simark added inline comments. Comment at: clangd/TUScheduler.h:69 + /// FIXME: remove the callback from this function + void updateCompileCommand(PathRef File, tooling::CompileCommand NewCommand, +IntrusiveRefCntPtr FS, ilya-biryukov

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark updated this revision to Diff 138490. simark added a comment. Rebase on current master, addressing previous comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44408 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServer.cpp clangd/Cla

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark updated this revision to Diff 138569. simark marked 6 inline comments as done. simark added a comment. Changes based on review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44408 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/ClangdServ

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

simark updated this revision to Diff 138581. simark marked 2 inline comments as done. simark added a comment. Address latest review comments. Also, update the comments in DraftStore.{h,cpp} that were outdated. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44408 Files: clang

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

This revision was automatically updated to reflect the committed changes. Closed by commit rL327711: Move DraftMgr from ClangdServer to ClangdLSPServer (authored by simark, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44408 Files: cl

[PATCH] D44272: [clangd] Support incremental document syncing

simark marked 3 inline comments as done. simark added inline comments. Comment at: clangd/ClangdLSPServer.cpp:101 json::obj{ -{"textDocumentSync", 1}, +{"textDocumentSync", 2}, {"documentFormattingProvider", true}, il

[PATCH] D44272: [clangd] Support incremental document syncing

simark updated this revision to Diff 138741. simark marked an inline comment as done. simark added a comment. Address review comments, rebase on latest master Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44272 Files: clangd/ClangdLSPServer.cpp clangd/DraftStore.cpp clan

[PATCH] D44226: [clangd] Add -log-lsp-to-stderr option

simark updated this revision to Diff 138743. simark marked 3 inline comments as done. simark added a comment. Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44226 Files: clangd/ClangdLSPServer.cpp clangd/JSONRPCDispatcher.cpp clangd/JSONRPCDispatch

[PATCH] D44628: Backport changes from llvm/.clang_tidy to clang/.clang_tidy configs

simark requested changes to this revision. simark added inline comments. This revision now requires changes to proceed. Comment at: .clang-tidy:8 - key: readability-identifier-naming.FunctionCase -value: lowerCase +value: camelBack + -

[PATCH] D44272: [clangd] Support incremental document syncing

simark marked 4 inline comments as done. simark added inline comments. Comment at: clangd/ClangdLSPServer.cpp:164 + if (!Contents) { +log(llvm::toString(Contents.takeError())); +return; ilya-biryukov wrote: > We should signal an error to the client by ca

[PATCH] D44272: [clangd] Support incremental document syncing

simark marked 3 inline comments as done. simark added inline comments. Comment at: clangd/DraftStore.cpp:61 + const Position &Start = Change.range->start; + size_t StartIndex = positionToOffset(Contents, Start); + if (StartIndex > Contents.length()) { -

[PATCH] D44673: Make positionToOffset return llvm::Expected

simark created this revision. Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov, klimek. simark added a reviewer: ilya-biryukov. To implement incremental document syncing, we want to verify that the ranges provided by the front-end are valid. Currently, positionToOffset

[PATCH] D53220: Remove possibility to change compile database path at runtime

simark created this revision. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, ioeric, ilya-biryukov. This patch removes the possibility to change the compilation database path at runtime using the didChangeConfiguration request. Instead, it is suggested to use the setting on

[PATCH] D51725: Allow un-setting the compilation database path

simark added a comment. Thanks for your input. I have opened https://reviews.llvm.org/D53220, which removes that option. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D53220: Remove possibility to change compile database path at runtime

simark marked an inline comment as done. simark added inline comments. Comment at: clangd/ClangdLSPServer.h:90 void reparseOpenedFiles(); + void applyConfiguration(const ClangdInitializationOptions &Settings); void applyConfiguration(const ClangdConfigurationParamsChange &

[PATCH] D53220: Remove possibility to change compile database path at runtime

simark marked 7 inline comments as done. simark added inline comments. Comment at: clangd/ClangdLSPServer.cpp:433 reparseOpenedFiles(); } sammccall wrote: > sammccall wrote: > > This isn't needed, the compilation database can only be set during > > ini

[PATCH] D53220: Remove possibility to change compile database path at runtime

simark updated this revision to Diff 169828. simark marked 3 inline comments as done. simark added a comment. Address comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53220 Files: clangd/ClangdLSPServer.cpp clangd/Protocol.cpp clangd/Protocol.h test/clangd/compil

[PATCH] D53220: Remove possibility to change compile database path at runtime

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE344614: Remove possibility to change compile database path at runtime (authored by simark, committed by ). Changed prior to commit: https://reviews.llvm.org/D53220?vs=169828&id=169833#toc Repository:

[PATCH] D54077: [clangd] Implemented DraftFileSystem

simark added a comment. In https://reviews.llvm.org/D54077#1287153, @klimek wrote: > I'm in yet another camp: I carefully save when I have something that is > correct enough syntax, so I only want errors from with changes from the exact > file I'm editing and the rest of the files in saved stat

[PATCH] D55250: [clangd] Enhance macro hover to see full definition

simark added inline comments. Comment at: clangd/XRefs.cpp:572 + + // Try to get the full definition, not just the name + SourceLocation StartLoc = Decl.Info->getDefinitionLoc(); hokein wrote: > if this is a complicated macro (like `AST_MATCHER`), do we still w

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

simark added a comment. Ohh awesome, I didn't know the LSP supported that. I'll try it it Theia when I have time. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://

[PATCH] D51725: Allow un-setting the compilation database path

simark added a comment. So I revisited this today. In the end, restarting clangd in our IDE works well. It should be merged anytime soon: https://github.com/theia-ide/theia/pull/3017 But I am wondering what to do with the feature that allows clangd to change compilation database path using t

[PATCH] D52311: [clangd] Add support for hierarchical documentSymbol

simark added a comment. I just tried this, this looks very promising! It should help build our outline view in a much more robust way than we do currently. A nit for the final patch, I would suggest omitting the fields that are optional, like `children` (when the list is empty) and `deprecated

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

simark added a comment. In D55363#1329652 , @ilya-biryukov wrote: > Sorry if I missed any important design discussions here, but wanted to clear > up what information we are trying to convey to the user with the status > messages? > E.g. why seeing "bu

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

simark created this revision. Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov. When compile_commands.json contains some source files expressed as relative paths, we can get duplicate responses to findDefinitions. The responses only differ by the URI, which are diffe

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

simark added a comment. In https://reviews.llvm.org/D48687#1146308, @ilya-biryukov wrote: > Thanks for the patch! > Could we try to figure out why the duplicates were there in the first place > and why the paths were different? I tried to do that, but it goes deep in the clang internals with

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

simark added a comment. Btw, this seems to happen only when the included header is in the preamble. If I put a variable declaration before `#include "first.h"`, things work as expected, we have not duplicates and the path is normalized. Repository: rCTE Clang Tools Extra https://reviews.ll

[PATCH] D48726: Attempt to write a test to reproduce #37963

simark created this revision. Herald added subscribers: cfe-commits, jkorous, ioeric, ilya-biryukov. This is not intended to be merged, it is my attempt at reproducing this: https://bugs.llvm.org/show_bug.cgi?id=37963 using a unit test. However, I can't manage to get the faulty result like this

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

simark added a comment. FYI, I have uploaded my attempt at writing a unit test here: https://reviews.llvm.org/D48726 I tried to make it as close as possible to the example in the bug report, but don't manage to reproduce the bug. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

simark added a comment. In https://reviews.llvm.org/D48687#1150960, @hokein wrote: > After taking a look closely, I figured why there are two candidates here -- > one is from AST (the one with ".." path); the other one is from dynamic > index, the deduplication failed because of the different p

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark created this revision. Herald added subscribers: cfe-commits, ioeric, ilya-biryukov. InMemoryFileSystem::status behaves differently than RealFileSystem::status. The Name contained in the Status returned by RealFileSystem::status will be the path as requested by the caller, whereas InMemory

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark updated this revision to Diff 154010. simark added a comment. Update commit message Repository: rC Clang https://reviews.llvm.org/D48903 Files: lib/Basic/VirtualFileSystem.cpp Index: lib/Basic/VirtualFileSystem.cpp ==

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

simark added a comment. An update, I traced the difference in behavior to the difference in how `RealFileSystem` and `InMemoryFileSystem` return `Status`es. I uploaded a patch to change `InMemoryFileSystem` to work like `RealFileSystem`: https://reviews.llvm.org/D48903 With this patch applied

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark added a comment. In https://reviews.llvm.org/D48903#1151847, @hokein wrote: > I'm not familiar with this part of code, but the change looks fine to me. I > think @bkramer is the right person to review it. > > Please make sure the style align with LLVM code style. Woops indeed I forgot t

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark added a comment. In https://reviews.llvm.org/D48903#1151866, @ilya-biryukov wrote: > Mimicing RealFS seems like the right thing to do here, so I would vouch for > checking this change in. > I'm a little worried that there are tests/clients relying on the old > behavior, have you run all

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark updated this revision to Diff 154118. simark added a comment. - Fixed formatting (ran git-clang-format) - Fixed expectation in TEST_F(InMemoryFileSystemTest, WorkingDirectory) - Added test TEST_F(InMemoryFileSystemTest, StatusName) Repository: rC Clang https://reviews.llvm.org/D48903

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark added a comment. I ran `ninja clang-test`: Testing Time: 1720.20s Expected Passes: 12472 Expected Failures : 18 Unsupported Tests : 263 Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark added a comment. In https://reviews.llvm.org/D48903#1152485, @ilya-biryukov wrote: > I usually run `ninja check-clang check-clang-tools` for clang changes. Have > never used `clang-test`, not sure what it does. I think `clang-test` is an alias for `check-clang`. > I ran it with this ch

[PATCH] D48951: [clang-move] ClangMoveTests: Remove dots in output paths

simark created this revision. Herald added subscribers: cfe-commits, ioeric. Following https://reviews.llvm.org/D48903 ([VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name), the paths output by clang-move in the FileToReplacements map may contain leading "./".

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark added a comment. I opened https://reviews.llvm.org/D48951 to fix the failures in clang-move. Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark added a comment. In https://reviews.llvm.org/D48903#1153142, @hokein wrote: > Seems to me you have a few comments unaddressed (and make sure you marked > them done when updating the patch). Ah damn I missed them, I'm not too used to how Phabricator displays things. I'll do that. Rep

[PATCH] D48951: [clang-move] ClangMoveTests: Remove dots in output paths

This revision was automatically updated to reflect the committed changes. Closed by commit rL336358: [clang-move] ClangMoveTests: Remove dots in output paths (authored by simark, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48951 Files

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark marked 6 inline comments as done. simark added inline comments. Comment at: lib/Basic/VirtualFileSystem.cpp:516 + explicit InMemoryFileAdaptor(InMemoryFile &Node, std::string RequestedName) + : Node(Node), RequestedName (std::move (RequestedName)) + {}

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark updated this revision to Diff 154321. simark marked 4 inline comments as done. simark added a comment. - Add RequestedName to InMemoryNode::getStatus. - Also fix the directory_iterator code path. Repository: rC Clang https://reviews.llvm.org/D48903 Files: lib/Basic/VirtualFileSystem

[PATCH] D49008: [clangd] Upgrade logging facilities with levels and formatv.

simark added a comment. In https://reviews.llvm.org/D49008#1154196, @sammccall wrote: > @simark - is it OK if I pick this up? Yes, totally fine. I didn't have time to follow up on my patch, and I think you did a great job here. I tested it, and I see that my main pain point was addressed.

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark updated this revision to Diff 154422. simark marked an inline comment as done. simark added a comment. - Use StringRef in InMemoryNode::getStatus - Remove unused variable in TEST_F(InMemoryFileSystemTest, StatusName) Repository: rC Clang https://reviews.llvm.org/D48903 Files: lib/Ba

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark added a comment. I found something fishy. There is this code in FileManager.cpp: if (UFE.File) if (auto RealPathName = UFE.File->getName()) UFE.RealPathName = *RealPathName; The real path is obtained from `UFE.File->getName()`. In the `RealFile` implementation, it returns t

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark marked 6 inline comments as done. simark added inline comments. Comment at: lib/Basic/VirtualFileSystem.cpp:488 + } + StringRef getName() const { return Stat.getName(); } InMemoryNodeKind getKind() const { return Kind; } ilya-biryukov wrote: > Given t

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark marked an inline comment as done. simark added a comment. In https://reviews.llvm.org/D48903#1155403, @ilya-biryukov wrote: > In https://reviews.llvm.org/D48903#1154846, @simark wrote: > > > With the `InMemoryFileSystem`, this now returns a non-real path. The > > result is that we fill `

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark updated this revision to Diff 154631. simark added a comment. - Use FileSystem::getRealPath in FileManager::getFile Repository: rC Clang https://reviews.llvm.org/D48903 Files: lib/Basic/FileManager.cpp lib/Basic/VirtualFileSystem.cpp unittests/Basic/VirtualFileSystemTest.cpp u

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark updated this revision to Diff 154662. simark added a comment. - Change InMemoryNode::getName to InMemoryNode::getFileName, to reduce the risk of mis-using it. Make the Stat field protected, make the subclasses' toString access it directly. Repository: rC Clang https://reviews.llvm.or

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark updated this revision to Diff 154782. simark marked 5 inline comments as done. simark added a comment. - Make InMemoryNode::Stat private again, add protected accessor. - Change condition formatting. Repository: rC Clang https://reviews.llvm.org/D48903 Files: lib/Basic/FileManager.cp

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark marked an inline comment as done. simark added a comment. In https://reviews.llvm.org/D48903#1157330, @ilya-biryukov wrote: > LGTM if that does not introduce any regressions in clang and clang-tools. Thanks. I have seen no failures in `check-clang` and `check-clang-tools`, so I will pu

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark updated this revision to Diff 154835. simark added a comment. Bump SmallString size from 32 to 256 Repository: rC Clang https://reviews.llvm.org/D48903 Files: lib/Basic/FileManager.cpp lib/Basic/VirtualFileSystem.cpp unittests/Basic/VirtualFileSystemTest.cpp unittests/Driver/T

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

simark added a comment. Herald added a subscriber: omtcyfz. I took a look at `SymbolCollector`'s `toURI`, and I am not sure what to get from it. It seems like a lot of it could be replaced with a call to `FileSystem::getRealPath`. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

This revision was automatically updated to reflect the committed changes. Closed by commit rC336807: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the… (authored by simark, committed by ). Changed prior to commit: https://reviews.llvm.org/D48903?vs=154835&id=154995#toc R

[PATCH] D49197: FileManager: Try to compute RealPathName in getVirtualFile

simark created this revision. Herald added a subscriber: cfe-commits. I noticed that when getVirtualFile is called for a file, subsequent calls to getFile will return a FileEntry without the RealPathName computed: const FileEntry *VFE = Mgr->getVirtualFile("/foo/../bar", ...); const FileEntry

[PATCH] D49197: FileManager: Try to compute RealPathName in getVirtualFile

simark updated this revision to Diff 155027. simark added a comment. Update commit message. Repository: rC Clang https://reviews.llvm.org/D49197 Files: lib/Basic/FileManager.cpp unittests/Basic/FileManagerTest.cpp Index: unittests/Basic/FileManagerTest.cpp =

[PATCH] D49197: FileManager: Try to compute RealPathName in getVirtualFile

simark added a comment. I'm not sure who should review this patch, please add anybody you think is qualified/responsible. Repository: rC Clang https://reviews.llvm.org/D49197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D49197: FileManager: Try to compute RealPathName in getVirtualFile

simark added a comment. Background: I'm trying to fix the cases why we receive a FileEntry without a real path computed in clangd, so we can avoid having to compute it ourselves. Repository: rC Clang https://reviews.llvm.org/D49197 ___ cfe-comm

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark added a comment. In https://reviews.llvm.org/D48903#1159023, @ioeric wrote: > Would you mind reverting this patch for now so that we can come up with a > solution to address those use cases? > > Sorry again about missing the discussion earlier! Of course, feel free to revert if needed (

[PATCH] D49197: FileManager: Try to compute RealPathName in getVirtualFile

simark added inline comments. Comment at: lib/Basic/FileManager.cpp:395 + SmallString<128> RealPathName; + if (!FS->getRealPath(InterndFileName, RealPathName)) +UFE->RealPathName = RealPathName.str(); ioeric wrote: > It seems that at this point, we have fai

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

simark updated this revision to Diff 155057. simark added a comment. This is an update of my work in progress. I haven't done any sharing with SymbolCollector, as it's not clear to me how to proceed with that. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 Files: clang

[PATCH] D49197: FileManager: Try to compute RealPathName in getVirtualFile

simark added a comment. In https://reviews.llvm.org/D49197#1159704, @ioeric wrote: > In https://reviews.llvm.org/D49197#1158972, @simark wrote: > > > Background: I'm trying to fix the cases why we receive a FileEntry without > > a real path computed in clangd, so we can avoid having to compute

[PATCH] D49197: FileManager: Try to compute RealPathName in getVirtualFile

simark added inline comments. Comment at: lib/Basic/FileManager.cpp:395 + SmallString<128> RealPathName; + if (!FS->getRealPath(InterndFileName, RealPathName)) +UFE->RealPathName = RealPathName.str(); ioeric wrote: > simark wrote: > > ioeric wrote: > > > It

[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name

simark added a comment. In https://reviews.llvm.org/D48903#1159303, @ioeric wrote: > For example, suppose you have header search directories `-I/path/to/include` > and `-I.` in your compile command. When preprocessor searches for an #include > "x.h", it will try to stat "/path/to/include/x.h" a

[PATCH] D49253: [clangd] Fix category in clangd-vscode's package.json

simark created this revision. Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov. When opening package.json, vscode shows: Use 'Programming Languages' instead Replacing "Languages" with this fixes it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org

[PATCH] D49253: [clangd] Fix category in clangd-vscode's package.json

simark updated this revision to Diff 155213. simark added a comment. - [clangd] JSONTracer: flush after writing event Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49253 Files: clangd/Trace.cpp clangd/clients/clangd-vscode/package.json Index: clangd/clients/clangd-vscode

[PATCH] D49253: [clangd] Fix category in clangd-vscode's package.json

simark updated this revision to Diff 155232. simark added a comment. Oops. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49253 Files: clangd/clients/clangd-vscode/package.json Index: clangd/clients/clangd-vscode/package.json

[PATCH] D49260: [clangd] JSONTracer: flush after writing event

simark created this revision. Herald added subscribers: cfe-commits, omtcyfz, jkorous, MaskRay, ioeric, ilya-biryukov. Let's say I use "CLANGD_TRACE=/tmp/clangd.json" and "tail -F /tmp/clangd.json", I'll often see unfinished lines, where the rest of the data is still in clangd's output buffer. D

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

simark created this revision. Herald added subscribers: cfe-commits, ioeric, ilya-biryukov. It does the obvious thing of comparing all fields. This will be needed for a clangd patch I have in the pipeline. Repository: rC Clang https://reviews.llvm.org/D49265 Files: include/clang/Tooling/C

[PATCH] D49267: [clangd] Watch for changes in compile_commands.json

simark created this revision. Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov. This patch adds support for watching for changes to compile_commands.json, and reparsing files if needed. The watching is done using the "workspace/didChangeWatchedFiles" notification, so

[PATCH] D49267: [clangd] Watch for changes in compile_commands.json

simark added a comment. Note, https://reviews.llvm.org/D49265 in clang is a prerequisite. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D49267: [clangd] Watch for changes in compile_commands.json

simark updated this revision to Diff 155284. simark added a comment. Remove unintended changes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49267 Files: clangd/ClangdLSPServer.cpp clangd/ClangdLSPServer.h clangd/GlobalCompilationDatabase.cpp clangd/GlobalCompilationDa

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

simark added a comment. In https://reviews.llvm.org/D49265#1163740, @dblaikie wrote: > Any chance this can/should be unit tested? (also, in general (though might > not matter in this instance), symmetric operators like == should be > implemented as non-members (though they can still be friends

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

simark updated this revision to Diff 155753. simark added a comment. - Add test - Make operator== a function instead of method - Add operator!= (so I can use EXPECT_NE in the test, and because it may be useful in general) Repository: rC Clang https://reviews.llvm.org/D49265 Files: include

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

simark added a comment. In https://reviews.llvm.org/D49265#1164227, @dblaikie wrote: > In theory you'd need separate tests for op== and op!= returning false > (currently all the tests would pass if both implementations returned true in > all cases), but not the biggest deal. Good point, I'll

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

simark updated this revision to Diff 155773. simark added a comment. Add tests for both == and !=. I need to rebuild ~800 files (because I pulled llvm/clang), so I did not actually test it, but I'll do so before pushing tomorrow, of course. Repository: rC Clang https://reviews.llvm.org/D4926

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

This revision was automatically updated to reflect the committed changes. Closed by commit rC337284: [Tooling] Add operator== to CompileCommand (authored by simark, committed by ). Changed prior to commit: https://reviews.llvm.org/D49265?vs=155773&id=155883#toc Repository: rL LLVM https://r

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

This revision was automatically updated to reflect the committed changes. Closed by commit rL337284: [Tooling] Add operator== to CompileCommand (authored by simark, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D49265 Files: cfe/trunk/

[PATCH] D44673: Make positionToOffset return llvm::Expected

simark updated this revision to Diff 139157. simark marked 9 inline comments as done. simark added a comment. Herald added a subscriber: mgorny. Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44673 Files: clangd/ClangdServer.cpp clangd/SourceCode.cpp

[PATCH] D44673: Make positionToOffset return llvm::Expected

simark added inline comments. Comment at: clangd/SourceCode.h:24 -/// Turn a [line, column] pair into an offset in Code. -size_t positionToOffset(llvm::StringRef Code, Position P); +/// Turn a [line, column] pair into an offset in Code according to the LSP +/// definition of a

[PATCH] D44673: Make positionToOffset return llvm::Expected

simark updated this revision to Diff 139292. simark marked 3 inline comments as done. simark added a comment. Remove spaces, add fixmes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44673 Files: clangd/ClangdServer.cpp clangd/SourceCode.cpp clangd/SourceCode.h unittest

[PATCH] D44673: Make positionToOffset return llvm::Expected

simark marked 6 inline comments as done. simark added inline comments. Comment at: clangd/ClangdServer.cpp:199 +return End.takeError(); + + return formatCode(Code, File, {tooling::Range(*Begin, *End - *Begin)}); ilya-biryukov wrote: > NIT: unnecessary empty

[PATCH] D44673: Make positionToOffset return llvm::Expected

This revision was automatically updated to reflect the committed changes. Closed by commit rL328100: Make positionToOffset return llvm::Expected (authored by simark, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44673 Files: c

[PATCH] D44673: Make positionToOffset return llvm::Expected

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE328100: Make positionToOffset return llvm::Expected (authored by simark, committed by ). Changed prior to commit: https://reviews.llvm.org/D44673?vs=139292&id=139297#toc Repository: rL LLVM

[PATCH] D44272: [clangd] Support incremental document syncing

simark marked 34 inline comments as done. simark added inline comments. Comment at: clangd/DraftStore.h:36 /// Replace contents of the draft for \p File with \p Contents. - void updateDraft(PathRef File, StringRef Contents); + void addDraft(PathRef File, StringRef Contents);

[PATCH] D44272: [clangd] Support incremental document syncing

simark updated this revision to Diff 139310. simark marked an inline comment as done. simark added a comment. Address review comments, except LSP version check Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44272 Files: clangd/ClangdLSPServer.cpp clangd/DraftStore.cpp cla

[PATCH] D44272: [clangd] Support incremental document syncing

simark updated this revision to Diff 139321. simark added a comment. Remove draft if update fails Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44272 Files: clangd/ClangdLSPServer.cpp clangd/DraftStore.cpp clangd/DraftStore.h clangd/Protocol.cpp clangd/Protocol.h t

[PATCH] D44272: [clangd] Support incremental document syncing

simark updated this revision to Diff 139328. simark added a comment. Add lit test case Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44272 Files: clangd/ClangdLSPServer.cpp clangd/DraftStore.cpp clangd/DraftStore.h clangd/Protocol.cpp clangd/Protocol.h test/clangd/

[PATCH] D44272: [clangd] Support incremental document syncing

simark marked 6 inline comments as done. simark added inline comments. Herald added a subscriber: MaskRay. Comment at: unittests/clangd/DraftStoreTests.cpp:27 + +static int rangeLength(StringRef Code, const Range &Rng) { + size_t Start = positionToOffset(Code, Rng.start); --

[PATCH] D44272: [clangd] Support incremental document syncing

simark updated this revision to Diff 139355. simark marked an inline comment as done. simark added a comment. Address review comments I failed to address previously Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44272 Files: clangd/ClangdLSPServer.cpp clangd/DraftStore.cpp

[PATCH] D44764: [clangd] Move GTest printers to separate file

simark added a comment. We could create a file `ClangdTesting.h" which includes everything tested related (gtest, gmock, printers, syncapi, etc). The convention would be that test files would just include that. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44764 __

[PATCH] D44272: [clangd] Support incremental document syncing

simark added inline comments. Comment at: clangd/DraftStore.h:36 /// Replace contents of the draft for \p File with \p Contents. - void updateDraft(PathRef File, StringRef Contents); + void addDraft(PathRef File, StringRef Contents); + ilya-biryukov wrote: >

[PATCH] D44272: [clangd] Support incremental document syncing

simark marked 10 inline comments as done. simark added inline comments. Comment at: clangd/DraftStore.cpp:75 + return llvm::make_error( + llvm::formatv("Range's end position (line={0}, character={1}) is " +"before start position (line={2}, ch

[PATCH] D44272: [clangd] Support incremental document syncing

simark updated this revision to Diff 139596. simark marked 2 inline comments as done. simark added a comment. Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44272 Files: clangd/ClangdLSPServer.cpp clangd/DraftStore.cpp clangd/DraftStore.h clangd/

[PATCH] D44764: [clangd] Use a header for common inclusions for tests

simark accepted this revision. simark added inline comments. Comment at: unittests/clangd/Printers.h:12 +// objects on failure. Otherwise, it is possible that there will be two +// different instantiations of the printer template for a given type and some +// tests could end up c

<    1   2   3   >