From 2dd7fa2d445665ae41db2dd0e1fe7b404e525a57 Mon Sep 17 00:00:00 2001 From: Arda Cinar Date: Wed, 7 Dec 2022 21:32:07 +0300 Subject: [PATCH] HexEditor: Handle some errors inside the editor Specifically, the ones HexEditor::did_complete_action possibly raised in case creating an undo stack entry or pushing it onto the undo stack fails. In this case, an error popup is displayed and the modifications are undone. This removes 2 FIXMEs inside the code :^) --- Userland/Applications/HexEditor/HexEditor.cpp | 75 +++++++++++++------ Userland/Applications/HexEditor/HexEditor.h | 10 +-- .../HexEditor/HexEditorWidget.cpp | 4 +- 3 files changed, 60 insertions(+), 29 deletions(-) diff --git a/Userland/Applications/HexEditor/HexEditor.cpp b/Userland/Applications/HexEditor/HexEditor.cpp index e1b23a7370..03d762f603 100644 --- a/Userland/Applications/HexEditor/HexEditor.cpp +++ b/Userland/Applications/HexEditor/HexEditor.cpp @@ -8,9 +8,10 @@ */ #include "HexEditor.h" -#include "AK/Format.h" #include "SearchResultsModel.h" #include +#include +#include #include #include #include @@ -77,10 +78,10 @@ void HexEditor::open_file(NonnullRefPtr file) update_status(); } -void HexEditor::fill_selection(u8 fill_byte) +ErrorOr HexEditor::fill_selection(u8 fill_byte) { if (!has_selection()) - return; + return {}; ByteBuffer old_values; ByteBuffer new_values; @@ -97,10 +98,19 @@ void HexEditor::fill_selection(u8 fill_byte) m_document->set(position, fill_byte); } - did_complete_action(m_selection_start, move(old_values), move(new_values)); + auto result = did_complete_action(m_selection_start, move(old_values), move(new_values)); + if (result.is_error()) { + for (size_t i = 0; i < length; i++) { + size_t position = m_selection_start + i; + m_document->set(position, old_values[i]); + } + return result; + } update(); did_change(); + + return {}; } void HexEditor::set_position(size_t position) @@ -463,22 +473,24 @@ void HexEditor::keydown_event(GUI::KeyEvent& event) } if (!event.ctrl() && !event.alt() && !event.text().is_empty()) { + ErrorOr result; if (m_edit_mode == EditMode::Hex) { - hex_mode_keydown_event(event); + result = hex_mode_keydown_event(event); } else { - text_mode_keydown_event(event); + result = text_mode_keydown_event(event); } - return; + if (result.is_error()) + GUI::MessageBox::show_error(window(), DeprecatedString::formatted("{}", result.error())); } event.ignore(); } -void HexEditor::hex_mode_keydown_event(GUI::KeyEvent& event) +ErrorOr HexEditor::hex_mode_keydown_event(GUI::KeyEvent& event) { if ((event.key() >= KeyCode::Key_0 && event.key() <= KeyCode::Key_9) || (event.key() >= KeyCode::Key_A && event.key() <= KeyCode::Key_F)) { if (m_document->size() == 0) - return; + return {}; VERIFY(m_position <= m_document->size()); @@ -493,12 +505,24 @@ void HexEditor::hex_mode_keydown_event(GUI::KeyEvent& event) u8 existing_change = m_document->get(m_position).value; u8 new_value = value << 4 | (existing_change & 0xF); // shift new value left 4 bits, OR with existing last 4 bits m_document->set(m_position, new_value); - did_complete_action(m_position, old_value, new_value); + + auto result = did_complete_action(m_position, old_value, new_value); + if (result.is_error()) { + m_document->set(m_position, old_value); + return result; + } + m_cursor_at_low_nibble = true; } else { u8 new_value = (m_document->get(m_position).value & 0xF0) | value; // save the first 4 bits, OR the new value in the last 4 m_document->set(m_position, new_value); - did_complete_action(m_position, old_value, new_value); + + auto result = did_complete_action(m_position, old_value, new_value); + if (result.is_error()) { + m_document->set(m_position, old_value); + return result; + } + if (m_position + 1 < m_document->size()) m_position++; m_cursor_at_low_nibble = false; @@ -509,21 +533,23 @@ void HexEditor::hex_mode_keydown_event(GUI::KeyEvent& event) update_status(); did_change(); } + + return {}; } -void HexEditor::text_mode_keydown_event(GUI::KeyEvent& event) +ErrorOr HexEditor::text_mode_keydown_event(GUI::KeyEvent& event) { if (m_document->size() == 0) - return; + return {}; VERIFY(m_position < m_document->size()); if (event.code_point() == 0) // This is a control key - return; + return {}; auto old_value = m_document->get(m_position).value; auto new_value = event.code_point(); m_document->set(m_position, new_value); - did_complete_action(m_position, old_value, new_value); + TRY(did_complete_action(m_position, old_value, new_value)); if (m_position + 1 < m_document->size()) m_position++; @@ -533,6 +559,8 @@ void HexEditor::text_mode_keydown_event(GUI::KeyEvent& event) update(); update_status(); did_change(); + + return {}; } void HexEditor::update_status() @@ -824,26 +852,27 @@ void HexEditor::reset_cursor_blink_state() m_blink_timer->restart(); } -void HexEditor::did_complete_action(size_t position, u8 old_value, u8 new_value) +ErrorOr HexEditor::did_complete_action(size_t position, u8 old_value, u8 new_value) { if (old_value == new_value) - return; + return {}; auto command = make(m_document->make_weak_ptr(), position); // We know this won't allocate because the buffers start empty MUST(command->try_add_changed_byte(old_value, new_value)); - // FIXME: Handle errors - MUST(m_undo_stack.try_push(move(command))); + + TRY(m_undo_stack.try_push(move(command))); + return {}; } -void HexEditor::did_complete_action(size_t position, ByteBuffer&& old_values, ByteBuffer&& new_values) +ErrorOr HexEditor::did_complete_action(size_t position, ByteBuffer&& old_values, ByteBuffer&& new_values) { auto command = make(m_document->make_weak_ptr(), position); - // FIXME: Handle errors - MUST(command->try_add_changed_bytes(move(old_values), move(new_values))); - MUST(m_undo_stack.try_push(move(command))); + TRY(command->try_add_changed_bytes(move(old_values), move(new_values))); + TRY(m_undo_stack.try_push(move(command))); + return {}; } bool HexEditor::undo() diff --git a/Userland/Applications/HexEditor/HexEditor.h b/Userland/Applications/HexEditor/HexEditor.h index 1b2cac4fea..8bb6ada27c 100644 --- a/Userland/Applications/HexEditor/HexEditor.h +++ b/Userland/Applications/HexEditor/HexEditor.h @@ -36,7 +36,7 @@ public: size_t buffer_size() const { return m_document->size(); } ErrorOr open_new_file(size_t size); void open_file(NonnullRefPtr file); - void fill_selection(u8 fill_byte); + ErrorOr fill_selection(u8 fill_byte); Optional get_byte(size_t position); bool save_as(NonnullRefPtr); bool save(); @@ -101,14 +101,14 @@ private: size_t cell_width() const { return character_width() * 3; } size_t offset_margin_width() const { return 80; } - void hex_mode_keydown_event(GUI::KeyEvent&); - void text_mode_keydown_event(GUI::KeyEvent&); + ErrorOr hex_mode_keydown_event(GUI::KeyEvent&); + ErrorOr text_mode_keydown_event(GUI::KeyEvent&); void set_content_length(size_t); // I might make this public if I add fetching data on demand. void update_status(); void did_change(); - void did_complete_action(size_t position, u8 old_value, u8 new_value); - void did_complete_action(size_t position, ByteBuffer&& old_values, ByteBuffer&& new_values); + ErrorOr did_complete_action(size_t position, u8 old_value, u8 new_value); + ErrorOr did_complete_action(size_t position, ByteBuffer&& old_values, ByteBuffer&& new_values); void reset_cursor_blink_state(); }; diff --git a/Userland/Applications/HexEditor/HexEditorWidget.cpp b/Userland/Applications/HexEditor/HexEditorWidget.cpp index 2f050770e6..f200c4d8b4 100644 --- a/Userland/Applications/HexEditor/HexEditorWidget.cpp +++ b/Userland/Applications/HexEditor/HexEditorWidget.cpp @@ -245,7 +245,9 @@ HexEditorWidget::HexEditorWidget() DeprecatedString value; if (GUI::InputBox::show(window(), value, "Fill byte (hex):"sv, "Fill Selection"sv) == GUI::InputBox::ExecResult::OK && !value.is_empty()) { auto fill_byte = strtol(value.characters(), nullptr, 16); - m_editor->fill_selection(fill_byte); + auto result = m_editor->fill_selection(fill_byte); + if (result.is_error()) + GUI::MessageBox::show_error(window(), DeprecatedString::formatted("{}", result.error())); } }); m_fill_selection_action->set_enabled(false);