simark added a comment.
In https://reviews.llvm.org/D48903#1189056, @ilya-biryukov wrote:
> I see, thanks for the explanation.
>
> LGTM for the update revision, given we have confirmation the tests pass on
> Windows.
Thanks, I'll push it, let's hope this time is the right time!
Repository:
This revision was automatically updated to reflect the committed changes.
Closed by commit rC339063: [VirtualFileSystem] InMemoryFileSystem::status:
Return a Status with the… (authored by simark, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48903?vs=158625&id=159401#toc
R
simark updated this revision to Diff 159764.
simark added a comment.
Herald added a subscriber: arphaman.
New version. I tried to share the code between this and SymbolCollector, but
didn't find a good clean way. Do you have concrete suggestions on how to do
this? Otherwise, would it be accepta
simark updated this revision to Diff 159768.
simark added a comment.
Formatting.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48687
Files:
clangd/FindSymbols.cpp
clangd/SourceCode.cpp
clangd/SourceCode.h
clangd/XRefs.cpp
unittests/clangd/TestFS.cpp
unittests/clang
simark marked 3 inline comments as done.
simark added a comment.
In https://reviews.llvm.org/D48687#1193470, @hokein wrote:
> Sorry for the delay. I thought there was a dependent patch of this patch, but
> it seems resolved, right?
>
> A rough round of review.
Right, the patch this one depends
simark updated this revision to Diff 159972.
simark marked an inline comment as done.
simark added a comment.
Replace
RelPathPrefix == StringRef()
with
RelPathPrefix.empty()
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48687
Files:
clangd/FindSymbols.cpp
clangd/Sou
simark marked 8 inline comments as done.
simark added inline comments.
Comment at: clangd/SourceCode.h:69
+llvm::Optional getRealPath(const FileEntry *F,
+const SourceManager &SourceMgr);
ilya-biryukov wrote:
> This funct
simark marked an inline comment as done.
simark added a comment.
In https://reviews.llvm.org/D48687#1195840, @hokein wrote:
> Looks good with a few nits. Looks like you didn't update the patch correctly.
> You have marked comments done, but your code doesn't get changed accordingly.
> Please do
simark updated this revision to Diff 160175.
simark marked 4 inline comments as done.
simark added a comment.
Latest update.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48687
Files:
clangd/FindSymbols.cpp
clangd/SourceCode.cpp
clangd/SourceCode.h
clangd/XRefs.cpp
u
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE339483: [clangd] Avoid duplicates in findDefinitions
response (authored by simark, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48687?vs=160175&id=160197#toc
Repository:
rCTE
simark added inline comments.
Comment at: clangd/SourceCode.h:65
/// Get the absolute file path of a given file entry.
TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
hokein wrote:
> nit: this comment is not needed.
Woops, merge mistake.
simark added inline comments.
Comment at: lib/Basic/FileManager.cpp:319
- SmallString<128> RealPathName;
- if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+ if (UFE.File) {
+if (auto Path = UFE.File->getName()) {
-
simark added inline comments.
Comment at: lib/Basic/FileManager.cpp:319
- SmallString<128> RealPathName;
- if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+ if (UFE.File) {
+if (auto Path = UFE.File->getName()) {
-
simark added inline comments.
Comment at: lib/Basic/FileManager.cpp:319
- SmallString<128> RealPathName;
- if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+ if (UFE.File) {
+if (auto Path = UFE.File->getName()) {
-
simark added inline comments.
Comment at: lib/Basic/FileManager.cpp:319
- SmallString<128> RealPathName;
- if (!FS->getRealPath(InterndFileName, RealPathName))
-UFE.RealPathName = RealPathName.str();
+ if (UFE.File) {
+if (auto Path = UFE.File->getName()) {
-
simark created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, ioeric,
ilya-biryukov.
This is a work in progress patch to support an eventual "get type
hierarchy" request that does not exist in the LSP today. I only plan to
support getting parent types (i.e. ba
simark added inline comments.
Comment at: clangd/Protocol.h:889
+ // Does this node implement the method targeted by the request?
+ bool DeclaresMethod;
+
kadircet wrote:
> I think comment and the name is in contradiction here, do you mean
> DefinesMethod?
Act
simark added inline comments.
Comment at: clangd/Protocol.h:891
+
+ std::vector Parents;
+
ilya-biryukov wrote:
> What is the proposal to add children (i.e. overriden methods) to the
> hierarchy?
> This seems like a place where we might want to have multiple re
simark created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, ioeric,
ilya-biryukov.
It is currently possible to tell clangd where to find the
compile_commands.json file through the initializationOptions or the
didChangeConfiguration message. However, it is no
simark updated this revision to Diff 164183.
simark added a comment.
Fix formatting.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51725
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/GlobalCompilationDatabase.cpp
clangd/GlobalCompilationDatabase.h
simark added a comment.
Not sure who should review this, please feel free to add anybody who would be
appropriate.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51725
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://
simark added a comment.
In https://reviews.llvm.org/D51725#1232092, @ilya-biryukov wrote:
> Wow, this is getting somewhat complicated.
>
> Have you considered rerunning clangd whenever someone changes an option like
> that?
> Would that be much more complicated on your side?
>
> Not opposed to
simark added a comment.
In https://reviews.llvm.org/D51725#1232748, @ilya-biryukov wrote:
> If this setting exposed directly the users in Theia or is it something that
> is exposed via a custom UI (e.g. choosing named build configs or something
> similar)?
It is through a custom UI, that we n
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337697: [clangd] Fix category in clangd-vscode's
package.json (authored by simark, committed by ).
Herald added a subscrib
simark added a comment.
In https://reviews.llvm.org/D49267#1171286, @ilya-biryukov wrote:
> Thanks for putting up this change! It can be really annoying that clangd does
> not pick up the compile commands that got updated.
>
> A few things of the top of my head on why we might want to punt on us
simark added a comment.
In https://reviews.llvm.org/D49267#1173266, @ilya-biryukov wrote:
> The approach is not ideal, but may be a good middle ground before we figure
> out how we approach file watching in clangd. Note that there are other things
> that won't force the updates currently, e.g.
simark added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:430
CDB.clear();
-
-reparseOpenedFiles();
+compileCommandsChangePost(CCChangeData);
}
malaperle wrote:
> ilya-biryukov wrote:
> > Maybe keep the old logic of reparsing all open f
simark created this revision.
simark added reviewers: malaperle, ilya-biryukov, bkramer.
This is a new version of https://reviews.llvm.org/D48903, which has been
reverted. @ioeric fixed
the issues caused by this patch downstream, so he told me it was good to
go again. I also fixed the test fail
simark added a comment.
I uploaded a new version of this patch here:
https://reviews.llvm.org/D49804
Repository:
rC Clang
https://reviews.llvm.org/D48903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
simark reopened this revision.
simark added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D48903#1175317, @ioeric wrote:
> It would make it easier for your reviewers to look at the new changes if
> you just reopen this patch and update the diff :)
I tr
simark updated this revision to Diff 157287.
simark added a comment.
Fix tests on Windows
Fix InMemoryFileSystem tests on Windows. The paths returned by the
InMemoryFileSystem directory iterators in the tests mix posix and windows
directory separators. THis is because we do queries with posix-s
simark added a comment.
Thanks, that's simple and efficient. I'll update
https://reviews.llvm.org/D49267 (to call `reparseOpenFiles` once again) once
this is merged.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49783
___
cfe-co
simark marked 7 inline comments as done.
simark added inline comments.
Comment at: lib/Basic/VirtualFileSystem.cpp:475
InMemoryNodeKind Kind;
+ Status Stat;
+
ilya-biryukov wrote:
> NIT: maybe keep the order of members the same to keep the patch more focused?
simark updated this revision to Diff 157467.
simark marked an inline comment as done.
simark added a comment.
I think this addresses all of Ilya's comments.
Repository:
rC Clang
https://reviews.llvm.org/D48903
Files:
lib/Basic/FileManager.cpp
lib/Basic/VirtualFileSystem.cpp
unittests/B
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338057: [VirtualFileSystem] InMemoryFileSystem::status:
Return a Status with the… (authored by simark, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48903?vs=157467&id=157548#toc
R
simark added a comment.
In https://reviews.llvm.org/D49833#1182615, @malaperle wrote:
> @simark would you mind finishing this patch and committing it? I won't be
> able to finish it given that I started it at a different company, etc.
Yes, I'll take it over, thanks for getting it started.
Re
simark updated this revision to Diff 158366.
simark added a comment.
Address Ilya's comment (add an indirection for initializationOptions).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49833
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/Protocol.cpp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338518: [clangd] Receive compilationDatabasePath in
'initialize' request (authored by simark, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D
simark marked 2 inline comments as done.
simark added a comment.
Thanks, done.
Repository:
rL LLVM
https://reviews.llvm.org/D49833
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
simark added a subscriber: eric_niebler.
simark added a comment.
@eric_niebler, question for you:
This patch causes clang to emit a `-Wnonportable-include-path` warning where it
did not before. It affects the following test on Windows:
https://github.com/llvm-mirror/clang/blob/master/test/PCH/c
simark updated this revision to Diff 158625.
simark added a comment.
Add -Wno-nonportable-include-path to test/PCH/case-insensitive-include.c
Repository:
rC Clang
https://reviews.llvm.org/D48903
Files:
lib/Basic/FileManager.cpp
lib/Basic/VirtualFileSystem.cpp
test/PCH/case-insensitive-
simark added a comment.
That's fine with me. I'll try it and time will tell if I like it or not :).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50154
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/
simark added a comment.
In https://reviews.llvm.org/D50154#1185605, @sammccall wrote:
> Capitalizing sounds nice. +1 to just do it without an option.
>
> My favorite essay on this is
> http://neugierig.org/software/blog/2018/07/options.html
Agreed.
Repository:
rCTE Clang Tools Extra
https
simark created this revision.
Herald added subscribers: cfe-commits, arphaman, jkorous, ioeric, ilya-biryukov.
This patch adds tests for the two ways of changing build configuration
(pointing to a particular compile_commands.json):
- Through the workspace/didChangeConfiguration notification.
- Th
simark added a comment.
This was not tested on windows yet.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50255
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
simark requested review of this revision.
simark added a comment.
In https://reviews.llvm.org/D48903#1187596, @ilya-biryukov wrote:
> This revision got 'reopened' and is now in the list of accepted revision.
> Should we close it again?
It got reverted a second time because it was breaking a te
simark updated this revision to Diff 159050.
simark added a comment.
The tests now work on Windows, I added some path adjustment steps.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50255
Files:
test/clangd/compile-commands-path-in-initialize.test
test/clangd/compile-comma
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338914: [clangd] Add test for changing build configuration
(authored by simark, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D50255
Files:
cl
simark added a comment.
In https://reviews.llvm.org/D50255#1187871, @arphaman wrote:
> Thanks! This LGTM.
Well, thanks for the hints!
Repository:
rL LLVM
https://reviews.llvm.org/D50255
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
simark added a comment.
In https://reviews.llvm.org/D48903#1188566, @malaperle wrote:
> Both check-clang and check-clang-tools pass successfully for me on Windows
> with the patch.
Awesome, thanks!
Repository:
rC Clang
https://reviews.llvm.org/D48903
___
simark updated this revision to Diff 133034.
simark added a comment.
Fix assertion about parsing a document that is not open
As found by Ilya, the getActiveFiles method would return the documents that
were previously opened and then closed.
Repository:
rCTE Clang Tools Extra
https://reviews
simark updated this revision to Diff 133367.
simark added a comment.
Herald added subscribers: ioeric, jkorous-apple.
New version
Here's a new version of the patch, quite reworked. I scaled down the scope a
little bit to make it simpler (see commit log). We'll add more features later
on.
Repo
simark added a comment.
In https://reviews.llvm.org/D35894#1001875, @ilya-biryukov wrote:
> Thanks for picking this up!
> I have just one major comment related to how we generate the hover text.
>
> Current implementation tries to get the actual source code for every
> declaration and then patc
simark added inline comments.
Comment at: unittests/clangd/XRefsTests.cpp:231
+TEST(Hover, All) {
+
+ struct OneTest {
ilya-biryukov wrote:
> NIT: remove empty line at the start of the function
Done.
Comment at: unittests/clangd/XRefsTests.cpp
simark added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:325
+void ClangdLSPServer::onHover(TextDocumentPositionParams &Params) {
+
+ llvm::Expected> H =
ilya-biryukov wrote:
> NIT: remove the empty line at the start of function
Done.
=
simark added a comment.
Another example where pretty-printing the AST gives better results:
int x, y = 5, z = 6;
Hover the `z` will now show `int z = 6`, before it would have shown `int x, y =
5, z = 6`. I think new version is better because it only shows the variable we
care about.
Repos
simark added a comment.
The only problem I see is that when hovering a function name, it now prints the
whole function definition. When talking with @malaperle, he told me that you
had discussed it before and we should not have the definition in the hover,
just the prototype. Glancing quickly
simark added a comment.
In https://reviews.llvm.org/D35894#1003342, @simark wrote:
> The only problem I see is that when hovering a function/struct name, it now
> prints the whole function/struct definition. When talking with @malaperle,
> he told me that you had discussed it before and we sho
simark added a comment.
> I'm not aware of the code that pretty-prints macros. There's a code that
> dumps debug output, but it's not gonna help in that case.
> Alternatively, we could simply print the macro name and not print the
> definition. That's super-simple and is in line with hovers for
simark added a comment.
I think I managed to make some tests by using the `MockCompilationDatabase`.
Basically with some code like:
#ifndef MACRO
static void func () {} // 1
#else
static void func () {} // 2
#endif
and these steps:
1. Server.addDocument(...)
2. Server.findDefiniti
simark updated this revision to Diff 134097.
simark added a comment.
Add tests, work in progress
Can you take a look at the "TEST(DidChangeConfiguration, DifferentDeclaration)"
test, and tell me if you see anything wrong with it? It seems to me like
changes in command line arguments (adding -DMA
simark added a comment.
In https://reviews.llvm.org/D39571#1007291, @ilya-biryukov wrote:
> It looks like a bug in the preamble handling. (It does not check if macros
> were redefined).
> You can workaround that by making sure the preamble ends before your code
> starts (preamble only captures
simark updated this revision to Diff 134306.
simark added a comment.
Generate Hover by pretty-printing the AST
This new version of the patch generates the hover by pretty-printing the AST.
The unit tests are updated accordingly. There are some inconsistencies in how
things are printed. For exam
simark added inline comments.
Comment at: unittests/clangd/XRefsTests.cpp:561
+
+EXPECT_EQ(H.contents.value, Test.expectedHover.str()) << Test.input;
+ }
Note that I used `.str()` here to make the output of failing tests readable and
useful. By default, gt
simark added inline comments.
Comment at: clangd/XRefs.cpp:354
+
+ return Name;
+}
ilya-biryukov wrote:
> We should call `flush()` before returning `Name` here. The
> `raw_string_ostream` is buffered.
I replaced it with `Stream.str()`.
Commen
simark updated this revision to Diff 134437.
simark added a comment.
New version
- Rebase on master, change findHover to have a callback-based interface
- Fix nits from previous review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D35894
Files:
clangd/ClangdLSPServe
simark added a comment.
In https://reviews.llvm.org/D35894#1008593, @ilya-biryukov wrote:
> Just a few last remarks and this is good to go.
> Should I land it for you after the last comments are fixed?
I just received my commit access, so if you are ok with it, I would try to push
it once I g
simark added a comment.
In https://reviews.llvm.org/D35894#1008861, @ilya-biryukov wrote:
> Only the naming of fields left.
>
> Also, please make sure to run `git-clang-format` on the code before
> submitting to make sure it's formatted properly.
Ahh, I was running clang-format by hand, didn't
simark updated this revision to Diff 134440.
simark added a comment.
Fix some nits
- Revert case change of struct fields in Protocol.h
- Change return formatting in onHover
- Use flush() + return Name in NamedDeclQualifiedName and TypeDeclToString
Repository:
rCTE Clang Tools Extra
https://r
simark updated this revision to Diff 134452.
simark added a comment.
Fix field names in XRefsTests.cpp
I realized I forgot to add some fixups in the previous version, the field names
in XRefsTests.cpp were not updated and therefore it did not build.
Repository:
rCTE Clang Tools Extra
https:/
simark accepted this revision.
simark added a comment.
This revision is now accepted and ready to land.
I don't have the setup to test at the moment, but the code looks good to me.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43454
__
simark added a comment.
In https://reviews.llvm.org/D39571#1011877, @ilya-biryukov wrote:
> > That looks like another preamble-handling bug. It's much easier to fix and
> > it's clangd-specific, so I'll make sure to fix that soon.
> > Thanks for bringing this up, we haven't been testing compile
simark updated this revision to Diff 135153.
simark added a comment.
New version, take 2
The previous version contains the changes of already merged patches, I'm not
sure what I did wrong. This is another try.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39571
Files:
clan
simark marked 5 inline comments as done.
simark added inline comments.
Comment at: unittests/clangd/ClangdTests.cpp:492
+
+ EXPECT_TRUE(DiagConsumer.contains(FooCpp));
+ EXPECT_TRUE(DiagConsumer.contains(BarCpp));
ilya-biryukov wrote:
> Maybe expose a copy of t
simark marked an inline comment as done.
simark added inline comments.
Comment at: unittests/clangd/ClangdTests.cpp:492
+
+ EXPECT_TRUE(DiagConsumer.contains(FooCpp));
+ EXPECT_TRUE(DiagConsumer.contains(BarCpp));
simark wrote:
> ilya-biryukov wrote:
> > Maybe
simark updated this revision to Diff 135229.
simark added a comment.
New version
Address comments in https://reviews.llvm.org/D39571#1014237
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39571
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServe
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325784: [clangd] DidChangeConfiguration Notification
(authored by simark, committed by ).
Herald added a subscriber: llvm-
simark updated this revision to Diff 137410.
simark added a comment.
Fix formatting
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44213
Files:
clangd/ClangdLSPServer.cpp
clangd/ProtocolHandlers.cpp
clangd/ProtocolHandlers.h
Index: clangd/ProtocolHandlers.h
simark created this revision.
Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov,
klimek.
Tested by rebuilding.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44213
Files:
clangd/ClangdLSPServer.cpp
clangd/ProtocolHandlers.cpp
clangd/ProtocolHand
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE326947: [clangd] Remove unused field in HandlerRegisterer
(authored by simark, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D44213?vs=137410&id=137471#toc
Repository:
rCTE Clan
simark created this revision.
Herald added subscribers: cfe-commits, ioeric, JDevlieghere, jkorous-apple,
ilya-biryukov, klimek.
Currently, clangd prints all the LSP communication on stderr. While
this is good for developping, I don't think it's good as a general
default. For example, we are in
simark updated this revision to Diff 137478.
simark added a comment.
Changed -log-to-stderr to -log-lsp-to-stderr
The first version disabled a bit too much, this version removes the LSP
communication logging in a more fine grained way.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.
simark added a comment.
In https://reviews.llvm.org/D44226#1030625, @ilya-biryukov wrote:
> I would vouch for adding a log level instead. It's a very well understood
> concept that certainly covers this use-case and can be useful in other places.
> WDYT?
I agree. How would you prefer the fla
simark updated this revision to Diff 137573.
simark added a comment.
Update
- Change switch to -verbose
- Add vlog function, do the filtering there
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44226
Files:
clangd/JSONRPCDispatcher.cpp
clangd/Logger.cpp
clangd/Logger.h
simark added a comment.
Now, if the client calls a method that we do not support (), clangd just
outputs:
C/C++: [10:55:16.033] Error -32601: method not found
It would be useful to at least print in this error message the name of the
method. Because the "unknown method" handler shares the t
simark marked an inline comment as done.
simark added inline comments.
Comment at: clangd/Logger.cpp:19
Logger *L = nullptr;
+bool Verbose_ = false;
} // namespace
ilya-biryukov wrote:
> Could we move the flag to implementation of `Logger`?
> I.e.:
> ```
> clas
simark added inline comments.
Comment at: clangd/tool/ClangdMain.cpp:79
+static llvm::cl::opt Verbose("verbose", llvm::cl::desc("Be more
verbose"),
+ llvm::cl::init(false));
ilya-biryukov wrote:
> Maybe just call it `-v`?
I wo
simark updated this revision to Diff 137582.
simark added a comment.
Update
- Add vlog method to Logger interface
- Add method name to "method not found" error message
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44226
Files:
clangd/ClangdLSPServer.cpp
clangd/JSONRPCDisp
simark marked 2 inline comments as done.
simark added inline comments.
Comment at: clangd/tool/ClangdMain.cpp:79
+static llvm::cl::opt Verbose("verbose", llvm::cl::desc("Be more
verbose"),
+ llvm::cl::init(false));
ilya-biryuk
simark created this revision.
Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov,
mgorny, klimek.
simark added a reviewer: malaperle.
This patch adds support for incremental document syncing, as described
in the LSP spec. The protocol specifies ranges in terms of Positio
simark marked 2 inline comments as done.
simark added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:101
json::obj{
-{"textDocumentSync", 1},
+{"textDocumentSync", 2},
{"documentFormattingProvider", true},
il
simark marked 2 inline comments as done.
simark added inline comments.
Comment at: clangd/ClangdServer.h:159
/// constructor will receive onDiagnosticsReady callback.
void addDocument(PathRef File, StringRef Contents,
+ WantDiagnostics WantDiags = WantDiag
simark created this revision.
Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov,
mgorny, klimek.
This patch moves the draft manager closer to the edge of Clangd, from
ClangdServer to ClangdLSPServer. This will make it easier to implement
incremental document sync, by ma
simark added a comment.
I uploaded a new patch that moves the draft store to ClangdLSPServer, that
implements what you suggested.
https://reviews.llvm.org/D44408
I will update the incremental sync patch once that patch is in.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D442
simark updated this revision to Diff 138253.
simark added a comment.
Rebase
Non-trivial rebase on today's master.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44408
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer
simark added a comment.
In https://reviews.llvm.org/D44408#1036846, @ilya-biryukov wrote:
> We shouldn't add `Contents` parameter to every method, it would defeat the
> purpose of caching ASTs and won't allow to properly manage lifetimes of the
> indexes.
Makes sense.
> The most tricky part
simark accepted this revision.
simark added a comment.
This revision is now accepted and ready to land.
I rebased my patch (https://reviews.llvm.org/D44408) on top of this one, it
looks good.
Comment at: clangd/ClangdServer.cpp:143
+ tooling::CompileCommand NewCommand = Compi
simark updated this revision to Diff 138344.
simark added a comment.
Rebase on https://reviews.llvm.org/D44462
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44408
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
simark added a comment.
I don't see how to avoid adding the Contents parameter to the codeComplete and
signatureHelp methods, since they needed to fetch the document from the draft
manager and pass it to the backend.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44408
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,
sammccall wro
1 - 100 of 217 matches
Mail list logo
