Skip to content

Normalize paths on Windows#73

Closed
Riatre wants to merge 5 commits into
MaskRay:masterfrom
Riatre:master
Closed

Normalize paths on Windows#73
Riatre wants to merge 5 commits into
MaskRay:masterfrom
Riatre:master

Conversation

@Riatre
Copy link
Copy Markdown
Contributor

@Riatre Riatre commented Sep 11, 2018

Generally, we have three sources of path strings:

  1. LSP document URI and project root.
  2. Clang indexer.
  3. compile_commands.json or GetFileFromDirectory.

Path strings from these three sources may have different slash (forward/backward) and drive letter (lowercase/uppercase) styles, as we use file path as map key, we have to normalize them.

Slashes will be converted to forward slash ('/'), drive letters will be converted to uppercase (On Windows, it is guaranteed to be case insensitive).

MaskRay and others added 5 commits September 10, 2018 00:46
1. Normalize paths in LSP document URIs and project root to forward
slash and uppercase drive letters.
2. Normalize paths in compile_commands.json to forward slash and
uppercase drive letters.
3. Normalize paths from directory listing to forward slash. (Drive
letter should be same as input dir path, which is already uppercase
since path of project root dir is normalized)
4. Add llvm::sys::path::convert_to_slash after certain llvm::sys::path
and llvm::fs calls.
@Riatre
Copy link
Copy Markdown
Contributor Author

Riatre commented Sep 11, 2018

Note that this does not address emacs-lsp/emacs-ccls#10 (comment).
It is caused by a different issue: the auto detection logic in lib/Tooling/JSONCompilationDatabase.cpp thinks the command line is in GNU syntax (where '\' is a escape character instead of path separator) so it eats all '\' in paths. I'll try to fix this in another PR.

@MaskRay MaskRay force-pushed the master branch 10 times, most recently from 9ce714d to 7e9097e Compare September 12, 2018 03:23
@MaskRay
Copy link
Copy Markdown
Owner

MaskRay commented Sep 12, 2018

Manually cherry picked.. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants