ilya-golovenko marked 2 inline comments as done.
ilya-golovenko added inline comments.
================
Comment at: clang-tools-extra/clangd/URI.cpp:29
+bool isWindowsPath(llvm::StringRef Path) {
+ return Path.size() > 1 && llvm::isAlpha(Path[0]) && Path[1] == ':';
----------------
sammccall wrote:
> the UNC paths are also basically a windows thing, can we have
> hasWindowsDriveLetter and isWindowsNetworkPath (or isWindowsUNCPath)?
This kind of network paths are also supported on Unix/Linux system, e.g.
`//hostname/path/file.txt` and `isNetworkPath` will handle those as well. For
example, samba and samba client support such paths. RFC 8089 calls them
"non-local files" with unspecified type of protocol to access the file, i.e. it
is not necessary a UNC path. Does it make sense to continue supporting
Linux/Unix version of network path?
================
Comment at: clang-tools-extra/clangd/URI.cpp:34
+bool isNetworkPath(llvm::StringRef Path) {
+ llvm::StringRef Sep = llvm::sys::path::get_separator();
+ return Path.consume_front(Sep) && Path.consume_front(Sep) && !Path.empty();
----------------
sammccall wrote:
> conventionally LLVM tools accept both slashes on windows and `/` on other
> platforms - i.e. we should probably treat `//foo/bar` as a valid network path
> on windows unless there's a good reason not to.
>
> So `Path.size() > 2 && llvm::sys::path::is_separator(Path[0]) &&
> llvm::sys::path::is_separator(Path[1])`, I guess
I tried to prevent treating paths like `/\path` or `\/path` as network paths.
Maybe the following variant will work:
```
bool isNetworkPath(llvm::StringRef Path) {
return Path.size() > 2 && Path[0] == Path[1] &&
llvm::sys::path::is_separator(Path[0]);
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84172/new/
https://reviews.llvm.org/D84172
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
