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

Cannot launch with externalConsole: true on macOS #760

Open
puremourning opened this issue Mar 2, 2022 · 8 comments · Fixed by #762
Open

Cannot launch with externalConsole: true on macOS #760

puremourning opened this issue Mar 2, 2022 · 8 comments · Fixed by #762

Comments

@puremourning
Copy link

PHP version: 8.1.2
Xdebug version: 3.1.3
VS Code extension version: 1.23.0

Your launch.json (actually .vimspector.json, but essentially contains the same info):

{
  "configurations": {
    "Run current scipt": {
      "adapter": "vscode-php-debug",
      "configuration": {
        "request": "launch",
        "program": "${file}",
        "args": [],
        "stopOnEntry": false,
        "externalConsole": true
      }
    }
  }
}

Xdebug php.ini config/etc all here: puremourning/vimspector#539 (comment)

I'm not using vscode, but another DAP client, vimspector.


When launching on macOS using the debug adapter directly, when externalConsole is true, the launch fails with:

osascript: /Users/ben/.vim/vimspector-conf/gadgets/macos/download/vscode-php-debug/v1.24.3/root/extension/out/terminalHelper.scpt

I have checked the .vsix does not contain this file, though it is in the repo.
I have checked the extension code and it is assuming the file exists in the same dir as the js files.

I think this is just a packaging error, and the terminalHelpler.scpt is just not being included in the extension bundle. Copying the one from this repo into that dir does indeed work and fixes the issue.

On a side note, I think there's another issue that the script is called TerminalHelper.scpt and the code looks for terminalHelper.scpt. Most Mac users will have case-insensitive filesystem, but at least some will have selected case-sensitive I think, so may be worth correcting that.

@puremourning
Copy link
Author

DAP trace, if it makes any difference:

2022-03-02 16:44:15,524 - DEBUG - Sending Message: {"command": "initialize", "arguments": {"adapterID": "php-debug", "clientID": "vimspector", "clientName": "vimspector", "linesStartAt1": true, "columnsStartAt1": true, "locale": "en_GB", "pathFormat": "path", "supportsVariableType": true, "supportsVariablePaging": false, "supportsRunInTerminalRequest": true, "supportsMemoryReferences": true}, "seq": 0, "type": "request"}
2022-03-02 16:44:15,598 - DEBUG - Message received: {'seq': 1, 'type': 'response', 'request_seq': 0, 'command': 'initialize', 'success': True, 'body': {'supportsConfigurationDoneRequest': True, 'supportsEvaluateForHovers': True, 'supportsConditionalBreakpoints': True, 'supportsFunctionBreakpoints': True, 'supportsLogPoints': True, 'supportsHitConditionalBreakpoints': True, 'supportsSetVariable': True, 'exceptionBreakpointFilters': [{'filter': 'Notice', 'label': 'Notices'}, {'filter': 'Warning', 'label': 'Warnings'}, {'filter': 'Error', 'label': 'Errors'}, {'filter': 'Exception', 'label': 'Exceptions'}, {'filter': '*', 'label': 'Everything'}], 'supportTerminateDebuggee': True}}
2022-03-02 16:44:15,598 - DEBUG - Sending Message: {"command": "launch", "arguments": {"request": "launch", "program": "/Users/ben/.vim/bundle/vimspector/support/test/php/test.php", "args": [], "stopOnEntry": false, "externalConsole": true, "name": "test"}, "seq": 1, "type": "request"}
2022-03-02 16:44:15,613 - DEBUG - Message received: {'seq': 2, 'type': 'response', 'request_seq': 1, 'command': 'launch', 'success': False, 'message': 'osascript: /Users/ben/.vim/vimspector-conf/gadgets/macos/download/vscode-php-debug/v1.24.3/root/extension/out/terminalHelper.scpt: No such file or directory\n', 'body': {'error': {'id': 0, 'format': 'osascript: /Users/ben/.vim/vimspector-conf/gadgets/macos/download/vscode-php-debug/v1.24.3/root/extension/out/terminalHelper.scpt: No such file or directory\n', 'showUser': True}}}

@zobo
Copy link
Contributor

zobo commented Mar 2, 2022

Hi! Thanks for the report. I have access to a mac and will test it out.

@zobo
Copy link
Contributor

zobo commented Mar 2, 2022

I'm revisiting this issue now. I'll probably just include the missing file, but I want to understand it better.
There is also #60 and #67 that deal with related issues. Passing process and terminal creation to the client/IDE.
As far as I can tell vimspetor supports https://microsoft.github.io/debug-adapter-protocol/specification#Reverse_Requests_RunInTerminal but does not handle "kind" parameter.

@puremourning
Copy link
Author

Yes you are right, vimspector does indeed support the runInTerminal request, but does not obey the 'kind' parameter, principally for the exact reason that it's tricky to launch an 'external' terminal and most vim users won't want that behaviour in practice. FWIW I only raised this as I noticed it while testing something quickly; it's not a high priority :)

If you decide that the best thing for vscode-php-debug is to send runInTerminal request rather than trying to spawn the external terminal yourself, I would agree that's a solid decision. I think vimspector users would appreciate it too because they get a better UX (runInTerminal uses a real TTY rather than just a log buffer) even if they don't get the 'external' terminal.

@zobo
Copy link
Contributor

zobo commented Mar 2, 2022

I think this is a better way to go...

There is one problem with the runInTerminal. There is currently no DAP message that would indicate that the process has completed.
So if Xdebug is miss-configured, the debugger would say in the "listening" state and the user would need to manually trigger the "stop" command (disconnectRequest).
But I think other DA implementations solve this with a timeout setting.

I'm considering adding a console option and having internalConsole as default. (See https://code.visualstudio.com/docs/editor/debugging#_launchjson-attributes).

Also, the externalConsole was broken for quite a while, I really doubt anyone was using it, so I might consider breaking config backward compatibility and just switch to console...

I'll think about it.

@puremourning
Copy link
Author

Also, the externalConsole was broken for quite a while, I really doubt anyone was using it, so I might consider breaking config backward compatibility and just switch to console...

Yeah I thought that too, though it was only broken on macOS which I guess is not a high usage environment for PHP in practice?

@zobo
Copy link
Contributor

zobo commented Mar 2, 2022

I think there is not much usage of the "run currently open script" and similar... I wanted to invest some time into some basic telemetry for some time. That would give some insight what's important and what problems people are dealing with.

@zobo
Copy link
Contributor

zobo commented Jun 28, 2024

Note for myself. When using runInTerminal the js debugger is using a poll mechanism to see if the process is alive https://github.com/microsoft/vscode-js-debug/blob/a710e44c8391e70c179279f54e9fc12d81452945/src/targets/node/program.ts#L190

I have to keep the current "external terminal" implementation and the one with runInTerminal for clients that do not support it.

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

Successfully merging a pull request may close this issue.

2 participants