2017-10-27 11:12 GMT+09:00 Amitesh Singh <singh.amit...@gmail.com>:
> Hello,
>
>
> On Thu, Oct 26, 2017 at 9:30 PM, Gustavo Sverzut Barbieri <
> barbi...@gmail.com> wrote:
>
> > On Thu, Oct 26, 2017 at 10:12 AM, Amitesh Singh <amitesh...@samsung.com>
> > wrote:
> >
> > thanks for the improvement, but one hint I said before went unnoticed
> > and would save great deal of this implementation:
> >
> >
> > > +typedef struct
> > > +{
> > > + const char *template;
> > > +} Efl_Ui_Format_Data;
> >
> > this is already stored in the object, due format_string_set... that's
> > why I'm saying that you should pass it to the callback, so there is no
> > need to store it again, providing a free_cb to release that.
> >
> >
> >
> > > +_default_format_cb(void *data, Eina_Strbuf *str, const Eina_Value
> value)
> >
> > here you should pass: str, value AND template :-)
> >
> >
> I don't think there is a need to pass tempate here since its least useful
> to user and its a burden for anyone to have both strbuf and template
> string.
> We got another API for passing string template.
>
Hmm I agree with Amitesh here.
If the template is given as well then the user cb is mostly useless, as the
default implementation would take care of doing the sprintf.
All that can then be done is prepend or append strings, at best.
I wonder if that wouldn't lead to more mistakes as we would then assume
what the format string is (eg. print a double eina_value with "%i units"
for instance).
>
>
> > > +_default_format_free_cb(void *data)
> >
> > then this is not needed.
> >
> >
> >
> > > +EOLIAN static void
> > > +_efl_ui_format_format_string_set(Eo *obj, Efl_Ui_Format_Data *sd,
> > const char *template)
> > > +{
> > > + if (!template) return;
> > > + eina_stringshare_replace(&sd->template, template);
> > > + efl_ui_format_cb_set(obj, sd, _default_format_cb,
> > _default_format_free_cb);
> >
> > because here you'd not call "efl_ui_format_cb_set()"... you'd do that
> > once at the constructor/init, then the callback gets the template
> > stored by efl_ui_format_string_set().
> >
> >
> > > - elm_layout_text_set(obj, "elm.units", buf);
> > > + sd->format_cb(sd->format_cb_data, sd->format_strbuf, val);
> >
> > this is not that good. You should wrap this into a protected method
> > that implementers call, like:
> >
> > efl_ui_format(obj, strbuf, val);
> >
> > Think of bindings (ie: python/javascript/c#), they may want to call
> > the implementation of efl.ui.slider.... how to? you need to provide a
> > method to allow that.
>
Hmm good point. I'll think about it.
--
Jean-Philippe André
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
