Page MenuHomePhabricator

csharp: Components enum are flags now.
ClosedPublic

Authored by brunobelo on Oct 28 2019, 7:52 AM.

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
brunobelo created this revision.Oct 28 2019, 7:52 AM
brunobelo requested review of this revision.Oct 28 2019, 7:52 AM

At first I also thought about suppressing but maybe for this specific case instead of suppressing we could change the Components enum to Configuration (Efl.All.Init(Efl.Csharp.Configuration.Ui)).

In the case of Csharp.Application: app.Launch(Efl.Csharp.Configuration.Basic)

What o you think @segfaultxavi? (summoning you as this is an API issue)

Right now we only have two components, but we might have more in the future.
In that case, I think it will be more useful to turn this enum into flags, so components can be selected individually.
Incidentally, that fixes the issue of the plural name :)

lauromoura requested changes to this revision.EditedNov 1 2019, 7:32 AM

Right now we only have two components, but we might have more in the future.
In that case, I think it will be more useful to turn this enum into flags, so components can be selected individually.
Incidentally, that fixes the issue of the plural name :)

Makes sense, so it would be a matter of using Flags[1] and adapting the usage (replacing == with &, etc). @brunobelo , ok?

We could start defining it to be something like:

[Flags]
public enum Components
{
    Basic = 0x1,
    Ui = Basic | 0x2
    All = UI,
}

And the usage in Init:

Efl.All.Init(Efl.Csharp.Components.All);`

[1] https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/enumeration-types#enumeration-types-as-bit-flags

This revision now requires changes to proceed.Nov 1 2019, 7:32 AM
brunobelo planned changes to this revision.Nov 1 2019, 9:26 AM
brunobelo retitled this revision from csharp: Suppress Warning for Components enum. to csharp: Components enum are flags now..Nov 4 2019, 12:27 PM
brunobelo updated this revision to Diff 26672.Nov 4 2019, 12:48 PM

whitespaces

lauromoura requested changes to this revision.Nov 4 2019, 7:04 PM
lauromoura added inline comments.
src/bindings/mono/efl_mono/efl_csharp_application.cs
47

Letting Elementary be used alone can be confusing as there is a hard dependency between it and the basic modules.

The user could do something like Init(Components.Elementary) which would try to initialize C#'s Elm without initializing the other basic modules.

Like a previous comment, Ui = Basic | 0x2 makes sure Ui requires the Basic stuff while adding a new bit.

This revision now requires changes to proceed.Nov 4 2019, 7:04 PM
segfaultxavi added inline comments.Nov 5 2019, 1:41 AM
src/bindings/mono/efl_mono/efl_csharp_application.cs
47

Also, we should try not to use Legacy library names anywhere in the Unified API. There's no Elementary now, only Efl.Ui.

lauromoura accepted this revision.Wed, Nov 20, 2:13 PM
This revision is now accepted and ready to land.Wed, Nov 20, 2:13 PM
This revision was automatically updated to reflect the committed changes.