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

Determine the current workspace directory #923

Merged
merged 17 commits into from
Mar 30, 2020
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 51 additions & 4 deletions autoload/phpactor.vim
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ endfunction
" Utility functions
"""""""""""""""""""""""
function! s:isOpenInCurrentWindow(filePath)
return expand('%:p') == a:filePath
return phpactor#_path() == a:filePath
endfunction

function! phpactor#_switchToBufferOrEdit(filePath)
Expand All @@ -353,7 +353,15 @@ function! phpactor#_source()
endfunction

function! phpactor#_path()
return expand('%:p')
let l:path = expand('%:p')

if filereadable(l:path) || stridx(l:path, '/') == 0
return l:path
endif

" todo if empty path
"
return printf('%s/%s', g:phpactorInitialCwd, l:path)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what problem does this change solve?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I needed a while to remember its origin after week.

It came directly from one of test cases that did not pass. I have explained it in #923 (comment).

endfunction

function! s:getStartOffsetFromMark(mark, linewise)
Expand Down Expand Up @@ -435,14 +443,53 @@ endfunction
" RPC -->-->-->-->-->--
"""""""""""""""""""""""

function! s:searchDirectoryUpwardForRootPatterns(initialDirectory, workspaceRootPatterns, fallbackDirectory)
if index(g:phpactorGlobalRootPatterns, '/') < 0
call add(g:phpactorGlobalRootPatterns, '/')
endif
Copy link
Collaborator

@dantleech dantleech Mar 30, 2020

Choose a reason for hiding this comment

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

maybe add some commentry to this function? not to concerned however (would have just saved me doing :help index() possibly, and not sure that's a good thing)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have explained it in doc comment added in ftplugin. It forces excluding system root directory / from treating as workspace root.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, it checks if g:phpactorGlobalRootPatterns contains / (it may be not added by user). If not, it adds the system root to that list.


let l:directory = a:initialDirectory

while index(g:phpactorGlobalRootPatterns, l:directory) < 0
if s:directoryMatchesToPatterns(l:directory, a:workspaceRootPatterns)
return l:directory
endif

let l:directory = fnamemodify(l:directory, ':h')
Copy link
Collaborator

Choose a reason for hiding this comment

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

this would still detect the /home is it had a global pattern in it or?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See comment below

endwhile

if index(g:phpactorGlobalRootPatterns, l:directory) >= 0
let l:directory = a:fallbackDirectory
endif
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Notice that there these patterns are used also to exclude them.

The one caveat is a case when a:fallbackDirectory is also one of them ;) (edit: it is also possible previously).

What do you think about writing some tests?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes - testing is relatively easy with https://github.com/junegunn/vader.vim tests are in tests/VimPlugin can help out there if needed - there are several examples already. (we don't reallyt have the habbit of writing tests for the plugin, but we should)


return l:directory
endfunction

function s:directoryMatchesToPatterns(directory, patterns) abort
for l:pattern in a:patterns
if (filereadable(a:directory .'/'. l:pattern))
return v:true
endif
endfor

return v:false
endfunction

function! phpactor#rpc(action, arguments)
" Remove any existing output in the message window
execute ':redraw'

let request = { "action": a:action, "parameters": a:arguments }

let cmd = g:phpactorPhpBin . ' ' . g:phpactorbinpath . ' rpc --working-dir=' . g:phpactorInitialCwd
let result = system(cmd, json_encode(request))
let l:workspaceDir = s:searchDirectoryUpwardForRootPatterns(
\ fnamemodify(phpactor#_path(), ':h'),
\ g:phpactorProjectRootPatterns,
\ g:phpactorInitialCwd
\)

let l:cmd = g:phpactorPhpBin . ' ' . g:phpactorbinpath . ' rpc --working-dir=' . l:workspaceDir

let result = system(l:cmd, json_encode(request))

if (v:shell_error == 0)
try
Expand Down
11 changes: 6 additions & 5 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions ftplugin/php.vim
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ let g:phpactorInputListStrategy = get(g:, 'phpactorInputListStrategy', 'phpactor
" to that window instead of switching buffers. The default is false.
let g:phpactorUseOpenWindows = get(g:, 'phpactorUseOpenWindows', v:false)

""
" The list of files that determine workspace root directory
" if contained within
let g:phpactorProjectRootPatterns = get(g:, 'phpactorProjectRootPatterns', ['composer.json', '.git', '.phpactor.json', '.phpactor.yml'])

""
" The list of directories that should not be considered as workspace root directory
" (in addition to '/' which is always considered)
let g:phpactorGlobalRootPatterns = get(g:, 'phpactorGlobalRootPatterns', ['/', '/home'])

if g:phpactorOmniAutoClassImport == v:true
autocmd CompleteDone *.php call phpactor#_completeImportClass(v:completed_item)
endif
Expand Down