Page MenuHomePhabricator

eolian-mono: Make Get/Set protected for generated properties
Needs ReviewPublic

Authored by felipealmeida on Tue, Jan 7, 1:46 PM.

Details

Summary

Make Get and Set methods protected for properties that get the
property syntax generated.

This is still a work in progress and fails compilation. Native calls
should use the property-syntax instead of the method, which may not be
available in some interfaces.

Diff Detail

Repository
rEFL core/efl
Branch
devs/felipealmeida/hide-properties-rebase
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 15465
felipealmeida created this revision.Tue, Jan 7, 1:46 PM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

felipealmeida requested review of this revision.Tue, Jan 7, 1:46 PM
YOhoho added a comment.EditedTue, Jan 7, 9:10 PM

What kind of Get and Set methods become protected?
When i checked Efl.Ui.Widget(efl_ui_widget.eo.cs), Get and Set methods of its property is still public.
For example,

public virtual bool GetFocusAllow() {
public virtual void SetFocusAllow(bool can_focus) {
public virtual bool GetDisabled() {
public virtual void SetDisabled(bool disabled) {

+
Why protected instead of internal or private?
because all properties are already virtual, it can be internal or private to avoid confusion as to what to override between method and property.

YOhoho requested changes to this revision.Tue, Jan 7, 10:39 PM
YOhoho added inline comments.
src/bin/eolian_mono/eolian/mono/klass.hh
464

This implementable_properties from get_all_implementable_properties include unnecessary properties for registration, which means it generate duplicated codes.
For example,
The following code should be only in efl_ui_single_selectable.eo.cs.

private static Efl.Ui.ISelectable last_selected_get(System.IntPtr obj, System.IntPtr pd)
{
...
}

I think get_all_implementable_properties can be replaced with get_all_registerable_properties to prevent problem above.

This revision now requires changes to proceed.Tue, Jan 7, 10:39 PM

Why last_selected_get should only be in SelectableConcrete? Shouldn't this native method be defined in inherits too? I don't think normal classes inherit from Concrete ones. Concrete classes are meant to be used when we find a interface-returning-method that which derived class C# doesn't know anything about.

Some methods were being made public unnecessarily. Made them all internal instead.

YOhoho added a comment.Thu, Jan 9, 8:23 PM

Why last_selected_get should only be in SelectableConcrete? Shouldn't this native method be defined in inherits too? I don't think normal classes inherit from Concrete ones. Concrete classes are meant to be used when we find a interface-returning-method that which derived class C# doesn't know anything about.

Yes, normal classes don't inherit from Concrete ones. however, GetEoOps of Concrete NativeMethods be called by generated code(see, line 521 - 532 in klasss.hh). so, we don't need duplicated functions like last_selected_get.

src/bin/eolian_mono/eolian/mono/klass.hh
528

here is line 521.

Merged with D11056 and small fixes