Skip to content

AST-based breakpoint validation #317

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.

Sign up for GitHub

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

Open
chrisdp wants to merge 8 commits into
base: master
Choose a base branch
from
+700 −34
Open

AST-based breakpoint validation #317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1a03a78
first pass at failing known bad breakpoints based on files contents o…
chrisdp Mar 28, 2026
875a31d
some fixes for BP's being alowed on end style statments
chrisdp Mar 28, 2026
a708de1
split the logic up a bit so things are more clear and we don't do tel…
chrisdp Mar 28, 2026
47b72d5
Rewrite isStagingLineExecutable with blacklist approach and structure…
chrisdp Mar 31, 2026
118f393
Refine breakpoint validity rules for BrightScript
chrisdp Mar 31, 2026
eb974ff
Revert BrightScriptDebugSession test workaround
chrisdp Mar 31, 2026
8c6d777
Rename isStagingLineExecutable to isValidBreakpointLine and update la…
chrisdp Mar 31, 2026
c555357
Merge branch 'master' into feature/smarter-breakpoint-location-setting
chrisdp Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/debugSession/BrightScriptDebugSession.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1113,7 +1113,8 @@ describe('BrightScriptDebugSession', () => {
await session.setBreakPointsRequest(<any>{}, args);
expect(response.body.breakpoints).to.be.lengthOf(0);

//add breakpoint during live debug session. one was there before, the other is new. Neither will be verified right now
//add breakpoint during live debug session. one was there before, the other is new.
//line 1 was already staged so it comes back verified; line 2 is new so it is not.
args.breakpoints = [{ line: 1 }, { line: 2 }];
await session.setBreakPointsRequest(<any>{}, args);
expect(
Expand Down
15 changes: 8 additions & 7 deletions src/debugSession/BrightScriptDebugSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1200,10 +1200,10 @@ export class BrightScriptDebugSession extends LoggingDebugSession {
//add breakpoint lines to source files and then publish
util.log('Adding stop statements for active breakpoints');

//write the `stop` statements to every file that has breakpoints (do for telnet, skip for debug protocol)
if (!this.enableDebugProtocol) {

await this.breakpointManager.writeBreakpointsForProject(this.projectManager.mainProject);
if (this.enableDebugProtocol) {
await this.breakpointManager.resolveBreakpointsForProject(this.projectManager.mainProject);
Copy link
Copy Markdown
Member

@TwitchBronBron TwitchBronBron Mar 31, 2026

Choose a reason for hiding this comment

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

Let's find different names for these.

"write" still feels good for when it actually is rewriting the code.

"resolve" is the ambiguous one, it feels like it's more like "scan the project and make sure that every breakpoint is allowed to be at this spot, or throw out the ones that aren't allowed or move them". So you know, a good function name for that. ;)

} else {
await this.breakpointManager.injectBreakpointsForProject(this.projectManager.mainProject);
Comment on lines +1204 to +1206
Copy link
Copy Markdown
Member

@TwitchBronBron TwitchBronBron Mar 31, 2026

Choose a reason for hiding this comment

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

Can we move this branching logic inside of a single call? Would prefer avoiding branchout out here if possible.

}

if (this.launchConfiguration.packageTask) {
Expand Down Expand Up @@ -1308,9 +1308,10 @@ export class BrightScriptDebugSession extends LoggingDebugSession {
// Add breakpoint lines to the staging files and before publishing
util.log('Adding stop statements for active breakpoints in Component Libraries');

//write the `stop` statements to every file that has breakpoints (do for telnet, skip for debug protocol)
if (!this.enableDebugProtocol) {
await this.breakpointManager.writeBreakpointsForProject(compLibProject);
if (this.enableDebugProtocol) {
await this.breakpointManager.resolveBreakpointsForProject(compLibProject);
} else {
await this.breakpointManager.injectBreakpointsForProject(compLibProject);
}

await compLibProject.postfixFiles();
Expand Down
Loading