- User Since
- Oct 11 2017, 3:04 AM (118 w, 2 d)
Yeah, we should take a look at ecrire, for sure. Unfortunately it uses legacy API (and does not seem to work very well right now).
After reading that guideline, I see that indexers are only discouraged in some cases.
Index through a tuple to fit multiple keys indeed looks very convoluted and should be avoided.
However, I think indexers are still useful in simple cases (like when there is only one key, and it's a string). Can we consider keeping them for these cases?
So let's proceed with the rename and keep the c_prefix.
Yeah, we have to be consistent with whatever we decide in D11108.
API-wise, there's not much more to discuss. Look right.
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.
Yes, this looks OK.
I agree with the rename. This makes these classes consistent with Efl.Accelerate_Interpolator, for example.
There's not much to stabilize here, the API is empty.
I think this is good to go :)
Wed, Jan 15
Builds and passes tests. Thanks for updating the docs too!
This looks like a fun thing to implement! I suggest we use the Text Editor example as a base.
In this way, we can create a cumulative tutorial in the future (that builds on top of the Text Editor tutorial, when we have one).
I just did some tweaks :)
I think so too.
Tue, Jan 14
Looks good to me!
Builds and passes tests.
These names look clear enough to me. The docs need improvement.
For example, what @ali.alzyod wrote in the task description could be used directly as docs.
I still don't know how factories work, I cannot help much here.
If you say it is not urgent, we can keep it beta, sure.
Yeah, Efl.Ui.Range_Display is totally unrelated, and I don't think we have any other class that serves this purpose.
Your comment does not help the project in any way.
Please bear in mind that very few of us are native English speakers and we all have to make an effort to understand each other.
Typos in code or documentation should be mercilessly pointed out and corrected. There's no point in bringing up typos in phabricator discussions, though. We're all bound to make them!
This enum has already been reviewed but I think it is still not consistent with the rest of the API.
We should not use shortened words: char->character, prev->previous, etc.
Mon, Jan 13
Still failing for me, same place :(
../src/tests/elementary/efl_ui_test_spin_button.c:222:E:efl_ui_spin_button:spin_direct_text_input:0: (after this point) Received signal 11 (Segmentation fault) ../src/tests/elementary/efl_ui_test_spin_button.c:222:E:efl_ui_spin_button:spin_direct_text_input_comma_value:0: (after this point) Received signal 11 (Segmentation fault)
Docs and new events look OK to me.
I'm OK with stating in the video docs that autorepeat is ignored. Then again, it does not look very hard to implement either...
Looks good to me.
Builds and passes tests.
I'm not talking about Efl.Canvas.Gesture. This one is OK. I'm talking about the derived classes: Efl.Canvas.Gesture_Flick, Efl.Canvas.Gesture_Zoom, etc.
Some of these derived classes have methods called *_get, which look to me like they should be readonly properties.
Makes a lot of sense to me.
Builds and passes tests.
For the future, "legacy cleanup" sounds like you are cleaning legacy stuff (like removing methods, for example). A more fitting title would be "Use Unified API instead of Legacy".
It's just a minor detail, but it helps when browsing lots of commits and you only have time to read the summary :)
Can't see anything wrong with this, right?
We're still discussing if some of these properties (like looping) belong here or to the Player interface, right?
I would close this task and create new ones if we need more renames (I DO think we need more renames, but that's being discussed somewhere else).
We should fix this!
We can discuss if the properties and the attributes should have the same name, but the values they accept should be absolutely the same!
I think the correct ones are the ones in the enums, and we should modify the ones in the attributes (and the style_apply docs).
Thanks for pointing this out, @Jaehyun_Cho!
Sun, Jan 12
Sat, Jan 11
I am not sure what are you trying to accomplish here.
Almost all EO properties related to text style are available as format strings:
For example, you have efl_text_strikethrough_type_set(obj, EFL_TEXT_STYLE_STRIKETHROUGH_TYPE_SINGLE) and also efl_canvas_textblock_style_apply(obj, "strikethrough=single").
I agree the names are sometimes slightly different. But I like it this way, because the C API is meant to be concise (In the above example, "this is the type of the strikethrough decoration") whereas the format string is meant to be short ("strikethrough=single" is short and clear).
Adding both options to the formatting string I think will be confusing... We would need to document both!
If you think the slightly different names is a problem, then I would prefer we use the exact same names in both APIs.
Fri, Jan 10
Just for the record, I think that feature is in use in Ecrire, Edi and Enlightenment.
These apps will have to work a bit more to be ported to the Unified interface if we drop this feature.
But OK, we can do it later.
Regarding possible autoplay - autorepeat confusions:
I think autoplay is clear enough, and a common way of referring to this feature.
We can rename autorepeat to looping or repeating. This does not clash with anything else in our current API.
Thu, Jan 9
The backing inconsistency is important... raising priority again :)
I still don't know why you're removing the File interface from Textbox.
I never liked this feature very much, but it was already there, so I guess somebody liked it.
Did anybody ask for this removal?
Not a method, but a property, yeah.
I don't like but I can't think of anything else I like more :/
I think the remaining issues do not affect the API (they are implementation or documentation issues, or the API can be extended later). Therefore I am lowering the priority of this.
@woohyun, out of curiosity, what are the plans for Access? Is there anybody working on it?
Good for me!
Hmmmm... adding all variations to Efl.Input_Text.Panel_Layout_Type is an option, but it has some problems.
- Its size will increase, as you say.
- It will be harder to detect the "base" type of a layout, since it will involve a lot of if's.
- We could alleviate the above problem using the lower bits of the enum to store the "base" type (same as it is now), and use the higher bits to store the variation:
This commit also changes the way _format_string_get works, and this should be explained in the commit message.
This makes ninja test fail in src/tests/evas/evas_test_textblock.c:4899.
Please always run ninja test before submitting patches.
Wed, Jan 8
Hmmm... yeah, I think I like opposite.
It feels weird tu remove functionality that was in legacy. I would keep input_hint as Beta and rename it to input_content_type, but not remove it.
We better create a task to rename this struct before we forget again and stabilize it :)
This struct is the only thing in the Efl.Gfx.Event namespace... looks weird.
One last comment: I would rename selection_handler_enabled to selection_handles_enabled. I think we should be talking about selection handles (a graphical gadget that helps you select text) and not selection handlers (an abstract "handler" that nobody knows what it does).
There are still some open issues here. Are there any plans to work on them?
I don't think we can stabilize style_apply() if we don't fix some of these issues.
At least I would like to have the multi-word properties separated with underscores.
I think so.
Hmmm... good point. On one hand, I want to have the option to add "end locale" later on, but on the other hand, the most common options are probably auto and end, so they should be kept short and simple.
It's hard for me to decide, since I never used LTR/RTL settings. Do we have anybody with more knowledge on this?
I am tempted to keep end and use locale_end in the future if we need the "inverted locale" option.
Why are these flags? They look like they can be combined, however, most combinations do not make sense.
I guess that "none", "auto_complete", and "sensitive_data" can be combined with the rest. So, the numbers are set with current definition.
According to the EO docs, sensitive_data means that "Typed text should not be stored". Therefore I don't think it makes sense to combine with any autofill option, no? Because, to populate the autofill database you need to store something.
And auto_complete is implied by any of the autofill_* options, no?
This is why I don't understand why this enum is made of combinable flags. I think a regular enum with none, credit_card_expiration_date, etc... is enough.
Tue, Jan 7
So what do we decide?
I already said that providing the event info directly as parameters was nice, but it is true that this breaks ABI. Therefore, I am OK with leaving things as they are now, with EventArgs.
Why are these interfaces being removed? I see a lot of code there... @woohyun, is Access not needed anymore?
This patch comes from the discussion in D10973#208299. The commit message should explain WHY this patch is needed, besides WHAT it does.
This is how I always expected step to work.
Would it be a lot of work to make the sliders behave like this?
Removing a feature does not feel right. I would prefer we added the missing "inverted locale" setting.
But if that's too much work, we can wait until somebody actually requests it, as @ali.alzyod suggested.
D10993 renames normal to auto and adds some more docs (that patch still needs review, btw).
It does not remove end.
Works as advertised, warnings are fixed. Thanks!
I'll update the docs later on.
API and names look good to me. I'll take care of the docs later on.
OK, so, if in the future we want to support scaling of monochrome fonts, we add another enum value monochrome, for example.
Oh, the EO file already explained that. This task description was outdated :/