diff --git a/Userland/DevTools/HackStudio/Debugger/Debugger.cpp b/Userland/DevTools/HackStudio/Debugger/Debugger.cpp index 2a5a193fcec..3f22bfcea88 100644 --- a/Userland/DevTools/HackStudio/Debugger/Debugger.cpp +++ b/Userland/DevTools/HackStudio/Debugger/Debugger.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2020, Itamar S. + * Copyright (c) 2024, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -48,36 +49,44 @@ Debugger::Debugger( pthread_cond_init(&m_ui_action_cond, nullptr); } -void Debugger::on_breakpoint_change(ByteString const& file, size_t line, BreakpointChange change_type) +bool Debugger::change_breakpoint(ByteString const& file, size_t line, BreakpointChange change_type) { auto position = create_source_position(file, line); - - if (change_type == BreakpointChange::Added) { - m_breakpoints.append(position); - } else { - m_breakpoints.remove_all_matching([&](Debug::DebugInfo::SourcePosition const& val) { return val == position; }); - } - auto session = Debugger::the().session(); - if (!session) - return; + if (session) { + auto address = session->get_address_from_source_position(position.file_path, position.line_number); + if (!address.has_value()) { + dbgln("Warning: couldn't get instruction address from source"); + return false; + } - auto address = session->get_address_from_source_position(position.file_path, position.line_number); - if (!address.has_value()) { - dbgln("Warning: couldn't get instruction address from source"); - // TODO: Currently, the GUI will indicate that a breakpoint was inserted/removed at this line, - // regardless of whether we actually succeeded to insert it. (For example a breakpoint on a comment, or an include statement). - // We should indicate failure via a return value from this function, and not update the breakpoint GUI if we fail. - return; + switch (change_type) { + case BreakpointChange::Added: + if (session->insert_breakpoint(address.value().address)) { + m_breakpoints.append(position); + return true; + } + break; + case BreakpointChange::Removed: + if (session->remove_breakpoint(address.value().address)) { + m_breakpoints.remove_all_matching([&](Debug::DebugInfo::SourcePosition const& val) { return val == position; }); + return true; + } + break; + } + return false; } - if (change_type == BreakpointChange::Added) { - bool success = session->insert_breakpoint(address.value().address); - VERIFY(success); - } else { - bool success = session->remove_breakpoint(address.value().address); - VERIFY(success); + // No active session, so just modify our internal list of breakpoints + switch (change_type) { + case BreakpointChange::Added: + m_breakpoints.append(position); + return true; + case BreakpointChange::Removed: + m_breakpoints.remove_all_matching([&](Debug::DebugInfo::SourcePosition const& val) { return val == position; }); + return true; } + VERIFY_NOT_REACHED(); } bool Debugger::set_execution_position(ByteString const& file, size_t line) @@ -125,6 +134,7 @@ void Debugger::start() bool success = m_debug_session->insert_breakpoint(address.value().address); VERIFY(success); } else { + // FIXME: Report the invalid breakpoint to the GUI somehow. dbgln("couldn't insert breakpoint"); } } diff --git a/Userland/DevTools/HackStudio/Debugger/Debugger.h b/Userland/DevTools/HackStudio/Debugger/Debugger.h index c6b833b005c..10873a8c013 100644 --- a/Userland/DevTools/HackStudio/Debugger/Debugger.h +++ b/Userland/DevTools/HackStudio/Debugger/Debugger.h @@ -34,7 +34,7 @@ public: static bool is_initialized(); - void on_breakpoint_change(ByteString const& file, size_t line, BreakpointChange change_type); + [[nodiscard]] bool change_breakpoint(ByteString const& file, size_t line, BreakpointChange change_type); bool set_execution_position(ByteString const& file, size_t line); void set_executable_path(ByteString const& path) { m_executable_path = path; } diff --git a/Userland/DevTools/HackStudio/Editor.cpp b/Userland/DevTools/HackStudio/Editor.cpp index 043ba7fa09c..14789139fb8 100644 --- a/Userland/DevTools/HackStudio/Editor.cpp +++ b/Userland/DevTools/HackStudio/Editor.cpp @@ -1,6 +1,7 @@ /* * Copyright (c) 2018-2021, Andreas Kling * Copyright (c) 2018-2022, the SerenityOS developers. + * Copyright (c) 2023-2024, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -774,9 +775,10 @@ void Editor::set_semantic_syntax_highlighting(bool value) ErrorOr Editor::add_breakpoint(size_t line_number) { if (!breakpoint_lines().contains_slow(line_number)) { - add_gutter_indicator(m_breakpoint_indicator_id, line_number); - TRY(breakpoint_lines().try_append(line_number)); - Debugger::the().on_breakpoint_change(wrapper().filename_title(), line_number, BreakpointChange::Added); + if (Debugger::the().change_breakpoint(wrapper().filename_title(), line_number, BreakpointChange::Added)) { + add_gutter_indicator(m_breakpoint_indicator_id, line_number); + TRY(breakpoint_lines().try_append(line_number)); + } } return {}; @@ -784,9 +786,10 @@ ErrorOr Editor::add_breakpoint(size_t line_number) void Editor::remove_breakpoint(size_t line_number) { - remove_gutter_indicator(m_breakpoint_indicator_id, line_number); - breakpoint_lines().remove_first_matching([&](size_t line) { return line == line_number; }); - Debugger::the().on_breakpoint_change(wrapper().filename_title(), line_number, BreakpointChange::Removed); + if (Debugger::the().change_breakpoint(wrapper().filename_title(), line_number, BreakpointChange::Removed)) { + remove_gutter_indicator(m_breakpoint_indicator_id, line_number); + breakpoint_lines().remove_first_matching([&](size_t line) { return line == line_number; }); + } } ErrorOr Editor::update_git_diff_indicators()