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

phpconfig.json #25

Closed
lgabeskiria opened this issue Sep 13, 2016 · 26 comments
Closed

phpconfig.json #25

lgabeskiria opened this issue Sep 13, 2016 · 26 comments

Comments

@lgabeskiria
Copy link
Contributor

We need to create phpconfig.json file with some configurable options (php target version, autloload.php path, etc...) per project.

@felixfbecker
Copy link
Owner

Don't really want to clutter the user's project with even more config files - can we just use workspace .vscode/settings.json for this? The built-in PHP linter for example already has the setting php.executablePath, we could offer more options. composer.json should be in workspace root, and autoloader in vendor/autoload.php

@felixfbecker
Copy link
Owner

That of course assumes the user is using VS Code, sorry. It's still the question whether this configuration should be made by the extension that is using the language server, for example the executable path needs to be known beforehand because the language server needs to be spawned with it. Do you have some more examples what should go into the config file?

@lgabeskiria
Copy link
Contributor Author

for example files/folders that should be excluded/included from indexing.
for wordpress developers we can specify files/folders (wp-load.php so we can follow require statements and give them intellisense) that should be indexed on initialization.

@felixfbecker
Copy link
Owner

felixfbecker commented Sep 13, 2016

I outlined some of my ideas for indexing in #9.
I never worked with Wordpress but it should be supported. Is it not using composer autoloading? If you use composer, then you can add files to autoload.files in composer.json to include some global functions that cannot be autoloaded through a class loader, and those would then of course get indexed.

We also can get configuration through the protocol: https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#didchangeconfiguration-notification

I like that because we don't need a config file and the IDE can decide where to save the config and how to present it. For example, in VS Code you can have global and workspace config that overrides the global one. Other IDEs might never use a JSON file and only present a GUI. All they have to do is send the config to the server.

@lgabeskiria
Copy link
Contributor Author

no, wordpress is not using composer

@felixfbecker
Copy link
Owner

I have no idea how we can reflect on the wordpress autoloading, so I cannot make any proposal here.
What do you think about providing the config through the protocol?

@lgabeskiria
Copy link
Contributor Author

I need some time to work on proposal.

@kaloyan-raev
Copy link
Contributor

My understanding of the language server protocol is that the user does not work with the server directly, but with the client.

Hence we should not introduce new configuration files that are specific to the server. IDEs have their own specific configuration files. It's their job to convert them to a didChangeConfiguration notification.

Instead of introducing a phpconfig.json file, we should specify how the DidChangeConfigurationParams settings object should look like.

@lgabeskiria
Copy link
Contributor Author

In vscode there are files for typescript and javascript (tsconfig.json, jsconfig.json) projects. There you set settings for your project (include/exclude files, target language version etc...). I think that it's reasonable to introduce phpconfig.json file.

@felixfbecker
Copy link
Owner

To be fair, tsconfig.json is not for the language server really. It primarily is a configuration file for the TypeScript compiler, and the language server then makes use of these compiler settings aswell. You cannot define any "indexing" settings there (for example, you can specify to exclude node_modules, but that is for the compiler, the language server will still give you IntelliSense from definition files in node_modules). jsconfig.json is just a tsconfig.json for JavaScript, since the TypeScript language server also powers JavaScript IntelliSense. The need for it has been often criticized.

I am not sure but I don't think other language servers (C#, PowerShell, Go) require a config file. Is there any benefit for having a config file over using the protocol for configuration, in a PHP context?

@lgabeskiria
Copy link
Contributor Author

c# has project.json file.

@felixfbecker
Copy link
Owner

felixfbecker commented Sep 13, 2016

But is it only for language server configuration? Afaik it is there for all kinds of compiler settings

@Ciantic
Copy link

Ciantic commented Oct 6, 2016

WordPress does not require autoloading, it's basically big library of global functions and Classes.

The language server needs a way to add external folder in the include path recursively. It's not just WordPress but also big plugins like WooCommerce that need to be indexed same way, as a library of globals.

In case of WP, the language server should not try to follow include / requires (because they are a mess, and sometimes dynamic), instead it should just include all functions and classes it finds in a global namespace.

@felixfbecker
Copy link
Owner

@Ciantic You seem to be a bit familiar with WP, I would appreciate some insights. I only worked with Composer so far, which is my plan to support first as it builds on the standards PSR-0 and PSR-4. Composer also has the ability to always include specific files with the files property in composer.json. How does Wordpress do this? Given an unknown class, how does it find the source file? Does it basically include all PHP files in the project folder?

@Ciantic
Copy link

Ciantic commented Oct 6, 2016

@felixfbecker yes, everything. When one develops plugin or theme, it's assumed all functions WP provides is available.

And this is how Eclipse PDT also does the include paths. It does not have "include resolving mechanism" it just basically adds all functions and classes in all files in a given folder recursively to an index.

@felixfbecker
Copy link
Owner

Closing this as all config should happen through requests.

@soderlind
Copy link

WordPress is available using composer: https://packagist.org/packages/johnpbloch/wordpress

.. but I have no idea how ~/.vscode/extensions/felixfbecker.php-intellisense-0.0.11/vendor/composer/autoload_files.php .. or composer works

@felixfbecker
Copy link
Owner

We implemented an approach that simply indexes all definitions in your projects and does not impose any requirements on autoloading. Your files only have to be inside your project folder.

@soderlind
Copy link

ok. but I'm doing a lot of WordPress development in different projects. Would I have to install the WordPress core in all the projects ? (would be nice if I could point to one external folder)

@lgabeskiria
Copy link
Contributor Author

@soderlind That's exactly what phpstorm does

@mniewrzal
Copy link
Contributor

As an workaround you can create link to WordPress core in your projects.

@felixfbecker
Copy link
Owner

Yep, a symlink should work 👍

@Ciantic
Copy link

Ciantic commented Oct 29, 2016

For me the symlink doesn't work, it screws up my auto-sync workflow, also I can't use Eclipse afterwards where my multiple projects still are.

Instead I have hacked the C:\Users\username\.vscode\extensions\felixfbecker.php-intellisense-0.0.11\vendor\felixfbecker\language-server\src\LanguageServer.php with this:

    /**
     * Parses workspace files, one at a time.
     *
     * @return void
     */
    private function indexProject()
    {
        $fileList = findFilesRecursive($this->rootPath, '/^.+\.php$/i');
        $fileList = array_merge($fileList, findFilesRecursive("C:\\DepSource\\WordPress", '/^.+\.php$/i'));
        $numTotalFiles = count($fileList);

        $startTime = microtime(true);
        $fileNum = 0;

@felixfbecker
Copy link
Owner

Could you elaborate a bit more why symlinks don't work? Does indexing work with symlinks? What do you mean with "auto-sync" workflow? You can add the symlink to .gitignore or .git/info/exclude. What is Eclipse's problem?

@Ciantic
Copy link

Ciantic commented Oct 29, 2016

I'm using WinSCP to synchronize. I don't want the WP to appear in eclipse...

@felixfbecker
Copy link
Owner

Oh dear god, you don't use version control?

You can disable following symlinks in WinSCP's settings:
image

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

No branches or pull requests

6 participants