-
Notifications
You must be signed in to change notification settings - Fork 374
[XDebug Bridge] Highlight syntax of php scripts from mime type in Devtools #2566
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
b7d38ef to
390ca9f
Compare
|
I could suggest a pull request to the chrome devtools team with these modifications : diff --git a/front_end/core/sdk/Script.ts b/front_end/core/sdk/Script.ts
index 7bae8f9428..7226993a5d 100644
--- a/front_end/core/sdk/Script.ts
+++ b/front_end/core/sdk/Script.ts
@@ -84,6 +84,7 @@ export class Script implements TextUtils.ContentProvider.ContentProvider, FrameA
originStackTrace: Protocol.Runtime.StackTrace|null;
readonly #codeOffsetInternal: number|null;
readonly #language: string|null;
+ readonly #mime: string|null;
#contentPromise: Promise<TextUtils.ContentData.ContentDataOrError>|null;
readonly #embedderNameInternal: Platform.DevToolsPath.UrlString|null;
readonly isModule: boolean|null;
@@ -93,7 +94,7 @@ export class Script implements TextUtils.ContentProvider.ContentProvider, FrameA
startLine: number, startColumn: number, endLine: number, endColumn: number, executionContextId: number,
hash: string, isContentScript: boolean, isLiveEdit: boolean, sourceMapURL: string|undefined,
hasSourceURL: boolean, length: number, isModule: boolean|null, originStackTrace: Protocol.Runtime.StackTrace|null,
- codeOffset: number|null, scriptLanguage: string|null, debugSymbols: Protocol.Debugger.DebugSymbols|null,
+ codeOffset: number|null, scriptLanguage: string|null, mimeType: string|null, debugSymbols: Protocol.Debugger.DebugSymbols|null,
embedderName: Platform.DevToolsPath.UrlString|null, buildId: string|null) {
this.debuggerModel = debuggerModel;
this.scriptId = scriptId;
@@ -116,6 +117,7 @@ export class Script implements TextUtils.ContentProvider.ContentProvider, FrameA
this.originStackTrace = originStackTrace;
this.#codeOffsetInternal = codeOffset;
this.#language = scriptLanguage;
+ this.#mime = mimeType;
this.#contentPromise = null;
this.#embedderNameInternal = embedderName;
}
@@ -167,6 +169,10 @@ export class Script implements TextUtils.ContentProvider.ContentProvider, FrameA
return this.#language;
}
+ mimeType(): string {
+ return this.#mime || (this.isWasm() ? 'application/wasm' : 'text/javascript');
+ }
+
executionContext(): ExecutionContext|null {
return this.debuggerModel.runtimeModel().executionContext(this.executionContextId);
}
diff --git a/front_end/models/bindings/ResourceScriptMapping.ts b/front_end/models/bindings/ResourceScriptMapping.ts
index b6e539d089..7b2cb82310 100644
--- a/front_end/models/bindings/ResourceScriptMapping.ts
+++ b/front_end/models/bindings/ResourceScriptMapping.ts
@@ -232,7 +232,7 @@ export class ResourceScriptMapping implements DebuggerSourceMapping {
this.#uiSourceCodeToScriptFile.set(uiSourceCode, scriptFile);
this.#scriptToUISourceCode.set(script, uiSourceCode);
- const mimeType = script.isWasm() ? 'application/wasm' : 'text/javascript';
+ const mimeType = script.mimeType();
project.addUISourceCodeWithProvider(uiSourceCode, originalContentProvider, metadata, mimeType);
void this.debuggerWorkspaceBinding.updateLocations(script);
}This might be a bit presumptuous of me, since they have no interest in open-sourcing anything other than the Wasm and JS files. Maybe Typescript well? |
Let's try that, who knows! Also CC @ThomasTheDane – we're trying to syntax highlight PHP files in Chrome Devtools. Would you have any interest in a patch such as above to make the source mimeType configurable? |
|
Great! My suggested code change for the DevTools front-end has already been approved by one reviewer! 🎉 Just one more vote to go! |
|
One reviewer disapproved, asking for modifications, but the error still appear and his suggestions were incorrect. We'll see. |
|
It doesn't look good. He suggests a way that I can't implement. And it duplicates files again [ like when debugging Typescript files ]. So we have a version with step debugging but code is black and we have syntax highlighting on a file we can't interact with... |
|
The last response in that discussion is:
@mho22 so, are you saying that the I've just tried loading a
Now, that's on a regular Alternatively, we could run a patched version of Chrome devtools-frontend as a standalone app – which would also help us in Playground web. But that's a much larger project. |
|
@adamziel Yes! That's what I am trying to do, but It doesn't look good. I am trying source mapping but :
This is my current script : for (const [bridgeUri, scriptId] of this.scriptIdByUrl.entries()) {
const cdpUri = this.uriFromBridgeToCDP(bridgeUri);
const phpContent = await this.readPHPFile(bridgeUri);
const phpLines = phpContent.split('\n');
const mappings = phpLines.map(() => 'A').join(';');
// Minimal PHP source map
const sourceMap = {
version: 3,
sources: [cdpUri],
sourcesContent: [phpContent],
mappings
};
// Serialize it into a data URI so DevTools can load it
const sourceMapDataUri =
'data:application/json;charset=utf-8;base64,' +
Buffer.from(JSON.stringify(sourceMap)).toString('base64');
// Load script
this.cdp.sendMessage({
method: 'Debugger.scriptParsed',
params: {
scriptId,
url: cdpUri,
startLine: 0,
endLine: phpLines.length,
startColumn: 0,
endColumn: 0,
executionContextId: 1,
isLiveEdit: false,
sourceMapURL: sourceMapDataUri,
hasSourceURL: false,
length: phpContent.length
},
});
}I am missing something... Maybe my mapping? The suggestion will create duplicates in the Devtools file tree, while my code change in Devtools only enable syntax highlight in files other than
Yes, that's a big feature 😄 |
|
Thank you @adamziel ! I will study your code. |
2fb2ee0 to
47d921f
Compare
|
In the end, it was easy to implement but some tricks had to be done. Like the
@adamziel @brandonpayton What do you think? |
|
I should add a test. |
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.
LGTM overall, let's just brush up the PR description and some inline notes. Great work @mho22!





Motivation for the change, related issues
This pull request aims to apply syntax highlighting to debuggable PHP code using the Xdebug Bridge and Chrome Devtools.
This comes with a downside. To enable syntax highlighting to Debugger's parsed scripts, we also import a copy of that file that will act as the given script. Adding two identical files in the Source panel File tree.
Testing Instructions
First, run the Bridge
Go to Devtools and add a breakpoint.
Then, run the CLI command
It will break in the script, not the resource.