- User Since
- Dec 31 2013, 12:06 AM (258 w, 4 d)
Wed, Dec 5
I am not sure about the third point:
To improve the above situation, [...] so the expensive properties can be set only once [...]
Mon, Dec 3
Just adding that the constructors block should be used to not only ALLOW (i.e. "optional" methods) setting the state of the object, but also REQUIRE setting its state.
Wed, Nov 28
I will mark so we can further discuss this before landing.
Please give me today to just see that everything is in order.
This is a proper fix to T7468, and I intended to fix it similarly.
Mon, Nov 26
See last comment.
We've discussed this a bit on IRC and on another ticket, so I will add a bit of input here in addition to closing this:
Mon, Nov 19
Continuing our conversation on IRC, I will express my opinions here:
Sun, Nov 18
Sat, Nov 17
Thanks for reporting.
I will work on resolving this regression.
Fri, Nov 16
Nov 14 2018
Hi! Thanks for the patch. I have some time today to review.
Nov 8 2018
Oh, OK. Let me check.
Thanks for reporting.
Sep 17 2018
Yes, this was meant for efl-1.22. Didn't want to disrupt all the work for efl-1.21.
Jul 17 2018
Is there a particular reason we have to iterate to the very top parent instead of using elm_widget_top_get?
I wonder if this never worked, or something that's been recently changed.
Jul 16 2018
Okay, I feel less concerned now. I assumed the treatment is the same as it was to a signed char.
However, upon further testing, the bitfield bool value was converted from the stored 1 to 1 (or: true), as opposed to -1, (which I expected it to be).
Looks like C99 adds an implicit conversion for bool types.
Fwiw, please consider that for bitfields of one bit (e.g. bool changed: 1;) the values will be 0 and -1 due to bool being a signed char. If we ensure consistency on usage it shouldn't be an issue.
Never said it was okay.
Anyway, if you believe this need to be fixed at some point, please open a ticket.
We can decide on the priority for win32 builds.
Got raster here to shoot down the bad bugs :)
Jul 15 2018
I too was concerned about it. However, efl_add and efl_add_ref are both implemented in the same manner.
I thought about it, but ended up preferring the explicit solution. I briefly scanned and didn't see EINA_MAGIC_CHECK used as a null-check replacement.
Jul 12 2018
This appears to be a duplicate of D6574. Was it abandoned?
pd->smanager, pd->box and pd->pan are bound to the object's lifecycle, so I would suggest to not explicitly efl_del those in the invalidate() call.
From what I understand, the invalidate() is meant for cases where you have to have access to the child before it gets deleted (which now occurs before the parent's destructor).
Oh, right. That is unfortunate.
Jul 10 2018
May I ask to reword so that it says "Focus Composition" and not just "Composition"?
I was rather confused.
Jul 9 2018
May I ask to provide a version in which those types were available?
I would like to verify that they are indeed "missing" now.
Jul 3 2018
@zmike, I have done a bunch of testing and I believe it will be a great addition to the upcoming release.
Asking your permission to merge this feature to master.
I added a few inline comments.
So far so good, but I would like to ask it to be its own test, as there is still work to be done (i.e. have the change not being global, but per object).
Also, I would rather see such test demonstrate more color classes other than "entry text", for demonstration and coverage purposes.
Jul 2 2018
Just a heads up as this patch is going through another round of review by me.
I will provide my insight (and hopefully accept) ASAP.
Alright. Good patch!
This will be followed by D3707.
Jun 28 2018
This will be pushed after D3703
I will simplify this line for you, if you don't mind. :)
The discussion here was a bit confusing.
So to sum up: D3707 introduced the infinite loop bug. OK.
This patch fixes it.
@bu5hm4n, I tend to agree with you.
@charlesmilette, also I would like to verify that you are testing efl-git and not v1.20.
@charlesmilette, thanks for the input.
Unfortunately, I am still unable to reproduce.
I have tried "Fire Mono" and the font in terminology, and did the "curl" command. Everything seems to be intact.
Let's try additional steps, if you don't mind.
Jun 27 2018
If anyone can provide a test scenario for this, please do.
I can't reproduce this.
Jun 22 2018
Resolved a long time ago.
Here is a quick test program for RTL (see attached file):
$ edje_cc label_text.edc $ edje_player label_text.edj
Pushed. Also, fixed some grammar mistakes, and also removed the local fix. New test still passes.
Absolutely good point: additional API should also include tests (even basic ones like in this case), to easily spot regressions.
Jun 21 2018
Since 2 can never occur, the following lines will be dead code.
Jun 20 2018
This patch doesn't describe an issue that was fixed. Why the @fix tag, then?
Nothing here ATM.
I am running the equivalent:
while true; do ./tests/evas/evas_suite "Object Textblock" || break; done
Jun 12 2018
Jun 11 2018
Resolved this one a while ago.
Thank you :)
Jun 8 2018
Shouldn't @commiters be subscribed here?
Jun 7 2018
I noticed that "recalc_apply" is called when the !ed->calc_only condition occurs.
Do you have any information what the !calc_only is used for, if you happened to investigate on those tricky spots.
I am worried that removing this call from when edje wants to "recalc" will lead to unwanted behavior.
This is still under review, as I want to have a decent method to measure the performance gain/hit here.
That is unfortunate.
As a (hopefully) temporary solution, please add a FIXME that describes the problem with how the paragraph direction update happens.
After I push and read this the issue, we will formulate a plan to somehow fix that.
In general, FIXMEs are easier to find in code.
Jun 6 2018
This has been in the discussion in the last few days.
I believe this ticket needs to elaborate what the actual issues are, specifically this:
Problem is, a lot of workarounds have been added to Eolian in order to accommodate for all the quirks in the Legacy API, which are impairing the bindings work. It would be far cleaner and faster if the Legacy API went back to C land and the EO files were used exclusively for the EFL API.
OK. As I wasn't able to reproduce this, may I ask for your build options ("configure" flags).
I tried disabling harfbuzz, but tests still pass.
Jun 5 2018
I will investigate this tomorrow.
Is this something recent, or has been like that for a while?
Jun 4 2018
May 29 2018
Sorry it took me some time to give feedback, as this is quite a large patch and I have been busy with other reviews and work.
May 17 2018
May 16 2018
Rebased and approved.
Also, please note that I remove the changes made to START_TEST(efl_canvas_text_cursor), as they seem unrelated to the commit message.
Thank you! Much appreciated.
Rebased to master.
May 14 2018
Agreed. No need for extra checks in non-legacy code path.
I dislike the inline comments, though. We don't need those.
Please remove the comments and I'll then push this patch.
May 10 2018
I think that we can actually save code by removing lines 6289 to 6299 of the lines in edje_object_part_text_get and simply call _edje_efl_text_text_get with the original given ed.
The warning should be moved to the latter function, too.
Unless, I am missing something that might break functionality.
May 7 2018
Performance improvement is always welcome! :)