[RFC] updates cwd with the g:phpactorInitialCwd

[cmizzi]

As PHPActor now handles a --working-dir option, we have to specify the initial working directory instead of Neovim CWD

[roxma]

#1 (comment)

If this PR get merged, the user change directory to another project with nvim's :cd dir, phpactor won't work?

[cmizzi]

Hum, the cwd is only took when the plugin is created. It is not updated on each buffer change until have this kind of functions :

" Update PHPActor cwd each time a new buffer is accessed
function! UpdatePHPActorPath()
	" Change working dir to the current file
	cd %:p:h

	" Set 'gitdir' to be the folder containing .git
	let gitdir = system("git rev-parse --show-toplevel")

	" See if the command output starts with 'fatal' (if it does, not in a git repo)
	if empty(matchstr(gitdir, '^fatal:.*'))
		let g:phpactorInitialCwd = gitdir
	endif
endfunction

autocmd BufEnter *.php call UpdatePHPActorPath()

The cwd will be same during nvim instance until the variable is voluntary changed (cwd is used once)

[roxma]

The cd command inside UpdatePHPActorPath doesn't look good to me.

If cd is executed automatically, plugins like FZF, ag will be affected.

I would rather automatically use the directory of current file for invoking phpactor.

[cmizzi]

This function is not used by the plugin. This is what I do in order to work on multiple projects into a single Vim instance. By default, without this function, the get:phpactorInitialCwd is took from instance creation and is not updated on buffer change.

[roxma]

OK

My point is, it doesn't look like a good idea to accept g:phpactorInitialCwd as phpactor/plugin does

Perhaps @dantleech could give some advice // phpactor/phpactor@65b9793#diff-b96c23eeb9efb3d1ce52e97eaf5019b6R11

[cmizzi]

phpactor assumes that when the instance creation is based on the instance working dir. I'm not really happy with that but as the phpactor binary can handle a --working-dir argument, it's not really a bad thing as it can be modified on the fly.

[roxma]

it's not really a bad thing as it can be modified on the fly.

Yeah. But current PR adds overhead to new users, because they may need to add vimrc to change g:phpactorInitialCwd, while current version doesn't require them to do so.

It is quite straightforward to simply use the value of getcwd() instead. Unless you explicitly configure it not to.

[dantleech]

I think it should be fine to pass the g:phpactorInitialCwd ?

Although since the introduction of RPC, we could equally delegate the call to the Phpactor VIM plugin:

    let suggestions = phpactor#rpc("complete", { "offset": offset, "source": source})

This would also have the additional benefit of any errors being echoed to the status line instead of silently failing.

[roxma]

Although since the introduction of RPC, we could equally delegate the call to the Phpactor VIM plugin:

The whole point of the existance of this plugin is for async completion support, which prevents blocking the ui when running phpactor (vimscript is single-threaded). Though performance is not an issue for phpactor.

[dantleech]

Though performance is not an issue for phpactor.

It can be in some cases (as parsing / reflection is done in real time, with huge classes it's can be a slight issue)

But current PR adds overhead to new users, because they may need to add vimrc to change g:phpactorInitialCwd

Hmm, only if this plugin is registered before the phpactor plugin? But maybe that's non-obvious.

[cmizzi]

I think it's important that the omnifunc and this plugin return same result. If this plugin handles getcwd (so :cd will change the current call path), phpactor needs to do the same thing. If I press <C-o> or simply use the smart-completion, it would be greater if results are the same ?

[roxma]

I understand.

But I still don't like current behavior of g:phpactorInitialCwd.

I think current behavior is better than that.

I'll keep this open, wait for more comments and see how it goes