Bug 55026 - [EFL] Doxygen documentation for ewk_window_features and ewk_context_menu
Summary: [EFL] Doxygen documentation for ewk_window_features and ewk_context_menu
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-23 02:28 PST by Grzegorz Czajkowski
Modified: 2011-03-08 07:42 PST (History)
6 users (show)

See Also:


Attachments
Doxygen documentation (21.45 KB, patch)
2011-02-23 02:40 PST, Grzegorz Czajkowski
no flags Details | Formatted Diff | Diff
Doxygen documentation (21.51 KB, patch)
2011-03-03 05:41 PST, Grzegorz Czajkowski
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Grzegorz Czajkowski 2011-02-23 02:28:15 PST
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
Comment 1 Grzegorz Czajkowski 2011-02-23 02:40:36 PST
Created attachment 83461 [details]
Doxygen documentation
Comment 2 Gyuyoung Kim 2011-02-23 15:21:49 PST
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 "///"
Comment 3 Ryuan Choi 2011-02-23 15:54:35 PST
(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.
Comment 4 Grzegorz Czajkowski 2011-03-03 05:41:19 PST
Created attachment 84548 [details]
Doxygen documentation
Comment 5 Grzegorz Czajkowski 2011-03-03 05:48:48 PST
(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
Comment 6 Gyuyoung Kim 2011-03-07 23:43:05 PST
LGTM.
Comment 7 Kent Tamura 2011-03-07 23:48:44 PST
Comment on attachment 84548 [details]
Doxygen documentation

I'm not familiar with Doxygen, but this patch must be harmless.
Comment 8 Gyuyoung Kim 2011-03-07 23:54:38 PST
(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.
Comment 9 Kent Tamura 2011-03-08 00:05:37 PST
(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.
Comment 10 Gyuyoung Kim 2011-03-08 00:11:05 PST
(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.
Comment 11 WebKit Commit Bot 2011-03-08 07:42:50 PST
Comment on attachment 84548 [details]
Doxygen documentation

Clearing flags on attachment: 84548

Committed r80563: <http://trac.webkit.org/changeset/80563>
Comment 12 WebKit Commit Bot 2011-03-08 07:42:56 PST
All reviewed patches have been landed.  Closing bug.