ncm-phpactor
ncm-phpactor copied to clipboard
[RFC] better project root detection.
I think this PR provides better project root detection, for most composer based php project.
It also works when you're openning a file without being in the project root. A typical use case would be openning files with MRU.
Note that this PR conflicts with #2 .
// cc @cmizzi @dantleech
Would it make sense to expose a method in the Phpactor plugin phpactor#get_working_dir and use that?
@dantleech
It make sense to me.
phpactor#get_working_dir should auto detect project root by default , since IMO it works for most composer based php projects.
should auto detect project root by default
It doesn't detect the project root, it just uses the directory in which VIM was initially loaded, I think there are too many edge cases when detecting the root (and the magic behavior can just be confusing).
I guess the other consideration is that this would couple this plugin to the phpactor.vim plugin, and I don't think is the case currently?
I think there are too many edge cases when detecting the root (and the magic behavior can just be confusing).
In what case? phpactor is always depending on vendor/autoload.php generated by composer. And I haven't seen a PHP project using composer without putting vendor directory to the project root.
If it doesn't work by default, then the user need to configure it to work. But with a static value cwd=initialCwd by default, a user mostly need to re-configure it to work.
I guess the other consideration is that this would couple this plugin to the phpactor.vim plugin, and I don't think is the case currently?
I have already documented the requirements to the README page. It's not an issue.
I like this PR. The project detection was removed from phpactor for a while. For example, when I'm using, I don't exit and re-create when switching projects. I just use MRU and I want to use phpactor to autocomplete any opened files (whatever if neovim was start in my $HOME or other directory).
If the detection fails for a project, a simple condition could be happen to the script detection in order to manage this case. I approve this PR because it's all I need right now to use this ncm plugin. It's not really different from my #2 , but this is more powerful and comprehensive.
I agree with @roxma to handle the project root detection method within the phpactor project (to also handle omnicompletion). This would be awesome !
In what case?
- If there is no composer, we traverse to the root of the FS, which is weird, a refactoring tool that does dangerous things shouldn't be in the root of your filesystem.
- If you are in a
vendorwithin avendor(i.e. you installed your deps in one of your dependencies for development), then you are not in the root of your project. - ... ?
You could take the parent .git root repository at this moment ?
You could take the parent .git root repository at this moment ?
More edge cases - what if the project doesn't use git? git submodules? (eventually Phpactor won't care which VCS you use, or even if you don't use one at all).
If there is no composer, we traverse to the root of the FS, which is weird, a refactoring tool that does dangerous things shouldn't be in the root of your filesystem.
I get it. So it shouldn't be auto detected for phpactor.
If you are in a vendor within a vendor (i.e. you installed your deps in one of your dependencies for development), then you are not in the root of your project.
If you're in a sub project, it doesn't matter if you're editting sub-project's files and treating it as a project root.
So it's safe for code completion usage, but unsafe for refactoring usage.
But split them will break consistency between plugins.
If you're in a sub project, it doesn't matter if you're editting sub-project's files and treating it as a project root.
I'm not sure -- I think @cmizzi mentioned he was using an auto chdir plugin. So if editing a file in a dependency you would only get completion for that depdency, not the whole project (great for decoupling, but not realistic).
I agree with @dantleech for this point of view. But, the solution could simply be to retrieve until latest found vendor or composer.json file. A loop to the root of the filesystem, for a plugin is not bad at all if the phpactor is not called on this path.
hmm
I'm more concerned about the security issue mentioned by @dantleech
If there's no better solution. I will merge #2 instead, for security and consistency
If there is no composer, we traverse to the root of the FS, which is weird, a refactoring tool that does dangerous things shouldn't be in the root of your filesystem.
Actually current PR doesn't traverse to the root of the FS. It fallbacks to getcwd() if composer is not found.
Further more, using a static var g:phpactorInitialCwd doesn't seem better than sticking with getcwd().
It's very common for vim users to :cd around. Keeping it consistent with vim's getcwd() is more straitforward and less confusing.
There're several vim plugins created for :cd around, e.g. vim-project