malaperle added inline comments.

================
Comment at: clangd/Protocol.h:295
+
+struct ClangdConfigurationParams {
+
----------------
ilya-biryukov wrote:
> malaperle wrote:
> > ilya-biryukov wrote:
> > > Maybe call it `ClangdConfigurationParamsChange` to make it clear those 
> > > are diffs, not the actual params?
> > The idea was that we can reuse the same struct for 
> > InitializeParams.initializationOptions
> Since `InitializeParams.initializationOptions` may also have unset values 
> (`llvm::None`), it also seems fine to treat those as a "diff" between the 
> default parameters and the new ones.
> The reasoning behind naming for me is that if we allow only a subset of 
> fields to be set and use the ones that were set override the corresponding 
> values, it really feels like an entity describing a **change** to the 
> configuration parameters, not the parameters themselves.
> 
> I don't have a strong opinion on this one, though. If you'd prefer to keep 
> the current name, it's totally fine with me.
That makes sense the way you explained it. I think 
ClangdConfigurationParamsChange is good.


https://reviews.llvm.org/D39571



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to