Page MenuHomePhabricator

Eina_Range
Closed, ResolvedPublic

Description

struct @extern @beta Eina.Range {

[[A range sequence of values.]]
start: size; [[Start of the range.]]
length: size; [[Length of the range.]]

}

ali.alzyod moved this task from Backlog to Evaluating on the efl: api board.

If we have other range structure to replace this one, it is ok too

@segfaultxavi

I don't think Efl.UI.RangeDisplay is not proper for this case.
Do you have another recommendation ?
Or, just keeping current definition would be better ?

I like having generic data type for range (two int value), which can be used in other places.

Yeah, Efl.Ui.Range_Display is totally unrelated, and I don't think we have any other class that serves this purpose.

Therefore, I am OK with keeping this class (even though the docs could be improved, as usual).

ali.alzyod added a comment.EditedJan 14 2020, 3:30 AM

@segfaultxavi Do you like make this structure more generic like Efl.Int_Range, where it can be used by other types other than text, or make it more precise is a better idea ?

@cedric there was some plan for a range because of MVVM, this looks simular, does that match ?

If we turn this into a generic int range struct, its proper place would probably be Eina.
I also want to hear @cedric's opinion since he knows more Eina.

Hum, it seems we are getting to a logical point where indeed range is a useful structure to add to Eina.

I am unlikely to have the time to take care of that, but I should be able to review the patch in the next 2 weeks.

In this case, I'm OK with moving this to Eina.Int_Range.

And then, we will need to review the whole EFL API and replace all ranges with this new class (where it makes sense).

@bu5hm4n @segfaultxavi @cedric

Are you ok with Eina_Int_Rage (in D11128) ?
If we have a plan to add Eina_Doulbe_Range, and so on ~ I think this idea looks good.

I am not convinced of the need for double, otherwise yes.

ali.alzyod added a comment.EditedJan 19 2020, 11:41 PM

I think we should call this new type Eina.Range instead of Eina.Int_Range which is the default range (and I agree with what @cedric says, I think there should not be more ranges other than this one)

If we don't have a plan to extend Eina_XXX_Range later (for other data types),
then I also agree that we need to call this as "Eina.Range" (instead of "Eina.Int.Range").

ali.alzyod renamed this task from Efl.Text_Range to Eina_Range.Jan 20 2020, 3:45 AM
ali.alzyod updated the task description. (Show Details)

Can we move this into stabilization ? I think everybody already likes the idea

Can we use this new type in more places in the current API, so we get a feeling of how usable this is?
I do not feel comfortable stabilizing this when it only has one use case...

ali.alzyod added a comment.EditedJan 23 2020, 3:14 AM

I thought everyone likes the Idea for using more generic type for range. even If one class using it, the other option is that this class/property will have its own unique type (which is against the first assumption for using generic type).

From my side, I will use it for new classes/interfaces if needed, and others need to check their beta classes if it needs such type.

No, no, I am convinced everybody likes this idea. I just wanted to test the current API a bit more, by using it in other places, before declaring it stable.
It's just a thought, nothing critical.

others need to check their beta classes if it needs such type.

Lots of EFL classes do not have an "owner" (because the original writer left, for example). Who would check those?

ali.alzyod added a comment.EditedJan 23 2020, 8:21 AM

Lots of EFL classes do not have an "owner" (because the original writer left, for example). Who would check those?

I think if any beta class is going to stabilize, we can recommend using this new structure. for released classes it is too late unless we do not care about compatibility support.

I think this is done and we can continue here.?

@bu5hm4n
Can I know what we need to continue.. here ? (Sorry for not understanding well)

My comment was related to what was said above. To be more explicit:

  • We are done here, Eina_Range is there
  • When we find another usecase before removing @beta we can add it there

@bu5hm4n
Oh - thanks for explanation :)

bu5hm4n moved this task from Evaluating to Stabilized on the efl: api board.Jan 31 2020, 7:12 AM