LibGUI: Introduce property deserializers to catch errors in GML property values
Yes, I know, I am generally not really interested in Serenity-specific stuff. And yes, I know, I was porting DeprecatedString to StringBase. But suddenly task stack became really deep:
- port
DeprecatedStringtoStringBase - remove references to
StringImpl - port
JsonValuetoVariantandStringBase - remove odd
T JsonValue::to_something(T default_value)API - make LibGUI not use this API
- do something so that GMLPlayground will not instantly crash on invalid input
Property deserializer is a small class which converts JsonValue to a required type while doing robust error checking at the same time. And previously that error checking in deserializing was basically "if something is supposed to be int but is not, a developer definitely wanted 0 there". What's more, if someone addresses one new error propagation FIXME I left, we will be able to show nice errors in GMLPlayground in real time.
I am aware of existence of GMLCompiler but AFAIC Film's intention is to continue using JSON input in GMLPlayground.
CC @kleinesfilmroellchen
Also, logging for discarded errors tells me that somewhere in DisplaySettings there is an invalid text_alignment value :)
Also, logging for discarded errors tells me that somewhere in DisplaySettings there is an invalid text_alignment value :)
Never used gml/gui apps but maybe this is wrong?
MonitorSettings.gml:
text_alignment: "CenterMiddle"
Maybe it should be Center?
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!
Not stale
Now with 100% less short-lived macros!
Sorry for taking so long on this! I like this change.
After testing it, I found a simple crash in GMLPlayground with invalid or in-progress property values.
For example:
@GUI::Widget {
fixed_height: -1
}
will crash when typing the 1 with
125.446 GMLPlayground(44): VERIFICATION FAILED: value >= 0 at Userland/Libraries/LibGUI/UIDimensions.h:38
...
126.959 CrashReporter(45:46): 0x00000017a941bb0c: [/usr/lib/libc.so] ak_verification_failed +0x2c (Assertions.cpp:108)
126.959 CrashReporter(45:46): 0x00000017e7ba0ed9: [/usr/lib/libgui.so.serenity] AK::Function<bool (AK::JsonValue const&)>::CallableWrapper<void GUI::Object::register_property<GUI::Widget::Widget()::$_51, GUI::PropertyDeserializer<int>, GUI::Widget::Widget()::$_52>(AK::StringView, GUI::Widget::Widget()::$_51&&, GUI::PropertyDeserializer<int>&&, GUI::Widget::Widget()::$_52&&)::'lambda'(AK::JsonValue const&)>::call(AK::JsonValue const&) +0x59 (UIDimensions.h:38 => Widget.cpp:77 => Object.h:90 => Function.h:182)
126.959 CrashReporter(45:46): 0x00000017e7b01ab8: [/usr/lib/libgui.so.serenity] AK::Function<bool (AK::JsonValue const&)>::operator()(AK::JsonValue const&) const +0x38 (Function.h:115)
126.963 CrashReporter(45:46): 0x00000017e7b9cacf: [/usr/lib/libgui.so.serenity] GUI::Widget::load_from_gml_ast(AK::NonnullRefPtr<GUI::GML::Node const>, AK::ErrorOr<AK::NonnullRefPtr<Core::EventReceiver>, AK::Error> (*)(AK::StringView)) +0x35f (Widget.cpp:1101 => AST.h:186 => Widget.cpp:1100)
126.963 CrashReporter(45:46): 0x00000017e7b9d389: [/usr/lib/libgui.so.serenity] GUI::Widget::load_from_gml_ast(AK::NonnullRefPtr<GUI::GML::Node const>, AK::ErrorOr<AK::NonnullRefPtr<Core::EventReceiver>, AK::Error> (*)(AK::StringView)) +0xc19 (AST.h:222 => Widget.cpp:1131)
126.963 CrashReporter(45:46): 0x00000017e7b9c829: [/usr/lib/libgui.so.serenity] GUI::Widget::load_from_gml_ast(AK::NonnullRefPtr<GUI::GML::Node const>, AK::ErrorOr<AK::NonnullRefPtr<Core::EventReceiver>, AK::Error> (*)(AK::StringView)) +0xb9 (Widget.cpp:1095)
126.966 CrashReporter(45:46): 0x00000017e7b9c6a4: [/usr/lib/libgui.so.serenity] GUI::Widget::load_from_gml(AK::StringView, AK::ErrorOr<AK::NonnullRefPtr<Core::EventReceiver>, AK::Error> (*)(AK::StringView)) +0xa4 (Widget.cpp:1089)
126.966 CrashReporter(45:46): 0x0000000c92a757b7: [/bin/GMLPlayground] AK::Function<void ()>::CallableWrapper<MainWidget::try_create(GUI::Icon const&)::$_0>::call() +0x77 (MainWidget.cpp:93 => Function.h:182)
126.966 CrashReporter(45:46): 0x00000017e79f1dd3: [/usr/lib/libgui.so.serenity] AK::Function<void ()>::operator()() const +0x33 (Function.h:115)
126.966 CrashReporter(45:46): 0x00000017e7b55657: [/usr/lib/libgui.so.serenity] non-virtual thunk to GUI::TextEditor::document_did_change(GUI::AllowCallback) +0x17 (TextEditor.cpp:2282 => TextEditor.cpp:0)
126.966 CrashReporter(45:46): 0x00000017e7b3d8bb: [/usr/lib/libgui.so.serenity] GUI::TextDocument::update_views(AK::Badge<Syntax::TextDocumentLine>) +0x5b (TextDocument.cpp:173 => TextDocument.cpp:166)
... but seeing as that crash exists in master too, it seems like this is strictly an improvement :sweat_smile: