[PATCH] D82736: [clangd] Rename FS.view(None) to FS.viewWithDefaultCWD()

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Yeah, IMO we spent a bunch of time on these names, I still like them, and I don't find the arguments since then convincing. New info is of course the warning, and the lack of consensus to change it at clang level.

As for this proposal specifically

- `viewWithDefaultCWD` suggests to me the default has some semantics which don't exist, if using this API "shape" I'd substitute `Arbitrary` here
- I think the argument for changing the *public* API is particularly weak (or missing?). It's more important than the private API, and no change is required to silence the warning. A new name is inevitably a mouthful, and it ties our hands for adding the `Optional` overload.
- I could certainly live with `private: virtual T viewImpl() = 0`. Sounds like some will find that nicer, it's not very intrusive, and we don't need to maintain a flag-divergence from clang

> [virtual]... not that we need it today or it is certain that we'll need one someday

We have an out-of-tree implementation that should be using this, though may not have switched yet. But this also certainly will never really matter, so let's drop `virtual` for simplicity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82736