search

AtomLinter / linter-phpmd

Atom linter plugin for php, using phpmd.
21 stars 4 forks source link

pear vs. composer #37

Open till opened 8 years ago

till commented 8 years ago

I think global install is nice and it can also be done with composer, but supporting a local install via composer would also be nice. We frequently pin phpmd to specific versions for a project.

It seems like you would have to probe for a local composer.phar or global (through $PATH) and then run the following:

$ ./composer.phar config bin-dir
vendor/bin

Now the path to a local phpmd would be vendor/bin/phpmd.

Generally, any thoughts on supporting this? I have never written an atom-plugin, but I suppose I'd give it a shot and try to make a PR for this. Let me know!

Arcanemagus commented 8 years ago

It's certainly possible, but can be quite the pain to get right. If you want to put together a PR I would be happy to review it, otherwise I'll leave this issue open for when I have some time to implement it. As I wouldn't use this personally though it might take a while before I get to it.

mdeboer commented 7 years ago

I can't think of a reason why you'd want to include composer.phar in your project (possibly repository) unless you really, really need a specific version of composer.

For that reason I think it's a lot easier to just check if composer is in the $PATH and add a setting in Atom to override the composer path, much like is done with the PHPMD binary now.

Replacing the variable executablePath on line 66 of lib/main.js with a function that checks and returns the local PHPMD binary if found and defaults to executablePath, would probably work.

Can't make any promises when or if, but I'd be happy to take a look and submit a PR.

Arcanemagus commented 7 years ago

For that reason I think it's a lot easier to just check if composer is in the $PATH and add a setting in Atom to override the composer path, much like is done with the PHPMD binary now.

I'm not sure what you mean here, simply leaving the executablePath setting as composer already accomplishes this.

Replacing the variable executablePath on line 66 of lib/main.js with a function that checks and returns the local PHPMD binary if found and defaults to executablePath, would probably work.

That's the gist of what needs to be done here, helpers.findAsync is probably going to be helpful for this. The tricky part is figuring out all the crazy variants on what constitutes "local path".

till commented 7 years ago

Some background: We've been using composer since early 2012. And we still include the phar because we enjoy reproducible installs and so on. It's the same version on CI, dev, staging and for production builds. Add to that, a version is managed by the VCS and it becomes obvious when and how it's updated.

Having said this, my question was about a local phpmd install. If that is solved, then we can also close this. ;)

mdeboer commented 7 years ago

Does the recent v2.0 release fix this for you? It includes the option to automatically search for phpmd binaries.