From 76243cf297aee0f94ae5e3d80799e21ea4f03860 Mon Sep 17 00:00:00 2001 From: Daniel Pour Bakhsh Date: Fri, 28 Aug 2026 23:37:24 +0200 Subject: [PATCH] Name the debug contracts after the roles they fill The contracts went out in 1.0.28 with names that do not say what their implementers do, and nothing in this repository implements them yet, so the cost of correcting that is at its lowest right now. IDebugLaunchProvider becomes IDebugTargetPreparer and IDebugAdapter becomes IDebugSessionLauncher. Together they are one chain: the preparer brings a target up and produces a DebugLaunchRequest, the launcher decides whether it can serve that request and builds the session. "Launch" already ran through the request and through CanLaunch; the type in between was the only part of that family named after something else. "Adapter" was borrowed from DAP, where it means a process speaking a wire protocol - this one is in-process and speaks none, which the old summary admitted in its first line. The id the two agree on becomes BackendId, which is what it identifies and what its own documentation already called it. Take the breakpoint members off OneWare.Essentials.EditorExtensions.BreakPoint. While a published contract named that class, every change to the editor's margin model would have been a change to published API. Rather than add a second type, DebugStackFrame becomes DebugBreakPointFrame and serves both directions: it already answered the question a breakpoint asks - which place in the target - only reading instead of writing. Setting a breakpoint where the target is halted now needs no conversion, and the contracts folder has no reference pointing out of it. DebugLaunchRequest gains InitCommands so that whoever brings a target up can state what the backend cannot learn from the executable alone. The alternative in practice is a file next to the executable whose name both sides agree on without either contract saying so. RegisterValue becomes DebugRegisterValue, the last entity without the prefix. The namespace does not disambiguate for a caller who has imported it. This is a breaking change for anyone compiled against 1.0.28. Nothing in this repository implements these contracts, so the build here is unaffected, but PluginCompatibilityChecker compares only major, minor and build - a plugin built against a patch-level predecessor still loads and then fails at runtime. Shipping this under a minor bump would turn that into a clean refusal. --- .../Debugger/Entities/DebugBreakPointFrame.cs | 29 +++++++++++++++++++ .../Debugger/Entities/DebugLaunchRequest.cs | 14 +++++++-- ...RegisterValue.cs => DebugRegisterValue.cs} | 2 +- .../Debugger/Entities/DebugSessionState.cs | 4 +-- .../Debugger/Entities/DebugStackFrame.cs | 20 ------------- .../Debugger/Interfaces/IDebugSession.cs | 10 +++---- ...bugAdapter.cs => IDebugSessionLauncher.cs} | 18 +++++++----- ...nchProvider.cs => IDebugTargetPreparer.cs} | 20 +++++++------ .../Debugger/Interfaces/IDebuggerService.cs | 24 +++++++-------- 9 files changed, 82 insertions(+), 59 deletions(-) create mode 100644 src/OneWare.Essentials/Debugger/Entities/DebugBreakPointFrame.cs rename src/OneWare.Essentials/Debugger/Entities/{RegisterValue.cs => DebugRegisterValue.cs} (89%) delete mode 100644 src/OneWare.Essentials/Debugger/Entities/DebugStackFrame.cs rename src/OneWare.Essentials/Debugger/Interfaces/{IDebugAdapter.cs => IDebugSessionLauncher.cs} (57%) rename src/OneWare.Essentials/Debugger/Interfaces/{IDebugLaunchProvider.cs => IDebugTargetPreparer.cs} (59%) diff --git a/src/OneWare.Essentials/Debugger/Entities/DebugBreakPointFrame.cs b/src/OneWare.Essentials/Debugger/Entities/DebugBreakPointFrame.cs new file mode 100644 index 00000000..33377662 --- /dev/null +++ b/src/OneWare.Essentials/Debugger/Entities/DebugBreakPointFrame.cs @@ -0,0 +1,29 @@ +namespace OneWare.Essentials.Debugger.Entities; + +/// +/// A place in the target. Used in both directions: the session reports where the target is +/// halted, and a breakpoint asks it to halt somewhere. Both answer the same question — where — +/// so both use the same type, and putting a breakpoint on the spot the target already stands on +/// needs no conversion. +/// Deliberately not the editor's own breakpoint model: while a published contract named that +/// class, every change to the margin was a change to published API. +/// +/// Name of the function, if the backend reported one. May also be set on +/// its own to place a breakpoint on a function by name. +/// +/// Absolute source path, or if the address could not be mapped — a +/// program without debug symbols, code compiled without them, or a halt at an address the line +/// table does not cover. Set on every breakpoint coming from the margin. +/// The editor only jumps to the source location when this is set. +/// +/// One-based line number. 0 means "unknown" when read from the target +/// and "no line given" when written to it. +/// +/// Program counter as formatted by the backend, e.g. 0x00000108. The only location +/// available when no debug symbols are present. +/// +public sealed record DebugBreakPointFrame( + string? Function, + string? File, + int Line, + string? Address); diff --git a/src/OneWare.Essentials/Debugger/Entities/DebugLaunchRequest.cs b/src/OneWare.Essentials/Debugger/Entities/DebugLaunchRequest.cs index ff05b612..e5c59284 100644 --- a/src/OneWare.Essentials/Debugger/Entities/DebugLaunchRequest.cs +++ b/src/OneWare.Essentials/Debugger/Entities/DebugLaunchRequest.cs @@ -7,14 +7,22 @@ namespace OneWare.Essentials.Debugger.Entities; /// is the whole remote seam: a plugin that brings up a target /// passes the address it is listening on and never learns which backend connects. /// -/// Identifies the backend, e.g. GDB. +/// Identifies the backend, e.g. GDB. /// /// Path to the executable, e.g. an ELF file. Carries the program and its debug symbols. /// /// Remote stub address, e.g. localhost:1234. /// Working directory for the debug session. +/// +/// Commands the backend applies at startup, before it connects to the target, in this order. +/// Whoever brings a target up states here what the backend cannot learn from the executable +/// alone — a register description, for instance. Passing them with the request keeps the +/// handover in one place: the alternative is a file next to the executable whose name both +/// sides have to agree on without either contract saying so. +/// public sealed record DebugLaunchRequest( - string AdapterId, + string BackendId, string? ExecutablePath = null, string? RemoteEndpoint = null, - string? WorkingDirectory = null); + string? WorkingDirectory = null, + IReadOnlyList? InitCommands = null); diff --git a/src/OneWare.Essentials/Debugger/Entities/RegisterValue.cs b/src/OneWare.Essentials/Debugger/Entities/DebugRegisterValue.cs similarity index 89% rename from src/OneWare.Essentials/Debugger/Entities/RegisterValue.cs rename to src/OneWare.Essentials/Debugger/Entities/DebugRegisterValue.cs index 15288c7e..06494438 100644 --- a/src/OneWare.Essentials/Debugger/Entities/RegisterValue.cs +++ b/src/OneWare.Essentials/Debugger/Entities/DebugRegisterValue.cs @@ -5,6 +5,6 @@ namespace OneWare.Essentials.Debugger.Entities; /// /// As reported by the target, e.g. sp or pc. /// Formatted by the backend; the UI displays the string unchanged. -public sealed record RegisterValue( +public sealed record DebugRegisterValue( string Name, string Value); diff --git a/src/OneWare.Essentials/Debugger/Entities/DebugSessionState.cs b/src/OneWare.Essentials/Debugger/Entities/DebugSessionState.cs index 2ceea45e..c9025a5d 100644 --- a/src/OneWare.Essentials/Debugger/Entities/DebugSessionState.cs +++ b/src/OneWare.Essentials/Debugger/Entities/DebugSessionState.cs @@ -22,14 +22,14 @@ public sealed record DebugSessionState /// /// Where the target is halted, or while it runs. /// - public DebugStackFrame? CurrentFrame { get; init; } + public DebugBreakPointFrame? CurrentFrame { get; init; } /// /// Register contents as of the last halt. Empty while the target runs, and empty for a /// backend that cannot read registers — the panel then simply shows nothing, which is what a /// separate capability flag would have told it to do anyway. /// - public IReadOnlyList Registers { get; init; } = []; + public IReadOnlyList Registers { get; init; } = []; /// /// Locals of as of the last halt. Empty while the target runs, diff --git a/src/OneWare.Essentials/Debugger/Entities/DebugStackFrame.cs b/src/OneWare.Essentials/Debugger/Entities/DebugStackFrame.cs deleted file mode 100644 index b076e0c6..00000000 --- a/src/OneWare.Essentials/Debugger/Entities/DebugStackFrame.cs +++ /dev/null @@ -1,20 +0,0 @@ -namespace OneWare.Essentials.Debugger.Entities; - -/// -/// Where the target is halted. -/// -/// Name of the function, if the backend reported one. -/// -/// Absolute source path, or if the address could not be mapped. -/// The editor only jumps to the source location when this is set. -/// -/// One-based line number, or 0 if unknown. -/// -/// Program counter as formatted by the backend, e.g. 0x00000108. The only location -/// available when no debug symbols are present. -/// -public sealed record DebugStackFrame( - string? Function, - string? File, - int Line, - string? Address); diff --git a/src/OneWare.Essentials/Debugger/Interfaces/IDebugSession.cs b/src/OneWare.Essentials/Debugger/Interfaces/IDebugSession.cs index f2efe466..e51187fe 100644 --- a/src/OneWare.Essentials/Debugger/Interfaces/IDebugSession.cs +++ b/src/OneWare.Essentials/Debugger/Interfaces/IDebugSession.cs @@ -1,5 +1,4 @@ using OneWare.Essentials.Debugger.Entities; -using OneWare.Essentials.EditorExtensions; namespace OneWare.Essentials.Debugger.Interfaces; @@ -16,7 +15,7 @@ public interface IDebugSession /// /// Identifies the backend, e.g. GDB. /// - public string AdapterId { get; } + public string BackendId { get; } /// /// Latest published state. @@ -85,12 +84,13 @@ public interface IDebugSession /// Returns if the target refused it, e.g. because it ran out of /// hardware breakpoints. /// - public Task SetBreakpointAsync(BreakPoint breakpoint); + public Task SetBreakpointAsync(DebugBreakPointFrame frame); /// - /// Removes a previously armed breakpoint. + /// Removes a previously armed breakpoint. Removing one that is not armed counts as success — + /// the requested state is what matters, not how it was reached. /// - public Task RemoveBreakpointAsync(BreakPoint breakpoint); + public Task RemoveBreakpointAsync(DebugBreakPointFrame frame); /// /// Reads memory from the target. is whatever the backend accepts — diff --git a/src/OneWare.Essentials/Debugger/Interfaces/IDebugAdapter.cs b/src/OneWare.Essentials/Debugger/Interfaces/IDebugSessionLauncher.cs similarity index 57% rename from src/OneWare.Essentials/Debugger/Interfaces/IDebugAdapter.cs rename to src/OneWare.Essentials/Debugger/Interfaces/IDebugSessionLauncher.cs index 17892d44..97f6548d 100644 --- a/src/OneWare.Essentials/Debugger/Interfaces/IDebugAdapter.cs +++ b/src/OneWare.Essentials/Debugger/Interfaces/IDebugSessionLauncher.cs @@ -3,26 +3,30 @@ namespace OneWare.Essentials.Debugger.Interfaces; /// -/// More of a session factory than a real adapter. The name is borrowed from VS Code's DAP -/// (Debug Adapter Protocol), where "debug adapter" is the term for the backend itself. +/// Turns a into a session for one particular backend. Together +/// with it forms the whole chain: the preparer brings the +/// target up and produces the request, the launcher decides who can serve it and builds the +/// session. +/// What VS Code's DAP calls a "debug adapter" is this — the name is deliberately not borrowed, +/// because this one is in-process and never speaks the protocol. /// is synchronous by intent, so that everything which can block or /// fail happens in — one failure path instead of two. /// -public interface IDebugAdapter +public interface IDebugSessionLauncher { /// - /// Stable identifier, referenced by . + /// Stable identifier, referenced by . /// public string Id { get; } /// - /// Shown when the user picks a backend. + /// Human-readable name of the backend, e.g. GNU Debugger. /// public string DisplayName { get; } /// - /// Returns if this adapter can handle the given request. - /// Must be cheap and free of side effects — it decides whether to offer this adapter at all. + /// Returns if this launcher can serve the given request. + /// Must be cheap and free of side effects — it decides whether this launcher is used at all. /// public bool CanLaunch(DebugLaunchRequest launchRequest); diff --git a/src/OneWare.Essentials/Debugger/Interfaces/IDebugLaunchProvider.cs b/src/OneWare.Essentials/Debugger/Interfaces/IDebugTargetPreparer.cs similarity index 59% rename from src/OneWare.Essentials/Debugger/Interfaces/IDebugLaunchProvider.cs rename to src/OneWare.Essentials/Debugger/Interfaces/IDebugTargetPreparer.cs index 7beb1871..57a5ddf7 100644 --- a/src/OneWare.Essentials/Debugger/Interfaces/IDebugLaunchProvider.cs +++ b/src/OneWare.Essentials/Debugger/Interfaces/IDebugTargetPreparer.cs @@ -4,29 +4,31 @@ namespace OneWare.Essentials.Debugger.Interfaces; /// /// Analogous to , but as the preparation step. The core asks -/// which provider fits the current project, has it prepare, and starts with whatever request +/// which preparer fits the current project, has it prepare, and starts with whatever request /// comes back. That keeps the entry point in the generic UI while everything target-specific /// stays in the plugin. /// -public interface IDebugLaunchProvider +public interface IDebugTargetPreparer { /// - /// Shown in the launch selection of the debug panel. + /// Names the preparer in the status line and in the debug console. /// public string DisplayName { get; } /// - /// Returns if this provider can handle the active project. - /// Must be cheap and free of side effects — the UI calls it to fill the selection. + /// Returns if this preparer can handle the active project. + /// Must be cheap and free of side effects — the UI calls it to pick a preparer. /// public bool CanPrepare(); /// - /// Brings the target up and returns the matching launch request. - /// Returns if preparation failed or was cancelled; the user has - /// already been notified in that case. + /// Brings the target up and returns the matching launch request. A running preparation + /// cannot be aborted; the UI locks the start button and waits for it, so keep the steps + /// short and report what is happening. + /// Returns if preparation failed; the user has already been notified + /// in that case. /// - public Task PrepareAsync(CancellationToken ct = default); + public Task PrepareAsync(); /// /// Releases whatever claimed. Also runs when the session ended diff --git a/src/OneWare.Essentials/Debugger/Interfaces/IDebuggerService.cs b/src/OneWare.Essentials/Debugger/Interfaces/IDebuggerService.cs index defc105b..fa9fede4 100644 --- a/src/OneWare.Essentials/Debugger/Interfaces/IDebuggerService.cs +++ b/src/OneWare.Essentials/Debugger/Interfaces/IDebuggerService.cs @@ -12,13 +12,13 @@ public interface IDebuggerService /// /// Registered backends, including the core's own. /// - public IReadOnlyList Adapters { get; } + public IReadOnlyList SessionLaunchers { get; } /// - /// Registered launch providers. Whoever fits the active project shows up in the launch - /// selection of the debug panel. + /// Registered target preparers. Starting the debugger picks the first one whose + /// accepts the active project. /// - public IReadOnlyList LaunchProviders { get; } + public IReadOnlyList TargetPreparers { get; } /// /// The active session, or if none is running. @@ -43,29 +43,29 @@ public interface IDebuggerService public event EventHandler? StateChanged; /// - /// Registers an adapter. Resolved from the container — the implementation gets constructor + /// Registers an launcher. Resolved from the container — the implementation gets constructor /// injection like any other service. /// - public void RegisterAdapter() where T : IDebugAdapter; + public void RegisterSessionLauncher() where T : IDebugSessionLauncher; /// - /// Registers a launch provider. Resolved from the container like adapters. + /// Registers a target preparer. Resolved from the container like session launchers. /// - public void RegisterLaunchProvider() where T : IDebugLaunchProvider; + public void RegisterTargetPreparer() where T : IDebugTargetPreparer; /// /// Starts a session, arms the breakpoints currently set in the editor and runs the program. - /// Returns if no adapter accepted the request or the backend did not + /// Returns if no launcher accepted the request or the backend did not /// come up; nothing is left running in that case. /// public Task StartAsync(DebugLaunchRequest launchRequest); /// - /// Calls first, then starts with the - /// returned request. runs as soon as the + /// Calls first, then starts with the + /// returned request. runs as soon as the /// session ends, no matter how it ended. /// - public Task StartAsync(IDebugLaunchProvider provider, CancellationToken ct = default); + public Task StartAsync(IDebugTargetPreparer preparer); /// /// Ends the active session. Does nothing if none is running.