xdebug / vscode-php-debug

PHP Debug Adapter for Visual Studio Code 🐞⛔
MIT License
771 stars 176 forks source link

Cannot launch with externalConsole: true on macOS #760

Open puremourning opened 2 years ago

puremourning commented 2 years ago

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: https://github.com/puremourning/vimspector/issues/539#issuecomment-1057127464

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 commented 2 years ago

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 commented 2 years ago

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

zobo commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 months ago

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.