ilya-biryukov added inline comments.
================
Comment at: clangd/ClangdLSPServer.h:23
+
+class ClangdLSPServer {
+ class LSPDiagnosticsConsumer;
----------------
klimek wrote:
> ilya-biryukov wrote:
> > klimek wrote:
> > > I'd have expected something that's called LSP server to work on the LSP
> > > protocol level (that is, have a server(iostream) equivalent method).
> > The idea is to have a class that wraps ClangdServer and is called by
> > clangd's ProtocolHandlers.cpp to do all the work.
> > I.e. it's a replacement of what used to be called 'ASTManager'. Any
> > suggestions for a better name? ClangdLSPHandler?
> >
> > It's seems hard to implement the interface you suggest right away, lots of
> > code will have to be moved. Happy to do that in further commits, but
> > suggest we leave this class as is in this commit.
> I think it's fine to have an intermediate step checked in; the important part
> is that we add comments explaining why this is the way it is, especially if
> we plan to change it in the future :) -> I'd add a class level comment
> explaining what this class does and why it exists.
Added comments to this class. And ClangdServer, it didn't have a comment too.
================
Comment at: clangd/ClangdUnit.h:43
+
+ /// Reparse with new contents
+ void reparse(StringRef Contents);
----------------
klimek wrote:
> Tiny nit: generally, use "." at the end of sentences :)
Fixed
================
Comment at: clangd/GlobalCompilationDatabase.h:28
+/// Provides compilation arguments used for building ClangdUnit.
+class GlobalCompilationDatabase {
+public:
----------------
klimek wrote:
> ilya-biryukov wrote:
> > klimek wrote:
> > > What's global about this?
> > The fact that it's created only once(as opposed to
> > tooling::CompilationDatabase, which can be recreated for each file) and
> > handles all CompileCommand-related things(i.e. the idea is to add tracking
> > of compile flags changes to this interface).
> > I called it ClangdCompilationDatabase first, but then I thought there's too
> > much Clangd-prefixed names :-)
> > Any suggestions for a better name?
> Why don't we just use a tooling::CompilationDatabase and pass it around?
It has getAllFiles and getAllCompileCommands which we don't need for now.
It doesn't have capabilities to track changes to flags, which we'll want to add
at some point. (Added a comment that we'll be adding those).
Also, currently implementations of CompilationDatabase actually require us to
recreate them for each file, this logic is currently moved into
DirectoryBasedGlobalCompilationDatabase for convenience.
https://reviews.llvm.org/D33047
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
