diff --git a/Userland/Libraries/LibCore/Object.cpp b/Userland/Libraries/LibCore/Object.cpp index d09ec7fe39..ccb93c4eb0 100644 --- a/Userland/Libraries/LibCore/Object.cpp +++ b/Userland/Libraries/LibCore/Object.cpp @@ -263,7 +263,7 @@ static HashMap& object_classes() return s_map; } -ObjectClassRegistration::ObjectClassRegistration(StringView class_name, Function()> factory, ObjectClassRegistration* parent_class) +ObjectClassRegistration::ObjectClassRegistration(StringView class_name, Function>()> factory, ObjectClassRegistration* parent_class) : m_class_name(class_name) , m_factory(move(factory)) , m_parent_class(parent_class) diff --git a/Userland/Libraries/LibCore/Object.h b/Userland/Libraries/LibCore/Object.h index 36eeae6784..2a618fb166 100644 --- a/Userland/Libraries/LibCore/Object.h +++ b/Userland/Libraries/LibCore/Object.h @@ -22,18 +22,18 @@ namespace Core { -#define REGISTER_ABSTRACT_CORE_OBJECT(namespace_, class_name) \ - namespace Core { \ - namespace Registration { \ - Core::ObjectClassRegistration registration_##class_name(#namespace_ "::" #class_name##sv, []() { return RefPtr(); }); \ - } \ +#define REGISTER_ABSTRACT_CORE_OBJECT(namespace_, class_name) \ + namespace Core { \ + namespace Registration { \ + Core::ObjectClassRegistration registration_##class_name(#namespace_ "::" #class_name##sv, []() { return Error::from_string_literal("Attempted to construct an abstract object."); }); \ + } \ } -#define REGISTER_CORE_OBJECT(namespace_, class_name) \ - namespace Core { \ - namespace Registration { \ - Core::ObjectClassRegistration registration_##class_name(#namespace_ "::" #class_name##sv, []() { return namespace_::class_name::construct(); }); \ - } \ +#define REGISTER_CORE_OBJECT(namespace_, class_name) \ + namespace Core { \ + namespace Registration { \ + Core::ObjectClassRegistration registration_##class_name(#namespace_ "::" #class_name##sv, []() { return namespace_::class_name::try_create(); }); \ + } \ } class ObjectClassRegistration { @@ -41,12 +41,12 @@ class ObjectClassRegistration { AK_MAKE_NONMOVABLE(ObjectClassRegistration); public: - ObjectClassRegistration(StringView class_name, Function()> factory, ObjectClassRegistration* parent_class = nullptr); + ObjectClassRegistration(StringView class_name, Function>()> factory, ObjectClassRegistration* parent_class = nullptr); ~ObjectClassRegistration() = default; StringView class_name() const { return m_class_name; } ObjectClassRegistration const* parent_class() const { return m_parent_class; } - RefPtr construct() const { return m_factory(); } + ErrorOr> construct() const { return m_factory(); } bool is_derived_from(ObjectClassRegistration const& base_class) const; static void for_each(Function); @@ -54,7 +54,7 @@ public: private: StringView m_class_name; - Function()> m_factory; + Function>()> m_factory; ObjectClassRegistration* m_parent_class { nullptr }; }; diff --git a/Userland/Libraries/LibGUI/GML/AST.h b/Userland/Libraries/LibGUI/GML/AST.h index f5c0104f80..85c84cc925 100644 --- a/Userland/Libraries/LibGUI/GML/AST.h +++ b/Userland/Libraries/LibGUI/GML/AST.h @@ -202,18 +202,18 @@ public: } } - // Uses IterationDecision to allow the callback to interrupt the iteration, like a for-loop break. - template> Callback> - void for_each_child_object_interruptible(Callback callback) + template> Callback> + ErrorOr try_for_each_child_object(Callback callback) { for (NonnullRefPtr child : m_sub_objects) { // doesn't capture layout as intended, as that's behind a kv-pair if (is(child.ptr())) { auto object = static_ptr_cast(child); - if (callback(object) == IterationDecision::Break) - return; + TRY(callback(object)); } } + + return {}; } RefPtr layout_object() const diff --git a/Userland/Libraries/LibGUI/GML/AutocompleteProvider.cpp b/Userland/Libraries/LibGUI/GML/AutocompleteProvider.cpp index 5f6ea7db18..a94fcf32d8 100644 --- a/Userland/Libraries/LibGUI/GML/AutocompleteProvider.cpp +++ b/Userland/Libraries/LibGUI/GML/AutocompleteProvider.cpp @@ -143,8 +143,8 @@ void AutocompleteProvider::provide_completions(Functionis_derived_from(widget_class) || registration->is_derived_from(layout_class))) { - if (auto instance = registration->construct()) { - for (auto& it : instance->properties()) { + if (auto instance = registration->construct(); !instance.is_error()) { + for (auto& it : instance.value()->properties()) { if (!it.value->is_readonly() && it.key.matches(pattern)) identifier_entries.empend(DeprecatedString::formatted("{}: ", it.key), partial_input_length, CodeComprehension::Language::Unspecified, it.key); } diff --git a/Userland/Libraries/LibGUI/ScrollableContainerWidget.cpp b/Userland/Libraries/LibGUI/ScrollableContainerWidget.cpp index 129529463a..321ff681a3 100644 --- a/Userland/Libraries/LibGUI/ScrollableContainerWidget.cpp +++ b/Userland/Libraries/LibGUI/ScrollableContainerWidget.cpp @@ -102,7 +102,7 @@ void ScrollableContainerWidget::set_widget(GUI::Widget* widget) update_widget_position(); } -bool ScrollableContainerWidget::load_from_gml_ast(NonnullRefPtr ast, RefPtr (*unregistered_child_handler)(DeprecatedString const&)) +ErrorOr ScrollableContainerWidget::load_from_gml_ast(NonnullRefPtr ast, RefPtr (*unregistered_child_handler)(DeprecatedString const&)) { if (is(ast.ptr())) return load_from_gml_ast(static_ptr_cast(ast)->main_class(), unregistered_child_handler); @@ -116,15 +116,13 @@ bool ScrollableContainerWidget::load_from_gml_ast(NonnullRefPtr auto content_widget_value = object->get_property("content_widget"sv); if (!content_widget_value.is_null() && !is(content_widget_value.ptr())) { - dbgln("content widget is not an object"); - return false; + return Error::from_string_literal("ScrollableContainerWidget content_widget is not an object"); } auto has_children = false; object->for_each_child_object([&](auto) { has_children = true; }); if (has_children) { - dbgln("children specified for ScrollableContainerWidget, but only 1 widget as content_widget is supported"); - return false; + return Error::from_string_literal("Children specified for ScrollableContainerWidget, but only 1 widget as content_widget is supported"); } if (!content_widget_value.is_null() && is(content_widget_value.ptr())) { @@ -133,19 +131,18 @@ bool ScrollableContainerWidget::load_from_gml_ast(NonnullRefPtr RefPtr child; if (auto* registration = Core::ObjectClassRegistration::find(class_name)) { - child = registration->construct(); + child = TRY(registration->construct()); } else { child = unregistered_child_handler(class_name); } if (!child) - return false; + return Error::from_string_literal("Unable to construct a Widget class for ScrollableContainerWidget content_widget property"); auto widget_ptr = verify_cast(child.ptr()); set_widget(widget_ptr); - static_ptr_cast(child)->load_from_gml_ast(content_widget.release_nonnull(), unregistered_child_handler); - return true; + TRY(static_ptr_cast(child)->load_from_gml_ast(content_widget.release_nonnull(), unregistered_child_handler)); } - return true; + return {}; } } diff --git a/Userland/Libraries/LibGUI/ScrollableContainerWidget.h b/Userland/Libraries/LibGUI/ScrollableContainerWidget.h index c7bb92c167..c57da3f62a 100644 --- a/Userland/Libraries/LibGUI/ScrollableContainerWidget.h +++ b/Userland/Libraries/LibGUI/ScrollableContainerWidget.h @@ -30,7 +30,7 @@ private: void update_widget_size(); void update_widget_position(); void update_widget_min_size(); - virtual bool load_from_gml_ast(NonnullRefPtr ast, RefPtr (*unregistered_child_handler)(DeprecatedString const&)) override; + virtual ErrorOr load_from_gml_ast(NonnullRefPtr ast, RefPtr (*unregistered_child_handler)(DeprecatedString const&)) override; ScrollableContainerWidget(); diff --git a/Userland/Libraries/LibGUI/Widget.cpp b/Userland/Libraries/LibGUI/Widget.cpp index 063f629ae9..434b40bbeb 100644 --- a/Userland/Libraries/LibGUI/Widget.cpp +++ b/Userland/Libraries/LibGUI/Widget.cpp @@ -1145,26 +1145,31 @@ void Widget::set_override_cursor(AK::Variant Widget::try_load_from_gml(StringView gml_string) { - return load_from_gml(gml_string, [](DeprecatedString const& class_name) -> RefPtr { + return try_load_from_gml(gml_string, [](DeprecatedString const& class_name) -> RefPtr { dbgln("Class '{}' not registered", class_name); return nullptr; }); } -bool Widget::load_from_gml(StringView gml_string, RefPtr (*unregistered_child_handler)(DeprecatedString const&)) +ErrorOr Widget::try_load_from_gml(StringView gml_string, RefPtr (*unregistered_child_handler)(DeprecatedString const&)) { - auto value = GML::parse_gml(gml_string); - if (value.is_error()) { - // FIXME: We don't report the error, so at least print it. - dbgln("Error while parsing GML: {}", value.error()); - return false; - } - return load_from_gml_ast(value.release_value(), unregistered_child_handler); + auto value = TRY(GML::parse_gml(gml_string)); + return load_from_gml_ast(value, unregistered_child_handler); } -bool Widget::load_from_gml_ast(NonnullRefPtr ast, RefPtr (*unregistered_child_handler)(DeprecatedString const&)) +bool Widget::load_from_gml(StringView gml_string) +{ + return !try_load_from_gml(gml_string).is_error(); +} + +bool Widget::load_from_gml(StringView gml_string, RefPtr (*unregistered_child_handler)(DeprecatedString const&)) +{ + return !try_load_from_gml(gml_string, unregistered_child_handler).is_error(); +} + +ErrorOr Widget::load_from_gml_ast(NonnullRefPtr ast, RefPtr (*unregistered_child_handler)(DeprecatedString const&)) { if (is(ast.ptr())) return load_from_gml_ast(static_ptr_cast(ast)->main_class(), unregistered_child_handler); @@ -1180,21 +1185,20 @@ bool Widget::load_from_gml_ast(NonnullRefPtr ast, RefPtrname(); if (class_name.is_null()) { - dbgln("Invalid layout class name"); - return false; + return Error::from_string_literal("Invalid layout class name"); } auto& layout_class = *Core::ObjectClassRegistration::find("GUI::Layout"sv); if (auto* registration = Core::ObjectClassRegistration::find(class_name)) { - auto layout = registration->construct(); - if (!layout || !registration->is_derived_from(layout_class)) { + auto layout = TRY(registration->construct()); + if (!registration->is_derived_from(layout_class)) { dbgln("Invalid layout class: '{}'", class_name.to_deprecated_string()); - return false; + return Error::from_string_literal("Invalid layout class"); } - set_layout(static_ptr_cast(layout).release_nonnull()); + set_layout(static_ptr_cast(layout)); } else { dbgln("Unknown layout class: '{}'", class_name.to_deprecated_string()); - return false; + return Error::from_string_literal("Unknown layout class"); } layout->for_each_property([&](auto key, auto value) { @@ -1204,33 +1208,32 @@ bool Widget::load_from_gml_ast(NonnullRefPtr ast, RefPtr(*this); - object->for_each_child_object_interruptible([&](auto child_data) { + TRY(object->try_for_each_child_object([&](auto child_data) -> ErrorOr { auto class_name = child_data->name(); // It is very questionable if this pseudo object should exist, but it works fine like this for now. if (class_name == "GUI::Layout::Spacer") { if (!this->layout()) { - dbgln("Specified GUI::Layout::Spacer in GML, but the parent has no Layout."); - return IterationDecision::Break; + return Error::from_string_literal("Specified GUI::Layout::Spacer in GML, but the parent has no Layout."); } this->layout()->add_spacer(); } else { RefPtr child; if (auto* registration = Core::ObjectClassRegistration::find(class_name)) { - child = registration->construct(); - if (!child || !registration->is_derived_from(widget_class)) { + child = TRY(registration->construct()); + if (!registration->is_derived_from(widget_class)) { dbgln("Invalid widget class: '{}'", class_name); - return IterationDecision::Break; + return Error::from_string_literal("Invalid widget class"); } } else { child = unregistered_child_handler(class_name); } if (!child) - return IterationDecision::Break; + return Error::from_string_literal("Unable to construct a Widget class for child"); add_child(*child); // This is possible as we ensure that Widget is a base class above. - static_ptr_cast(child)->load_from_gml_ast(child_data, unregistered_child_handler); + TRY(static_ptr_cast(child)->load_from_gml_ast(child_data, unregistered_child_handler)); if (is_tab_widget) { // FIXME: We need to have the child added before loading it so that it can access us. But the TabWidget logic requires the child to not be present yet. @@ -1239,10 +1242,10 @@ bool Widget::load_from_gml_ast(NonnullRefPtr ast, RefPtr(namespace_::class_name::construct()); }, ®istration_Widget); \ - } \ +#define REGISTER_WIDGET(namespace_, class_name) \ + namespace Core { \ + namespace Registration { \ + Core::ObjectClassRegistration registration_##class_name( \ + #namespace_ "::" #class_name##sv, []() -> ErrorOr> { return static_ptr_cast(TRY(namespace_::class_name::try_create())); }, ®istration_Widget); \ + } \ } namespace GUI { @@ -353,7 +353,11 @@ public: AK::Variant> const& override_cursor() const { return m_override_cursor; } void set_override_cursor(AK::Variant>); - bool load_from_gml(StringView); + ErrorOr try_load_from_gml(StringView); + ErrorOr try_load_from_gml(StringView, RefPtr (*unregistered_child_handler)(DeprecatedString const&)); + + // FIXME: Replace all uses of load_from_gml() with try_load_from_gml() + bool load_from_gml(StringView gml_string); bool load_from_gml(StringView, RefPtr (*unregistered_child_handler)(DeprecatedString const&)); // FIXME: remove this when all uses of shrink_to_fit are eliminated @@ -363,7 +367,7 @@ public: bool has_pending_drop() const; // In order for others to be able to call this, it needs to be public. - virtual bool load_from_gml_ast(NonnullRefPtr ast, RefPtr (*unregistered_child_handler)(DeprecatedString const&)); + virtual ErrorOr load_from_gml_ast(NonnullRefPtr ast, RefPtr (*unregistered_child_handler)(DeprecatedString const&)); protected: Widget();