Sfoglia il codice sorgente

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 :^)
Arda Cinar 2 anni fa
parent
commit
2dd7fa2d44

+ 52 - 23
Userland/Applications/HexEditor/HexEditor.cpp

@@ -8,9 +8,10 @@
  */
 
 #include "HexEditor.h"
-#include "AK/Format.h"
 #include "SearchResultsModel.h"
 #include <AK/Debug.h>
+#include <AK/DeprecatedString.h>
+#include <AK/Format.h>
 #include <AK/ScopeGuard.h>
 #include <AK/StringBuilder.h>
 #include <LibGUI/Action.h>
@@ -77,10 +78,10 @@ void HexEditor::open_file(NonnullRefPtr<Core::File> file)
     update_status();
 }
 
-void HexEditor::fill_selection(u8 fill_byte)
+ErrorOr<void> 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<void> 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<void> 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<void> 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<void> HexEditor::did_complete_action(size_t position, u8 old_value, u8 new_value)
 {
     if (old_value == new_value)
-        return;
+        return {};
 
     auto command = make<HexDocumentUndoCommand>(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<void> HexEditor::did_complete_action(size_t position, ByteBuffer&& old_values, ByteBuffer&& new_values)
 {
     auto command = make<HexDocumentUndoCommand>(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()

+ 5 - 5
Userland/Applications/HexEditor/HexEditor.h

@@ -36,7 +36,7 @@ public:
     size_t buffer_size() const { return m_document->size(); }
     ErrorOr<void> open_new_file(size_t size);
     void open_file(NonnullRefPtr<Core::File> file);
-    void fill_selection(u8 fill_byte);
+    ErrorOr<void> fill_selection(u8 fill_byte);
     Optional<u8> get_byte(size_t position);
     bool save_as(NonnullRefPtr<Core::File>);
     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<void> hex_mode_keydown_event(GUI::KeyEvent&);
+    ErrorOr<void> 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<void> did_complete_action(size_t position, u8 old_value, u8 new_value);
+    ErrorOr<void> did_complete_action(size_t position, ByteBuffer&& old_values, ByteBuffer&& new_values);
 
     void reset_cursor_blink_state();
 };

+ 3 - 1
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);