Skip to content

Make PHP LSP bridge robust: filter noise + buffer JSON output #71

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.

Sign up for GitHub

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

Open
jan-dh wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make PHP LSP bridge robust: filter noise + buffer JSON output #71

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions packages/language-server/src/utils/exec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@ export const exec = async (command: string, args: string[], options: SpawnOption
let stdout = '';
let stderr = '';

child.stdout.on('data', (data) => stdout += data);
child.stderr.on('data', (data) => stderr += data);
child.stdout.on('data', (data) => {stdout += data.toString();});
child.stderr.on('data', (data) => {stderr += data.toString();});

child.on('close', (code) => resolve({ stdout, stderr, code }));
child.on('close', (code) => {
const jsonStart = stdout.search(/[{[]/);
Copy link
Owner

@moetelo moetelo Dec 11, 2025

Choose a reason for hiding this comment

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

I don't really want to rely in trying to find [ or { in the stdout. Deprecation notices possibly could use these characters to format their messages, like [Deprecated]. I see that it's not the case but this solution does not seem that robust to me.

In continuation of this comment: #69 (comment)

I was expecting all the warnings go to stderr, and the rest to stdout. Now I see that running php bin/console depends on the user environment (php.ini).

According to the PHP runtime documentation, we can use the ini option display_errors to make the php executable do the expected thing.
https://www.php.net/manual/en/errorfunc.configuration.php#ini.display-errors

Value "stderr" sends the errors to stderr instead of stdout.

And we can pass ini option as an argument to php using -d param:
php -d display_errors=stderr bin/console debug:twig --format json
(src: https://www.php.net/manual/en/features.commandline.options.php)

Now, if we change this:

twiggy/packages/language-server/src/phpInterop/PhpExecutor.ts

Lines 21 to 26 in c65e81f

const result = await exec(this._phpExecutable, [
command,
...args,
], {
cwd: this._workspaceDirectory
});

to this:

const result = await exec(this._phpExecutable, [
    '-d',
    'display_errors=stderr',
    command,
    ...args,
], {
    cwd: this._workspaceDirectory
});

that would hopefully to fix the issue.

if (jsonStart > 0) {
stdout = stdout.slice(jsonStart);
}
resolve({ stdout, stderr, code });
});
});
};

Expand Down