Bug 55026 – [EFL] Doxygen documentation for ewk_window_features and ewk_context_menu

• Component: Text
• Status: RESOLVED FIXED

[Grzegorz Czajkowski]

It adds and modifies doxygen documentation for files:

ewk_context_menu.h/cpp
ewk_window_features.h/cpp

Additionally I modified ewk_context_menu_item_select method. Now it returns EINA_FALSE when CONTEXT_MENU is disabled. What is your opinion on that?
I would like to suggest similar policy for the internal methods in ewk_context_menu.cpp - to move CONTEXT_MENU macro inside the functions. When the macro is disabled the methods will be returned EINA_FALSE or 0 (NULL pointer). Current for the internal methods doxygen documentation won't be available (even if doxgen config will be changed).

Thanks

[Grzegorz Czajkowski]

Created attachment 83461 [details]
Doxygen documentation

[Gyuyoung Kim]

Comment on attachment 83461 [details]
Doxygen documentation

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

> Source/WebKit/efl/ChangeLog:5
> +        [EF] Doxygen documentation for ewk_window_features and ewk_context_menu

[EF] => [EFL]

> Source/WebKit/efl/ewk/ewk_contextmenu.cpp:57
> +    Ewk_Context_Menu* submenu; /** contains the pointer to the submenu of the item */

Please add "<" to "/**"

> Source/WebKit/efl/ewk/ewk_contextmenu.h:79
> +    EWK_CONTEXT_MENU_ITEM_TAG_SPELLING_MENU, /**< spelling or spelling/grammar sub-menu */

This is not sync with ewk_cookie.cpp. Which one is better?  "/**<" or "///"

> Source/WebKit/efl/ewk/ewk_contextmenu.h:84
> +    EWK_CONTEXT_MENU_ITEM_TAG_FONT_MENU, /**< font sub-menu */

This is not sync with ewk_cookie.cpp. Which one is better?  "/**<" or "///"

[Ryuan Choi]

(In reply to comment #2)
> (From update of attachment 83461 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=83461&action=review
> 
> > Source/WebKit/efl/ChangeLog:5
> > +        [EF] Doxygen documentation for ewk_window_features and ewk_context_menu
> 
> [EF] => [EFL]
> 
> > Source/WebKit/efl/ewk/ewk_contextmenu.cpp:57
> > +    Ewk_Context_Menu* submenu; /** contains the pointer to the submenu of the item */
> 
> Please add "<" to "/**"
> 
> > Source/WebKit/efl/ewk/ewk_contextmenu.h:79
> > +    EWK_CONTEXT_MENU_ITEM_TAG_SPELLING_MENU, /**< spelling or spelling/grammar sub-menu */
> 
> This is not sync with ewk_cookie.cpp. Which one is better?  "/**<" or "///"
> 
> > Source/WebKit/efl/ewk/ewk_contextmenu.h:84
> > +    EWK_CONTEXT_MENU_ITEM_TAG_FONT_MENU, /**< font sub-menu */
> 
> This is not sync with ewk_cookie.cpp. Which one is better?  "/**<" or "///"

IMO,  "/**<" is better because EFL is based on C application.

[Grzegorz Czajkowski]

Created attachment 84548 [details]
Doxygen documentation

[Grzegorz Czajkowski]

(In reply to comment #2)
> (From update of attachment 83461 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=83461&action=review
> 
> > Source/WebKit/efl/ChangeLog:5
> > +        [EF] Doxygen documentation for ewk_window_features and ewk_context_menu
> 
> [EF] => [EFL]

fixed

> 
> > Source/WebKit/efl/ewk/ewk_contextmenu.cpp:57
> > +    Ewk_Context_Menu* submenu; /** contains the pointer to the submenu of the item */
> 
> Please add "<" to "/**"

added

> 
> > Source/WebKit/efl/ewk/ewk_contextmenu.h:84
> > +    EWK_CONTEXT_MENU_ITEM_TAG_FONT_MENU, /**< font sub-menu */
> 
> This is not sync with ewk_cookie.cpp. Which one is better?  "/**<" or "///"

All "/// comment" were replaced to "/**< comment */" or to "/** comment */" in ewk_context_menu/ewk_widow_features.
"/**< comment */ is dedicated for struct member
"/** comment */ is dedicated for structs/enums description

I will fix this in ewk_cookies soon because it's better style in C applications

[Gyuyoung Kim]

LGTM.

[Kent Tamura]

Comment on attachment 84548 [details]
Doxygen documentation

I'm not familiar with Doxygen, but this patch must be harmless.

[Gyuyoung Kim]

(In reply to comment #7)
> (From update of attachment 84548 [details])
> I'm not familiar with Doxygen, but this patch must be harmless.

Is there documentation rule in WebKit ?  I can't find documentation rule.

[Kent Tamura]

(In reply to comment #8)
> Is there documentation rule in WebKit ?  I can't find documentation rule.

I think there is no project-wide agreement and we may use any documentation style for platform-specific files at our own discretion.

[Gyuyoung Kim]

(In reply to comment #9)
> (In reply to comment #8)
> > Is there documentation rule in WebKit ?  I can't find documentation rule.
> 
> I think there is no project-wide agreement and we may use any documentation style for platform-specific files at our own discretion.

ok, we think doxyzen is good documentation style for EFL port.

[WebKit Commit Bot]

Comment on attachment 84548 [details]
Doxygen documentation

Clearing flags on attachment: 84548

Committed r80563: <http://trac.webkit.org/changeset/80563>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.