Skip to content

Conversation

mho22
Copy link
Collaborator

@mho22 mho22 commented Jul 25, 2025

Motivation for the change, related issues

Based on this comment and pull request.

In Playground CLI, the first line where Xdebug stops in Devtools is the auto_prepend_file.php file located in the internal directory. This pull request aims to ignore what's in the directory by adding a excludedPaths option in the bridge that will be ignored when stacking or stepping.

Implementation details

Added an excludedPath option to the startBridge function. When specified, XdebugCDPBridge will check this option, and the DevTools will step_over any file matching the excluded path, skipping its parsing.

Testing Instructions

With script cli.ts :

import { runCLI } from "./packages/playground/cli/src/run-cli";

const script = `<?php

$test = 42;

echo "Output!\n";

function test() {
	echo "Hello Xdebug World!\n";
}

test();
`;

const cliServer = await runCLI({command: 'server', xdebug: true, experimentalDevtools: true});

cliServer.playground.writeFile('xdebug.php', script);

const result = await cliServer.playground.run({scriptPath: `xdebug.php`});

console.log(result.text);

Run command :

node --no-warnings --experimental-wasm-stack-switching --experimental-wasm-jspi --loader=./packages/meta/src/node-es-module-loader/loader.mts cli.ts

Before

before

After

after

@mho22 mho22 changed the title [Xdebug Bridge] Exclude file and directory paths from devtools stack and debugging [XDebug Bridge] Exclude file and directory paths from devtools stack and debugging Jul 25, 2025
@mho22 mho22 mentioned this pull request Jul 25, 2025
8 tasks
@mho22
Copy link
Collaborator Author

mho22 commented Aug 4, 2025

I will need to add tests before making it ready for review.

@mho22
Copy link
Collaborator Author

mho22 commented Aug 18, 2025

I added tests based on the tests from pull request #2409.

@mho22 mho22 marked this pull request as ready for review August 18, 2025 15:28
@brandonpayton brandonpayton requested a review from a team August 20, 2025 17:48
@adamziel
Copy link
Collaborator

There's a few conflicts to resolve and the implementation section of the description is missing (a sentence or two should suffice here) but other than that it LGTM. Let's fix those and merge – thank you @mho22!

@mho22 mho22 requested a review from adamziel August 26, 2025 12:43
},
});
if (this.isExcludedPath(fileUri)) {
this.sendDbgpCommand('step_over');
Copy link
Collaborator

@adamziel adamziel Aug 28, 2025

Choose a reason for hiding this comment

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

The excluded file could call a function defined in a non-excluded file. Would step_over ignore the entire call and never step into that other file? This isn't relevant for breaking on the first line but it is relevant for calls made by Playground-level mu-plugins, e.g. the SQLite integration plugin.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oho. I guess it will ignore the file. It should also check the command. If the command is step_into then, don't ignore the file. I'll need to test that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah I didn't understand your sentence correctly a few days ago! The excluded file won't be "loaded" by Devtools but its code will be run as expected. So if an excluded file is running a function from a non excluded file it will pause the script on the first line of the non-excluded file.

The opposite will happen though. If you run a script that calls another one that has an excluded path, you won't see anything from that excluded path script. That is why I understood with my last comment.

That is why I should let the current code change but I should add a second condition in the if statement indicating that if the current command is step_into and the file has an excluded path we should not step_over and show the script.

I am adding a new test for that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

Copy link
Collaborator

@adamziel adamziel Sep 2, 2025

Choose a reason for hiding this comment

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

@mho22 to make sure we're aligned, let's discuss different scenarios. Let's also create a test case for each of them.

Scenario 1 – basic exclusion

// In main.php:
require_once "excluded.php";
// Breakpoint is set on this line:
excluded_function();
boot_wordpress();

// In excluded.php:
function excluded_function() {
	// ... logic ...
}

In this scenario, stepping into excluded_function would be the same as stepping over it – we'd advance to boot_wordpress().

Scenario 2 – excluded file in a stack trace

// In main.php:
require_once "excluded.php";
function boot_wordpress() {
    // Breakpoint on the next line
    require "wp-load.php";
}

// Breakpoint is set on this line:
excluded_function();

// In excluded.php:
function excluded_function() {
	global $log;
	$log->info("Booting WordPress");
	boot_wordpress();
}

In this scenario, stepping into excluded_function(); would:

  • Skip the body of excluded_function()
  • Move the execution inside boot_wordpress() where we could continue stepping into or stepping over

Similarly, in this scenario:

// main.php
// breakpoint set here:
require_once "excluded.php";

// excluded.php:
require_once "my-app.php";

// my-app.php:
call1();
call2();

Stepping into the firs require_once would, ideally, move us into the first line of my-app.php.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll need some time to test your scenarios.

a2
) => {}) /* a dynamic function call to signature vii, but there are no exported function pointers with that signature, so this path should never be taken. Build with ASSERTIONS enabled to validate. */(
handle,
((a1, a2) => {})(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should php_8_3.js and other PHP JS files be shipped with this PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Something went wrong with the merge conflicts. Let's start again.

}),
]),
it('ignores files from excluded path except when stepping into manually', async () => {
const excludedPath = `${fixtures}/excluded-paths/`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use more intuitive names for those files. E.g. excluded-paths.php could be called something runs-excluded-script and then the excluded-paths directory could be called excluded-source-root or something like that.


let steps = 5;

await new Promise<void>((resolve) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I found it difficult to follow what this test is doing. Let's add some human-friendly annotations.

@mho22 mho22 marked this pull request as draft September 4, 2025 10:37
@mho22 mho22 force-pushed the exclude-paths-during-debugging-in-devtools branch from 66676d3 to b233abe Compare September 15, 2025 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants