Page MenuHomePhabricator

theme: attempt to fix failure to correctly namespace uiclock parts
AbandonedPublic

Authored by zmike on May 25 2018, 2:54 PM.

Details

Summary

all parts referenced by library code must be namespaced. this adds
namespacing to those parts with the smallest amount of changes, but
further work should be done to avoid having namespaced parts which
are not API

ref T6965
ref D3939
Depends on D6208

Diff Detail

Repository
rEFL core/efl
Branch
edje-feature
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6351
zmike created this revision.May 25 2018, 2:54 PM
zmike requested review of this revision.May 25 2018, 2:54 PM
Hermet added a subscriber: Hermet.May 27 2018, 10:09 PM
This comment was removed by Hermet.

how about efl than elm?
we are already moving on efl_ui from elm.

zmike added a comment.May 28 2018, 3:16 AM

If there is a released legacy API (ie. the theme is in edc/elm/ in the tree) then the namespace should be 'elm'. If it is only an eo API (ie. the theme is in edc/efl/ in the tree) then the namespace should be 'efl'.

The tree location should relate to the C implementation; if the widget has a released legacy API (ie. elm_$widget_add) then it must have an 'elm' theme component.

@zmike

I think efl_ui_clock.c should support both "elm" namespace for legacy and "efl" namespace for ui interface.

To support that, I used "elm_widget_is_legacy()" to distinguish legacy case although it increased # of code lines..

zmike abandoned this revision.Jun 4 2018, 4:41 AM

It seems that this widget has not been released as a legacy widget, which means that it should only have 'efl' namespace. I'm going to abandon this patch, but I think it can be used as a reference when someone makes a better attempt at fixing the issues than I have.

I have some questions.

  1. uiclock has never been released as legacy, so what was the previous clock widget? elm/clock?
  1. There's elm/uiclock.edc and efl/uiclock.edc. Does this mean that we are going to release this new widget both as efl and legacy?
  1. If yes (we are going to release the new widget both as elm and efl), should the parts in each edc file have its own prefix (elm. and efl.) ?
  1. If yes (we are going to release the new widget both as elm and efl), should we create a new src/lib/elementary/elm_ui_clock.c?
  1. What is wrong with @zmike's diff above?
zmike added a comment.Jun 25 2018, 3:33 AM

@Hermet or @Jaehyun_Cho can confirm, but it seems that there is no legacy API for this widget and it is only to be used with interfaces? In this case, I'm not sure why there is any "elm" code or theme groups in the codebase, as it seems like this should only use "efl" and have no legacy checks. This means that it should be possible to safely remove the edc/elm/uiclock.edc file entirely.

I could not find any reference to uiclock in E or other apps, so yeah, I am tempted to just remove the file unless its author (@singh.amitesh) says otherwise...

zmike added a comment.Jun 25 2018, 4:26 AM

To be clear, I assume you are talking about edc/elm/uiclock.edc?

Yes, I would remove data/elementary/themes/edc/elm/uiclock.edc and leave data/elementary/themes/edc/efl/uiclock.edc