Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve compatibility with windows #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions plugin/gitsessions.vim
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ endfunction

" fix for Windows users (https://github.com/wting/gitsessions.vim/issues/2)
if !exists('g:VIMFILESDIR')
let g:VIMFILESDIR = has('unix') ? $HOME . '/.vim/' : $VIM . '/vimfiles/'
let g:VIMFILESDIR = has('unix') ? $HOME . '/.vim/' : $HOME . '/vimfiles/'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this a tautology and break the original reason why we added this line? Why bother with the unix check?

endif

" sessions save path
Expand Down Expand Up @@ -65,37 +65,45 @@ function! s:os_sep()
endfunction

function! s:is_abs_path(path)
return a:path[0] == s:os_sep()
if has('unix')
return a:path[0] == s:os_sep()
else
return a:path[1] == ':'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of hardcoding the path separator could we simply change the index position? Why use == ':'?

Suggested change
return a:path[1] == ':'
return a:path[1] == s:os_sep()

endif
endfunction

" LOGIC FUNCTIONS

function! s:parent_dir(path)
let l:sep = s:os_sep()
let l:front = s:is_abs_path(a:path) ? l:sep : ''
let l:front = s:is_abs_path(a:path) && has('unix') ? l:sep : ''
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the has('unix') check make the is_abs_path(a:path) changes unnecessary since it'll never be true on Windows?

The result is all absolute paths are converted into relative paths on Windows, regardless if it was an absolute path to begin with or not.

return l:front . join(split(a:path, l:sep)[:-2], l:sep)
endfunction

function! s:find_git_dir(dir)
if isdirectory(a:dir . '/.git')
return a:dir . '/.git'
elseif has('file_in_path') && has('path_extra')
if isdirectory(a:dir . s:os_sep() . '.git')
return a:dir . s:os_sep() . '.git'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this! 👍

elseif has('file_in_path') && has('path_extra') && has('unix')
return finddir('.git', a:dir . ';')
else
return s:find_git_dir_aux(a:dir)
endif
endfunction

function! s:find_git_dir_aux(dir)
return isdirectory(a:dir . '/.git') ? a:dir . '/.git' : s:find_git_dir_aux(s:parent_dir(a:dir))
return isdirectory(a:dir . s:os_sep() . '.git') ? a:dir . s:os_sep() . '.git' : s:find_git_dir_aux(s:parent_dir(a:dir))
endfunction

function! s:find_project_dir(dir)
return s:parent_dir(s:find_git_dir(a:dir))
endfunction

function! s:session_path(sdir, pdir)
let l:path = a:sdir . a:pdir
if has('unix')
let l:path = a:sdir . a:pdir
else
let l:path = a:sdir . substitute(a:pdir,'^\([A-Z]\):','\\\1','g')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is some regex backreferencing stuff. Can you help me understand why this change is necessary?

endif
return s:is_abs_path(a:sdir) ? l:path : g:VIMFILESDIR . l:path
endfunction

Expand Down