Bug 71433 – [EFL] Use standard boolean data type.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Gyuyoung Kim]

This is a fourth step in order to be more compliant with WebKit coding style. Use standard boolean data type instead of efl boolean data type. It makes efl port is more close to webkit coding style. In addition, GTK port is also using standard boolean type despite header files are using "gboolean" type.

[Gyuyoung Kim]

Created attachment 113419 [details]
Patch

[Gyuyoung Kim]

CC'ing kenneth, tonikitoo, kubo, leandro, ryuan.

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 113419 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=113419&action=review

> Source/WebKit/efl/ewk/ewk_contextmenu.cpp:259
>  Eina_Bool ewk_context_menu_free(Ewk_Context_Menu* menu)

Internal functions such as this one could have their return type changed to bool as well.

[Gyuyoung Kim]

Created attachment 113428 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #3)
> (From update of attachment 113419 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=113419&action=review
> 
> > Source/WebKit/efl/ewk/ewk_contextmenu.cpp:259
> >  Eina_Bool ewk_context_menu_free(Ewk_Context_Menu* menu)
> 
> Internal functions such as this one could have their return type changed to bool as well.

Ok, I change return type with bool for internal functions.

[Ryuan Choi]

Comment on attachment 113428 [details]
Patch

I think that we also change Eina_Bool in static functions which start to "_ewk_XXX"

View in context: https://bugs.webkit.org/attachment.cgi?id=113428&action=review

> Source/WebKit/efl/ewk/ewk_frame.cpp:469
> -    EINA_SAFETY_ON_NULL_RETURN_VAL(smartData->frame, EINA_FALSE);
> +    EINA_SAFETY_ON_NULL_RETURN_VAL(smartData->frame, false);

Although this bug is for fixing style issue, this should be 0.

[Gyuyoung Kim]

Created attachment 113429 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #6)
> (From update of attachment 113428 [details])
> I think that we also change Eina_Bool in static functions which start to "_ewk_XXX"
> 
> View in context: https://bugs.webkit.org/attachment.cgi?id=113428&action=review
> 
> > Source/WebKit/efl/ewk/ewk_frame.cpp:469
> > -    EINA_SAFETY_ON_NULL_RETURN_VAL(smartData->frame, EINA_FALSE);
> > +    EINA_SAFETY_ON_NULL_RETURN_VAL(smartData->frame, false);
> 
> Although this bug is for fixing style issue, this should be 0.

Yes, right. it is fixed.

[Grzegorz Czajkowski]

You didn't update any public header files of WebKit-EFL. Present documentation describes that API returns EINA_TRUE or EINA_FALSE not true or false.
Please update header files too.

[Gyuyoung Kim]

(In reply to comment #9)
> You didn't update any public header files of WebKit-EFL. Present documentation describes that API returns EINA_TRUE or EINA_FALSE not true or false.
> Please update header files too.

I'm not sure if we should change the return type in documentation. Is it better to know return type is *Eina_Bool* type instead of *bool* ?

[Grzegorz Czajkowski]

(In reply to comment #10)
> (In reply to comment #9)
> > You didn't update any public header files of WebKit-EFL. Present documentation describes that API returns EINA_TRUE or EINA_FALSE not true or false.
> > Please update header files too.
> 
> I'm not sure if we should change the return type in documentation. Is it better to know return type is *Eina_Bool* type instead of *bool* ?

You've already changed documentation of private functions by replacing EINA_TRUE/EINA_FALSE to true/false. I think we should consistently update API documentation  too.

[Gyuyoung Kim]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > You didn't update any public header files of WebKit-EFL. Present documentation describes that API returns EINA_TRUE or EINA_FALSE not true or false.
> > > Please update header files too.
> > 
> > I'm not sure if we should change the return type in documentation. Is it better to know return type is *Eina_Bool* type instead of *bool* ?
> 
> You've already changed documentation of private functions by replacing EINA_TRUE/EINA_FALSE to true/false. I think we should consistently update API documentation  too.

Hmm, it seems it can make confusion when we add new APIs or internal functions. However, we can't change efl style with webkit style in public headers. Thus, I think it is difficult to change Eina_Bool of documentation with true/false. Because, I think efl application developers refer to public headers basically. But, in internal function case, that is used by webkit efl port developers. So, I think it is better to keep Eina_Bool in documentation of public headers. I'd like to know other guys opinions.

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #12)
> Hmm, it seems it can make confusion when we add new APIs or internal functions. However, we can't change efl style with webkit style in public headers. Thus, I think it is difficult to change Eina_Bool of documentation with true/false. Because, I think efl application developers refer to public headers basically. But, in internal function case, that is used by webkit efl port developers. So, I think it is better to keep Eina_Bool in documentation of public headers. I'd like to know other guys opinions.

Yes. Just like the public headers still have "Evas_Object *o", I think they should keep things familiar to the EFL users who are going to use the API -- whether we use bools or Eina_Bools, or the coding style we use internally should be transparent.

[Raphael Kubo da Costa (:rakuco)]

LGTM.

[Ryuan Choi]

Looks good to me, too.

[Zoltan Herczeg]

Comment on attachment 113429 [details]
Patch

Nice work!

r=me

[WebKit Review Bot]

Comment on attachment 113429 [details]
Patch

Clearing flags on attachment: 113429

Committed r99268: <http://trac.webkit.org/changeset/99268>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Gyuyoung Kim]

Created attachment 115132 [details]
Patch

[Gyuyoung Kim]

I missed to change *Eina_Bool* in internal struct. So, I re-open this bug.

[Raphael Kubo da Costa (:rakuco)]

LGTM, just not sure if it's better to reopen this one or open a new bug.

[Xan Lopez]

Comment on attachment 115132 [details]
Patch

This looks fine but you really need to open a new bug for it. CC me and I'll r+ it.

[Xan Lopez]

Closing.