Page MenuHomePhabricator

efl.canvas.textblock: update style strings
ClosedPublic

Authored by ali.alzyod on Jan 24 2020, 11:45 AM.

Details

Summary

Update

backing -> background_type
backing_color -> background_color
underline_dash_color -> underline_dashed_color
underline - > underline_type
strikethrough - > strikethrough_type
style -> (effect_type + shadow_direction)
underline_dash_width -> underline_dashed_width
underline_dashed_gap -> underline_dashed_gap

+prevent unified APIs from supporting legacy style tags, and prevent legacy APIs from the ability to use new unified tags

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.
ali.alzyod created this revision.Jan 24 2020, 11:45 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

ali.alzyod requested review of this revision.Jan 24 2020, 11:45 AM
ali.alzyod updated this revision to Diff 28497.EditedJan 25 2020, 3:31 AM

add more pain, to prevent unified from supporting legacy style tags, and prevent legacy from the ability to use new unified tags

@zmike Now we have common tags which will be checked first if the style is not found in common tags, last else will check if it is unified only tag or legacy only tag.

ali.alzyod edited the summary of this revision. (Show Details)Jan 26 2020, 12:57 AM
ali.alzyod updated this revision to Diff 28500.Jan 26 2020, 1:31 AM
  • update documentation
ali.alzyod edited the summary of this revision. (Show Details)

I agree with the docs and the spirit of this change. Won't comment on the code.

src/lib/evas/canvas/evas_object_textblock.c
1993–1994

Isn't this one already being checked in the _format_command_* functions?

ali.alzyod marked an inline comment as done.Jan 27 2020, 1:59 AM
zmike added a comment.Jan 27 2020, 7:21 AM

Isn't this missing a couple tags? Like strikethrough and underline_dashed (and variants)?

ali.alzyod updated this revision to Diff 28534.EditedJan 27 2020, 8:06 AM

Isn't this missing a couple tags? Like strikethrough and underline_dashed (and variants)?

@zmike code updated, I was going to update others, but I want to ensure the approch is ok

zmike added a comment.Jan 27 2020, 9:39 AM

Ah yeah, looks great, thanks!.

ali.alzyod updated this revision to Diff 28571.Jan 28 2020, 1:54 AM
  • update all style types
ali.alzyod retitled this revision from efl.canvas.textblock: update backing style strings to efl.canvas.textblock: update style strings.Jan 28 2020, 1:58 AM
ali.alzyod edited the summary of this revision. (Show Details)
segfaultxavi requested changes to this revision.Jan 28 2020, 3:09 AM

The default values for underline_type and background_type are off, which is an invalid value for Unified (should be none).
Please review the docs for style_apply, they still mention underline and strikethrough, for example, and they use off in several places.

This revision now requires changes to proceed.Jan 28 2020, 3:09 AM
ali.alzyod updated this revision to Diff 28575.Jan 28 2020, 3:27 AM
  • update documentation
ali.alzyod updated this revision to Diff 28576.Jan 28 2020, 3:29 AM

change background default type from off to none

I'm sorry but there are still several references to the old format strings in the docs.
I highlighted a few, but there are more.

Can you please review carefully the complete file and make sure they are all removed?

src/lib/evas/canvas/efl_canvas_textblock.eo
144

underline_type

148–149

underline_type

180

strikethrough_type

222–223

off -> none

224–225

none

ali.alzyod updated this revision to Diff 28577.Jan 28 2020, 3:45 AM
ali.alzyod marked 5 inline comments as done.
  • update
ali.alzyod updated this revision to Diff 28578.Jan 28 2020, 3:48 AM
  • update docs
segfaultxavi requested changes to this revision.Jan 28 2020, 3:54 AM

Almost there!

underline_type, strikethrough_type and effect_type still contain references to off. Using the "search" functionality of my browser I found 5.

src/lib/evas/canvas/efl_canvas_textblock.eo
240

I repeat again, please read the text you modify. This sentence makes no sense: "Possible values are $none (No decoration, alias for $off and $none)".
The "alias" part can be removed, leave only "No decorations" inside the parenthesis.

This revision now requires changes to proceed.Jan 28 2020, 3:54 AM
ali.alzyod added inline comments.Jan 28 2020, 4:08 AM
src/lib/evas/canvas/efl_canvas_textblock.eo
240

It is already updated, you commented too fast

ali.alzyod requested review of this revision.Jan 28 2020, 4:28 AM
segfaultxavi accepted this revision.Jan 28 2020, 9:52 AM

OK! No more comments on the docs side :)

Is code alright?

This revision is now accepted and ready to land.Jan 28 2020, 9:52 AM
bu5hm4n accepted this revision.Jan 29 2020, 12:35 AM

Looks good, any famous last words @zmike ?

This revision was automatically updated to reflect the committed changes.