-
Notifications
You must be signed in to change notification settings - Fork 365
[XDebug Bridge] Load files in Devtools before running PHP with Xdebug enabled #2527
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
Conversation
…reakpoints when PHP script is run with Xdebug enabled
|
I think the user experience is cool. You load your files, you set breakpoints, you run your code with PHP runtime and Xdebug enabled and everytime you run your code it breaks on the lines you chose. Really handy. However, I noticed two visual caveats :
I was thinking of something : In PHP.wasm CLI, when we add the I also need to implement tests before setting it ready for review. |
This sounds like a good feature. I wonder an option like I think it may be clearer to use a name for the option that indicates "when". Some possibilities might be "breakOnLoad" or "breakOnStartup". |
brandonpayton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @mho22! This tests well for me and is very cool.
| const { url, lineNumber } = params; | ||
| const fileUri = url; | ||
| const file = this.uriFromCDPToBridge(url); | ||
| const uri = this.uriFromBridgeToDBGP(file); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When reading this method and its references to url, file, and uri, my working memory has to remember the intention behind each of the generic names in order to best understand the code. Could we try to reduce that cognitive load by using more specific variable names that mention their purpose?
Update:
After reading more code within this module, it is possible that the generic variable names are used throughout. If that is the case, maybe now is not the time to change them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I am juggling with url, uri, file, fileUri. I unfortunately need to use three types of "uris" :
- The filePath in the Node Filesystem to find the source code.
- The absolute path of the file because Xdebug works only with absolute paths
- The relative path indicated when starting the Bridge.
I will try to clean up this.
Highlighting also works for C/C++ when using the C/C++ DevTools Support (DWARF) Chrome extension: I've poked around the local extension code for Google's Wasm debugging extension and so far haven't seen anything special in the extension for adding syntax highlighting. Maybe Google just added support for syntax highlighting those file types as part of Chrome. :-/ I also experimented with providing a different URL with the |
@mho22 It seems like syntax-highlighting based on MIME type ought to work if my scanning of the Chromium devtools source is correct. There is a case to select the PHP language in the CodeHighlighter.ts based on the They even have a test for it: Maybe there is still hope that we can load and syntax-highlight PHP files if we can effectively relay the MIME type to devtools. |
|
@brandonpayton Thank you for your investigation! I actually also found these informations and I was looking for a protected async getLanguageSupport(content: string|CodeMirror.Text): Promise<CodeMirror.Extension> {
// This is a pretty horrible work-around for webpack-based Vue2 setups. See
// https://crbug.com/1416562 for the full story behind this.
let {contentType} = this;
if (contentType === 'text/x.vue') {
content = typeof content === 'string' ? content : content.sliceString(0);
if (!content.trimStart().startsWith('<')) {
contentType = 'text/javascript';
}
}
const languageDesc = await CodeHighlighter.CodeHighlighter.languageFromMIME(contentType);
if (!languageDesc) {
return [];
}
return [
languageDesc,
CodeMirror.javascript.javascriptLanguage.data.of({autocomplete: CodeMirror.completeAnyWord}),
];
}I first thought, as a legacy file, that it won't be related to the Source panel. I was wrong. But the current way to send our script source to the Devtools tool was by using the // Bind UISourceCode to scripts.
const scriptFile = new ResourceScriptFile(this, uiSourceCode, script);
this.#uiSourceCodeToScriptFile.set(uiSourceCode, scriptFile);
this.#scriptToUISourceCode.set(script, uiSourceCode);
const mimeType = script.isWasm() ? 'application/wasm' : 'text/javascript';
project.addUISourceCodeWithProvider(uiSourceCode, originalContentProvider, metadata, mimeType);
void this.debuggerWorkspaceBinding.updateLocations(script);And set the mimeType as I tried one last thing by using something different than
My current code implementation is a complete mess. I need to clean it up before pushing. I am still not sure if everything will work like with |
This is great, @mho22! What did you end up using instead? I'm very interested. 🍿 |
|
I used a serie of events to replace And in order to replace I have a lot of work to do to find out if all these steps are necessary, if the step debugging still works and how I can simplify it the most but seeing the colored code was delightful. However, this will be a separate pull request. |
|
I still need to do the following:
|
| entry: { | ||
| source: 'other', | ||
| level: 'info', | ||
| text: '🎉 Welcome to WordPress Playground DevTools! 🎉\n ‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾\n\n1. Add breakpoints in your files to start step debugging.\n\n2. Run your php file, project, plugin or theme using PHP.wasm or the Playground.\n\n3. Witness the magic break.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's explain how to run them. Inline instructions would be convenient if we can commit to the command and avoid any BC-breakes. A link might also be useful. CC @fellyph to coordinate a doc page with examples.
Also – "the Playground" makes it seem like playground.wordpress.net, if that's about Playground CLI, let's say it explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More I am thinking about this more I find ways to run a php code with Xdebug and the wordpress-playground materials. So, I have two suggestions :
- We could either add a link but we could implement it in another pull request in coordination with @fellyph. Something like this :
- I could add the smallest possible way to run Xdebug to not fill the console. Something like this :
But as we cannot pretty print the logs in the console.log. This makes it not easy to read. I would personally prefer the first suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Combining both would be nice, as in "Here's how to run it: (command). For more details go here:". That way you can still do something when you're offline.
Here's a weird, alternative idea – we could ship the documentation offline with the CLI package. It could either be the entire docsite, which would be large, or just the markdown files – potentially with a man-like CLI reader. Then we could reference the local resource. But that's way beyond the scope of this PR. cc @fellyph and @dmsnell for thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not a blocker here by the way, we can follow up with an improvement
adamziel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is seriously cool and super useful! I left a few nitpicks and I think this is good to go.
Let's just update the PR description and document the commands needed to use it outside of the Playground repo (with npx). They will come handy once it's time to write a doc page and a blog post about this feature.
🎉
On it. |
|
@adamziel I think we are good here if you agree with my suggestion to create a proper documentation in another pull request. |
|
Looks good overall and solves an important problem that gets us closer to dependency-less usage in Playground CLI – let's get it in! Thank you @mho22! |



Motivation for the change, related issues
Based on the following pull request :
--auto-mountoption enabled #2442The first approach was to load Devtools when running a file with Xdebug enabled PHP.wasm. Unfortunately, the user experience was not great. e.g. files were loaded only when executed.
The new approach opens de Devtools Source panel with the relevant loaded files ready to be manipulated.
Once the breakpoints are set, the PHP file can be executed and paused with Xdebug enabled PHP.wasm.
Implementation details
breakOnFirstLineOption : Allows breaking on the first executed line when no breakpoints exist.Testing Instructions with Xdebug Bridge
Testing Instructions with PHP.wasm CLI
Testing Instructions with PHP.wasm Node
Testing Instructions in
wordpress-playground