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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Nice, thank you!



================
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) {
----------------
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)


================
Comment at: clang-tools-extra/clangd/support/Path.h:18
+#if defined(_WIN32) || defined(__APPLE__)
+#define CLANGD_PATH_CASE_INSENSITIVE
+#endif
----------------
(we could conditionally define this as either 0 or 1 and then use it for regex flags without ifdef, up to you)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96690