[PATCH] D96690: [clangd] Treat paths case-insensitively depending on the platform

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/support/Path.cpp:22
 
-bool pathEqual(PathRef A, PathRef B) {
-#if defined(_WIN32) || defined(__APPLE__)
-  return A.equals_lower(B);
-#else
-  return A == B;
-#endif
+bool pathIsAncestor(PathRef Ancestor, PathRef Path,
+                    llvm::sys::path::Style Style) {
----------------
sammccall wrote:
> we should decide what the preconditions are here
> 
> Could assert both are absolute paths, this places a burden on callers.
> 
> I do like the idea this is "just" a smarter lexical startswith. (I'd consider pathStartsWith rather than pathIsAncestor for this reason)
> In this case we should doc it (e.g. foo/./bar is not an ancestor of foo/bar/baz) and return false when Ancestor is empty (rather than crash)
oops nice catch. going with the latter option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96690