[RFC] Add Windows support to path_is_absolute() #5198
Conversation
219ada0 to
1ee7a53
Compare
be7ba16 to
48bd0be
Compare
|
Fixed lint errors |
| /// @return `TRUE` if "fname" is absolute. | ||
| int path_is_absolute_path(const char_u *fname) | ||
| { | ||
| #ifdef WIN32 | ||
| // A name like "d:/foo" and "//server/share" is absolute | ||
| return ((fname[0] && fname[1] == ':' && (fname[2] == '/' || fname[2] == '\\')) |
There was a problem hiding this comment.
In get_past_head() we used isalpha(fname[0]) instead of fname[0] to check for 'd' in "d:". Why is it not used here?
|
Looking at it the check could be changed into |
e743f39 to
f76aaec
Compare
| // A name like "d:/foo" and "//server/share" is absolute | ||
| return (isalpha(fname[0]) && fname[1] == ':' | ||
| && vim_ispathsep_nocolon(fname[2])) | ||
| || (fname[0] == fname[1] && vim_ispathsep_nocolon(fname[0])); |
There was a problem hiding this comment.
If fname can be an empty string, the check fname[0] == fname[1] should not be used (here), because fname[1] is not part of the string. The arguments of && could be switched to fix this.
f76aaec to
62c7dec
Compare
| /// @return `TRUE` if "fname" is absolute. | ||
| int path_is_absolute_path(const char_u *fname) | ||
| { | ||
| #ifdef WIN32 | ||
| // A name like "d:/foo" and "//server/share" is absolute | ||
| return (isalpha(fname[0]) && fname[1] == ':' |
There was a problem hiding this comment.
AFAIR isalpha is locale-dependent and may return true unexpectedly if locale happens to be non-UTF-8 (and, theoretically, it may return unexpected false if locale is not C, but I doubt you will find any locale which rejects latin letters). I would suggest using not C library, but our ascii_isalpha function from src/nvim/lib/ascii.h.
There was a problem hiding this comment.
I would prefer to deal with this one separately. The use of isalpha is similar to what is done in vim/vim and there are a least a couple other locations, unrelated to this PR, where isalpha is used.
62c7dec to
1951cda
Compare
43b6082 to
ccb6af0
Compare
vim-patch:0
Check if drive letter is alphabetic character in path_is_absolute_path().
|
Poked travis, builds look ok now. |
| /// @return `TRUE` if "fname" is absolute. | ||
| int path_is_absolute_path(const char_u *fname) | ||
| { | ||
| #ifdef WIN32 | ||
| // A name like "d:/foo" and "//server/share" is absolute | ||
| return ((isalpha(fname[0]) && fname[1] == ':' |
There was a problem hiding this comment.
Vim's mch_isFullName doesn't check isalpha. Apparently old versions of Windows supported non-alpha drive letters.
|
What about UNC paths? |
|
UNC paths are covered by the condition. |
From, cherry picked from #810 vim/vim https://github.com/vim/vim/blob/8767f52fbfd4f053ce00a978227c95f1d7d323fe/src/os_mswin.c#L449