Przeglądaj źródła

KeyboardMapper: Propagate errors using ErrorOr and TRY

KeyboardMapperWidget's load_map_from_file, load_map_from_system, save,
and save_to_file now all return ErrorOr<void> and no longer handles
alerting the user to potential errors.

main is now responsible for handling errors originating from its calls
to these four functions; it will simply alert the user using the new
method KeyboardMapperWidget::show_error_to_user(Error), which simply
creates a MassageBox displaying the error's string_literal.

This makes the whole program slight more clean feeling :^).
RasmusNylander 3 lat temu
rodzic
commit
9678ff15a8

+ 20 - 38
Userland/Applications/KeyboardMapper/KeyboardMapperWidget.cpp

@@ -114,17 +114,12 @@ u32* KeyboardMapperWidget::map_from_name(const StringView map_name)
     return map;
 }
 
-void KeyboardMapperWidget::load_from_file(String filename)
+ErrorOr<void> KeyboardMapperWidget::load_map_from_file(const String& filename)
 {
-    auto result = Keyboard::CharacterMapFile::load_from_file(filename);
-    if (!result.has_value()) {
-        auto error_message = String::formatted("Failed to load character map from file {}", filename);
-        GUI::MessageBox::show(window(), error_message, "Error", GUI::MessageBox::Type::Error);
-        return;
-    }
+    auto character_map = TRY(Keyboard::CharacterMapFile::load_from_file(filename));
 
     m_filename = filename;
-    m_character_map = result.value();
+    m_character_map = character_map;
     set_current_map("map");
 
     for (auto& widget : m_map_group->child_widgets()) {
@@ -133,15 +128,15 @@ void KeyboardMapperWidget::load_from_file(String filename)
     }
 
     update_window_title();
+    return {};
 }
 
-void KeyboardMapperWidget::load_from_system()
+ErrorOr<void> KeyboardMapperWidget::load_map_from_system()
 {
-    auto result = Keyboard::CharacterMap::fetch_system_map();
-    VERIFY(!result.is_error());
+    auto character_map = TRY(Keyboard::CharacterMap::fetch_system_map());
 
-    m_filename = String::formatted("/res/keymaps/{}.json", result.value().character_map_name());
-    m_character_map = result.value().character_map_data();
+    m_filename = String::formatted("/res/keymaps/{}.json", character_map.character_map_name());
+    m_character_map = character_map.character_map_data();
     set_current_map("map");
 
     for (auto& widget : m_map_group->child_widgets()) {
@@ -150,14 +145,15 @@ void KeyboardMapperWidget::load_from_system()
     }
 
     update_window_title();
+    return {};
 }
 
-void KeyboardMapperWidget::save()
+ErrorOr<void> KeyboardMapperWidget::save()
 {
-    save_to_file(m_filename);
+    return save_to_file(m_filename);
 }
 
-void KeyboardMapperWidget::save_to_file(StringView filename)
+ErrorOr<void> KeyboardMapperWidget::save_to_file(StringView filename)
 {
     JsonObject map_json;
 
@@ -182,34 +178,16 @@ void KeyboardMapperWidget::save_to_file(StringView filename)
 
     // Write to file.
     String file_content = map_json.to_string();
-
-    auto file = Core::File::construct(filename);
-    file->open(Core::OpenMode::WriteOnly);
-    if (!file->is_open()) {
-        StringBuilder sb;
-        sb.append("Failed to open ");
-        sb.append(filename);
-        sb.append(" for write. Error: ");
-        sb.append(file->error_string());
-
-        GUI::MessageBox::show(window(), sb.to_string(), "Error", GUI::MessageBox::Type::Error);
-        return;
-    }
+    auto file = TRY(Core::File::open(filename, Core::OpenMode::WriteOnly));
 
     bool result = file->write(file_content);
-    if (!result) {
-        int error_number = errno;
-        StringBuilder sb;
-        sb.append("Unable to save file. Error: ");
-        sb.append(strerror(error_number));
-
-        GUI::MessageBox::show(window(), sb.to_string(), "Error", GUI::MessageBox::Type::Error);
-        return;
-    }
+    if (!result)
+        return Error::from_errno(file->error());
 
     m_modified = false;
     m_filename = filename;
     update_window_title();
+    return {};
 }
 
 void KeyboardMapperWidget::keydown_event(GUI::KeyEvent& event)
@@ -265,3 +243,7 @@ void KeyboardMapperWidget::update_window_title()
 
     window()->set_title(sb.to_string());
 }
+
+void KeyboardMapperWidget::show_error_to_user(Error error){
+    GUI::MessageBox::show_error(window(), error.string_literal());
+}

+ 5 - 4
Userland/Applications/KeyboardMapper/KeyboardMapperWidget.h

@@ -17,10 +17,11 @@ public:
     virtual ~KeyboardMapperWidget() override;
 
     void create_frame();
-    void load_from_file(const String);
-    void load_from_system();
-    void save();
-    void save_to_file(StringView);
+    ErrorOr<void> load_map_from_file(const String&);
+    ErrorOr<void> load_map_from_system();
+    ErrorOr<void> save();
+    ErrorOr<void> save_to_file(StringView);
+    void show_error_to_user(Error);
 
 protected:
     virtual void keydown_event(GUI::KeyEvent&) override;

+ 16 - 11
Userland/Applications/KeyboardMapper/main.cpp

@@ -17,7 +17,7 @@
 
 ErrorOr<int> serenity_main(Main::Arguments arguments)
 {
-    const char* path = nullptr;
+    StringView path;
     Core::ArgsParser args_parser;
     args_parser.add_positional_argument(path, "Keyboard character mapping file.", "file", Core::ArgsParser::Required::No);
     args_parser.parse(arguments);
@@ -38,25 +38,28 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     window->set_resizable(false);
 
     auto keyboard_mapper_widget = (KeyboardMapperWidget*)window->main_widget();
-    if (path != nullptr) {
-        keyboard_mapper_widget->load_from_file(path);
-    } else {
-        keyboard_mapper_widget->load_from_system();
-    }
+    if (path.is_empty())
+        TRY(keyboard_mapper_widget->load_map_from_system());
+    else
+        TRY(keyboard_mapper_widget->load_map_from_file(path));
 
     TRY(Core::System::pledge("stdio thread rpath cpath wpath recvfd sendfd"));
 
     auto open_action = GUI::CommonActions::make_open_action(
         [&](auto&) {
             Optional<String> path = GUI::FilePicker::get_open_filepath(window, "Open", "/res/keymaps/");
-            if (path.has_value()) {
-                keyboard_mapper_widget->load_from_file(path.value());
-            }
+            if (!path.has_value()) return;
+
+            ErrorOr<void> error_or = keyboard_mapper_widget->load_map_from_file(path.value());
+            if (error_or.is_error())
+                keyboard_mapper_widget->show_error_to_user(error_or.error());
         });
 
     auto save_action = GUI::CommonActions::make_save_action(
         [&](auto&) {
-            keyboard_mapper_widget->save();
+            ErrorOr<void> error_or = keyboard_mapper_widget->save();
+            if (error_or.is_error())
+                keyboard_mapper_widget->show_error_to_user(error_or.error());
         });
 
     auto save_as_action = GUI::CommonActions::make_save_as_action([&](auto&) {
@@ -65,7 +68,9 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         if (!save_path.has_value())
             return;
 
-        keyboard_mapper_widget->save_to_file(save_path.value());
+        ErrorOr<void> error_or = keyboard_mapper_widget->save_to_file(save_path.value());
+        if (error_or.is_error())
+            keyboard_mapper_widget->show_error_to_user(error_or.error());
     });
 
     auto quit_action = GUI::CommonActions::make_quit_action(