WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 68209
[EFL] Change WebKit EFL coding style
https://bugs.webkit.org/show_bug.cgi?id=68209
Summary
[EFL] Change WebKit EFL coding style
Gyuyoung Kim
Reported
2011-09-15 19:16:05 PDT
WebKit EFL port has used EFL coding style. However, EFL port has used C-coding style despite files are C++. In addition, this style disturbed reviewers to review EFL patches. So, we discussed this issues by means of webkit-efl mailing list.
https://lists.webkit.org/pipermail/webkit-efl/2011-September/000049.html
I list current issues up as below. And, this bug is a meta bug to track this issues. 1. pointer operator coding style based on efl coding style. : pointer('*') operator coding style follows efl coding style. It looks we need to adhere webkit coding style. 2. Should we use EINA_TRUE and EINA_FALSE? : It can be replace with true / false. 3. Internal functions are using '(void)' parameter. : As we already discussed, internal function needs to use C++ coding style. 4. Manual memory management. : ewk_xxx files are using malloc, calloc and realloc instead of smart pointer. For example, GOwnPtr. 5. C style casting operator. : Though file format is .cpp, some codes are still using c type casting operator. 6. Meaningless parameter name. : We have used coding style of efl parameter so far. However, that is not webkit coding style. I heard that port implementation tends to be platform coding style. So, I added a parameter coding style exception to style checker for efl port. But, now I'm not sure whether we're able to use it continually.
Attachments
Add attachment
proposed patch, testcase, etc.
Lucas De Marchi
Comment 1
2011-09-17 09:08:50 PDT
(In reply to
comment #0
)
> WebKit EFL port has used EFL coding style. However, EFL port has used C-coding style despite files are C++. In addition, this style disturbed reviewers to review EFL patches. So, we discussed this issues by means of webkit-efl mailing list.
https://lists.webkit.org/pipermail/webkit-efl/2011-September/000049.html
> > I list current issues up as below. And, this bug is a meta bug to track this issues. > > 1. pointer operator coding style based on efl coding style. > : pointer('*') operator coding style follows efl coding style. It looks we need to adhere webkit coding style.
Ok, on
bug 68231
I sent an initial configuration for uncrustify to fix this.
> > 2. Should we use EINA_TRUE and EINA_FALSE? > : It can be replace with true / false.
The problem here is that in public functions we must use Eina_Bool type. It's somewhat automatic then in those functions to use EINA_TRUE/EINA_FALSE. Internal functions should be fine using bool and thus true/false. For public functions, I'm not sure what approach we should take.
> 3. Internal functions are using '(void)' parameter. > : As we already discussed, internal function needs to use C++ coding style.
Agreed.
> 4. Manual memory management. > : ewk_xxx files are using malloc, calloc and realloc instead of smart pointer. > For example, GOwnPtr.
These are more difficult to handle. Recently Raphael Kubo converted some of them.
> 5. C style casting operator. > : Though file format is .cpp, some codes are still using c type casting operator.
This is another easy one. Since most of the time we are converting C structs or native types, almost all of them will be converted to static_cast<>. We could even automatically convert all of them to static_cast<> and going back if we need another type of cast.
> 6. Meaningless parameter name. > : We have used coding style of efl parameter so far. However, that is not webkit coding style. > I heard that port implementation tends to be platform coding style. So, I added a parameter coding style exception to > style checker for efl port. But, now I'm not sure whether we're able to use it continually.
Some of them are common language to EFL committers/users. I don't think changing it is a good idea, but if it's required we can use sed/coccinelle for this task.
Kenneth Rohde Christiansen
Comment 2
2011-09-17 10:41:08 PDT
> > 2. Should we use EINA_TRUE and EINA_FALSE? > > : It can be replace with true / false. > > The problem here is that in public functions we must use Eina_Bool type. It's somewhat automatic then in those functions to use EINA_TRUE/EINA_FALSE.
For return values this should be fine.
Raphael Kubo da Costa (:rakuco)
Comment 3
2011-09-17 12:11:27 PDT
(In reply to
comment #2
)
> > > 2. Should we use EINA_TRUE and EINA_FALSE? > > > : It can be replace with true / false. > > > > The problem here is that in public functions we must use Eina_Bool type. It's somewhat automatic then in those functions to use EINA_TRUE/EINA_FALSE. > > For return values this should be fine.
Eina_Bool Foo::bar() { // ... return true; } should work just fine too, there's just an implicit bool -> unsigned char conversion involved.
Gyuyoung Kim
Comment 4
2011-09-18 21:12:32 PDT
(In reply to
comment #3
)
> Eina_Bool Foo::bar() > { > // ... > return true; > } >
Can this conversion to reduce readability or make confusion to ewk contributor ? I think it is better to use only EINA_BOOL in public functions.
Raphael Kubo da Costa (:rakuco)
Comment 5
2011-12-01 05:13:41 PST
All the bugs this report depends on have been closed, can we close this one too?
Gyuyoung Kim
Comment 6
2011-12-13 20:32:15 PST
(In reply to
comment #5
)
> All the bugs this report depends on have been closed, can we close this one too?
I'd like to close this bug after
Bug 74474
is landed. Because, that bug is also similar bug.
Gyuyoung Kim
Comment 7
2011-12-14 22:40:01 PST
All coding style issues mentioned by this bug are closed. So, I close this bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug