njames93 added inline comments.

================
Comment at: clang-tools-extra/clangd/DraftStore.h:37
 
+  DraftStore(const ThreadsafeFS &BaseFS);
+
----------------
sammccall wrote:
> having DraftStore sit on top of TFS seems a bit inside-out, giving it bigger 
> scope than necessary.
> 
> What about giving DraftStore an asVFS() method that returns an in-memory 
> filesystem?
> 
> then separately we can have a separate DirtyFS : TFS, that has a 
> `DraftStore&` and a `TFS &Base` and implements viewImpl() on top of their 
> public interfaces. Having the dependency in that direction seems more natural 
> to me.
How I had it was a symptom of using the underlying TFS for getting the UniqueID 
of files. But yes now that's gone this approach works.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94554/new/

https://reviews.llvm.org/D94554

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

Reply via email to