Skip to content
This repository has been archived by the owner on May 25, 2020. It is now read-only.

Efl.Ui.Format revamp #18

Merged
merged 1 commit into from
Jul 4, 2019
Merged

Conversation

segfaultxavi
Copy link
Collaborator

This class helps widgets which contain a numerical value and must display it,
like Progressbar (units label), Spin, Spin_Button, Slider (both units and popup
labels, in legacy), Tags (when in shrunk mode) or Calendar (year_month label).

Previously this was a mix of interface and mixin: widgets had to support setting a
formatting func, and the mixin offered support for formatting strings, by setting
an internal formatting func. On top of that, the spinner widget supported "special
values", a list of values that should be shown as certain strings instead.

This has now been simplified and unified:
Widgets including this mixin can use the formatted_value_get() method which accepts
an Eina_Value and returns a string. Thats's it.
The mixin adds three properties to the widget (format_values, format_func and
format_string) which users can use to tailor formatting. The widget does not need
to know which method has been used, it just retrieves the resulting string.
This removes a lot of duplicated widget code, and adds functionality which was
missing before. For example, all widgets support passing a list of values now.

Widgets must implement the apply_formatted_value() method so they are notified
of changes in the format and they can redraw anything they need.

Tests have been added to the Elementary Spec suite for all cases.

Legacy widgets behavior has not been modified, although a few needed some code
changes.

Copy link
Collaborator

@Bluebugs Bluebugs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few nitpick, overall concept is great improvement!

src/bin/elementary/test_ui_spin_button.c Outdated Show resolved Hide resolved
src/bin/elementary/test_ui_spin_button.c Show resolved Hide resolved
src/lib/elementary/efl_ui_format.c Outdated Show resolved Hide resolved
src/lib/elementary/efl_ui_format.c Outdated Show resolved Hide resolved
src/lib/elementary/efl_ui_format.eo Show resolved Hide resolved
src/tests/elementary/spec/efl_test_format.c Outdated Show resolved Hide resolved
src/tests/elementary/spec/efl_test_format.c Show resolved Hide resolved
src/tests/elementary/spec/efl_test_format.c Outdated Show resolved Hide resolved
src/tests/elementary/spec/efl_test_format.c Show resolved Hide resolved
src/tests/elementary/spec/efl_test_format.c Outdated Show resolved Hide resolved
This class helps widgets which contain a numerical value and must display it,
like Progressbar (units label), Spin, Spin_Button, Slider (both units and popup
labels, in legacy), Tags (when in shrunk mode) or Calendar (year_month label).

Previously this was a mix of interface and mixin: widgets had to support setting a
formatting func, and the mixin offered support for formatting strings, by setting
an internal formatting func. On top of that, the spinner widget supported "special
values", a list of values that should be shown as certain strings instead.

This has now been simplified and unified:
Widgets including this mixin can use the formatted_value_get() method which accepts
an Eina_Value and returns a string. Thats's it.
The mixin adds three properties to the widget (format_values, format_func and
format_string) which users can use to tailor formatting. The widget does not need
to know which method has been used, it just retrieves the resulting string.
This removes a lot of duplicated widget code, and adds functionality which was
missing before. For example, all widgets support passing a list of values now.

Widgets must implement the apply_formatted_value() method so they are notified
of changes in the format and they can redraw anything they need.

Tests have been added to the Elementary Spec suite for all cases.

Legacy widgets behavior has not been modified, although a few needed some code
changes.
@segfaultxavi segfaultxavi force-pushed the devs/xartigas/efl_ui_format_revamp branch from a885932 to 3a1a405 Compare July 4, 2019 14:43
@segfaultxavi
Copy link
Collaborator Author

Eina_Inarray is now used for the internal list of format values.
The eina_value helpers are used to simplify the code.
The widget is not requested to redraw if it is being destroyed (avoids unnecessary work and in one case a leak).

@segfaultxavi segfaultxavi requested a review from Bluebugs July 4, 2019 14:47
@segfaultxavi segfaultxavi merged commit e776f5f into master Jul 4, 2019
@segfaultxavi segfaultxavi deleted the devs/xartigas/efl_ui_format_revamp branch July 4, 2019 17:40
lauromoura pushed a commit to lauromoura/efl-tmp that referenced this pull request Nov 26, 2019
This reverts commit 2f676a6.

This causes segv's in edje_cc - i suspect the eet changes (or in combo
to how they are used in edje):

AddressSanitizer:DEADLYSIGNAL
=================================================================
==8991==ERROR: AddressSanitizer: SEGV on unknown address 0x000001010000 (pc 0xffff9f002604 bp 0xfffffa747700 sp 0xfffffa747700 T0)
==8991==The signal is caused by a READ memory access.
    #0 0xffff9f002600 in _eet_hash_gen ../src/lib/eet/eet_utils.c:25
    Enlightenment#1 0xffff9efdd024 in eet_dictionary_string_add ../src/lib/eet/eet_dictionary.c:103
    Enlightenment#2 0xffff9efbe324 in eet_data_put_string ../src/lib/eet/eet_data.c:849
    Enlightenment#3 0xffff9efc1c4c in eet_data_put_type ../src/lib/eet/eet_data.c:1427
    Enlightenment#4 0xffff9efd9128 in eet_data_put_unknown ../src/lib/eet/eet_data.c:4730
    Enlightenment#5 0xffff9efdb320 in _eet_data_descriptor_encode ../src/lib/eet/eet_data.c:5108
    Enlightenment#6 0xffff9efd5958 in eet_data_put_variant ../src/lib/eet/eet_data.c:4309
    Enlightenment#7 0xffff9efdb320 in _eet_data_descriptor_encode ../src/lib/eet/eet_data.c:5108
    Enlightenment#8 0xffff9efd9270 in eet_data_put_unknown ../src/lib/eet/eet_data.c:4739
    Enlightenment#9 0xffff9efdb320 in _eet_data_descriptor_encode ../src/lib/eet/eet_data.c:5108
    Enlightenment#10 0xffff9efd8ca0 in eet_data_put_array ../src/lib/eet/eet_data.c:4692
    Enlightenment#11 0xffff9efdb320 in _eet_data_descriptor_encode ../src/lib/eet/eet_data.c:5108
    Enlightenment#12 0xffff9efc7768 in eet_data_write_cipher ../src/lib/eet/eet_data.c:2403
    Enlightenment#13 0xffff9efc78a4 in eet_data_write ../src/lib/eet/eet_data.c:2420
    Enlightenment#14 0xaaaabb151dcc in data_thread_group ../src/bin/edje/edje_cc_out.c:2045
    Enlightenment#15 0xaaaabb152130 in data_write_groups ../src/bin/edje/edje_cc_out.c:2086
    Enlightenment#16 0xaaaabb157734 in data_write ../src/bin/edje/edje_cc_out.c:2866
    Enlightenment#17 0xaaaabb14122c in main ../src/bin/edje/edje_cc.c:456
    Enlightenment#18 0xffff9dbd92a0 in __libc_start_main (/usr/lib/aarch64-linux-gnu/libc.so.6+0x242a0)
    Enlightenment#19 0xaaaabb13ea00  (/home/raster/C/git/efl/build/src/bin/edje/edje_cc+0x38a00)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV ../src/lib/eet/eet_utils.c:25 in _eet_hash_gen
==8991==ABORTING
Aborted (core dumped)

When compiling breaks... it's certainly time to revert ASAP :(
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants