-
-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor impl Default
generation
#1446
Conversation
I know a gir rework is ongoing, so I am not sure if refactoring PRs makes sense. The reason for this is to tackle gtk-rs/gtk-rs-core#975 by adding a new |
gtk3 fails because it has
which is now generated... on the one hand I guess it is a good thing we can drop manual code, on the other it is not clear to me why this was not happening before :) |
let name = &analysis.name; | ||
if let Some(func) = analysis.default_constructor() { | ||
general::declare_default(w, env, name, func) | ||
} else if has_builder_properties(&analysis.builder_properties) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why with this patch RadioButton etc get picked up is because before we were falling back to object::new only if there was a new()
function, but it did not have 0 params.
That seems rather odd and with the new code I did not get any false positive in core and gtk3
Move the detection of a default constructor function in the analysis. Move the logic about using Object::new into object.
// For now we only look for new() with no params | ||
&& f.name == "new" | ||
&& f.parameters.rust_parameters.is_empty() | ||
// Cannot generate Default implementation for Option<> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should also do the same for Result?
Would be nice to land this. But it needs PRs in at least 2 out of gtk-rs-core/gstreamer-rs/gtk4-rs with the resulting changes. |
I looked at this again, and I do not think the PR is correct: the old code generated the fallback To make it correct we would have to remove the "0 args" check in the generic code, but that would defeat the point of centralizing this logic. |
Move the detection of a default constructor function in the analysis. Move the logic about using Object::new into object.