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

Include local and global installation instructions in README.md. #639

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

LeviHarman
Copy link

This pull request alters the installation section of the readme to include the installation instructions found at emacs-lsp/lsp-php

I found that guide helpful. Hopefully it helps others.

@codecov
Copy link

codecov bot commented Apr 26, 2018

Codecov Report

Merging #639 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #639   +/-   ##
=========================================
  Coverage     81.52%   81.52%           
  Complexity      912      912           
=========================================
  Files            62       62           
  Lines          2078     2078           
=========================================
  Hits           1694     1694           
  Misses          384      384

README.md Outdated
```
{
"minimum-stability": "dev",
"prefer-stable": true
Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Okay I changed it. I am unsure at the moment if I need the following line.

After editing your composer.json, you can install felixfbecker/php-language-server.
composer global require felixfbecker/language-server

I can check if the line above is needed but it will have to be tomorrow. I'm about to turn off the computer for the day.

Copy link
Author

Choose a reason for hiding this comment

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

I checked the lines in question above they are needed.

@felixfbecker
Copy link
Owner

Side question, what is the use case for installing globally?

@LeviHarman
Copy link
Author

I haven't done it myself. I included it because it was in emacs-lsp/lsp-php readme. I'll ask the creator of the README and see why he included it.

@rchl
Copy link

rchl commented Sep 29, 2018

Would appreciate this getting merged too as current instructions in the readme did not work for me and I wasn't sure why as I don't know composer.

I got this error with default instructions:

% composer require felixfbecker/language-server
Using version ^5.4 for felixfbecker/language-server
./composer.json has been created
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - felixfbecker/language-server v5.4.2 requires jetbrains/phpstorm-stubs dev-master -> satisfiable by jetbrains/phpstorm-stubs[dev-master] but these conflict with your requirements or minimum-stability.
    - felixfbecker/language-server v5.4.1 requires jetbrains/phpstorm-stubs dev-master -> satisfiable by jetbrains/phpstorm-stubs[dev-master] but these conflict with your requirements or minimum-stability.
    - felixfbecker/language-server v5.4.0 requires jetbrains/phpstorm-stubs dev-master -> satisfiable by jetbrains/phpstorm-stubs[dev-master] but these conflict with your requirements or minimum-stability.
    - Installation request for felixfbecker/language-server ^5.4 -> satisfiable by felixfbecker/language-server[v5.4.0, v5.4.1, v5.4.2].


Installation failed, deleting ./composer.json.

@rchl
Copy link

rchl commented Sep 29, 2018

Well, to be fair these instructions didn't help either. Still getting same error.

But personally I think installing globally makes sense as this is the kind of package that IMO should be installed globally in user's directory rather than in specific project as it is generally consumed by text editors.

@rchl
Copy link

rchl commented Sep 29, 2018

I have found #611 and managed to fix my issue.

Problem for me was that I already had ~/.composer directory so running composer global require felixfbecker/language-server used composer.json from that directory instead of global one from ~/.config/composer/composer.json.

EDIT: But then after deleting ~/.composer, running composer global require felixfbecker/language-server still didn't user configuration file from ~/.config/composer/composer.json so I had to actually put these configuration options in ~/.composer/composer.json.

The recommended installation method is through [Composer](https://getcomposer.org/). Follow the following installation instructions for local or global installation after installing composer on your system.

### Local Installation
Create a directory for php-language-server. Create a composer.json file in it, with the following contents:
Copy link
Owner

Choose a reason for hiding this comment

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

The idea for local installation is that your add this to the composer.json file of your project, where the project is something like https://github.com/felixfbecker/vscode-php-intellisense or some other editor integration or use case for the language server. The directory shouldn't be considered "the directory for php-language-server" because that will be ./vendor/felixfbecker/language-server

Copy link

Choose a reason for hiding this comment

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

I believe the idea here is that some users will want to try this out without making any changes to their global composer config, and these instructions provide a way of creating a local (perhaps temporary) instance of the language server without touching anything else.

The text editor can then be pointed to the local installation for testing. If it all pans out, the user might move to a global install (or not, as the case may be).

So "local" installation here can be thought of as a self-contained installation, rather than integrating it into another project like vscode-php-intellisense. Perhaps that should be covered as a third case, though?

Copy link

Choose a reason for hiding this comment

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

An example of why users might want a separate/self-contained installation is so that they can patch the language server for specific use-cases without affecting their global installation. For instance, modifying the hard-coded set of filename extensions which should be treated as PHP files in a particular project type -- a need that I personally ran into yesterday.

```

### Global installation
Before installing php-language-server, make sure your ~/.config/composer/composer.json includes the lines below. The settings apply to all globally installed Composer packages, so proceed with caution. If you do not want to edit your global Composer configuration, see the section for local installation above.

Choose a reason for hiding this comment

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

At least on my system (Mac with homebrew installed php and composer), the global config is in ~/.composer/composer.json rather than ~/.config/composer/composer.json.

Copy link

Choose a reason for hiding this comment

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

More generally, use $COMPOSER_HOME if it's set. Refer to https://getcomposer.org/doc/03-cli.md#composer-home

If it's not set, note that a hard-coded fallback could be error-prone, as the default value is quite variable depending on the OS (and version thereof) you're running.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants