Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Peter Kasting
On Fri, Dec 3, 2010 at 2:42 PM, David Hyatt  wrote:

> An example of nice boolean usage is:
>
> paintTextWithShadows(context, font, textRun, 0, length, length,
> textOrigin, boxRect, textShadow, textStrokeWidth > 0, isHorizontal());
> GOOD!
>
> The last parameter is a boolean indicating whether or not the text run is
> vertical text or horizontal text.  It would be unreadable if you passed raw
> true or raw false to the function, but all the call sites pass
> isHorizontal(), so it's perfectly fine.
>

Furthermore, the second-to-last arg here looks like a bool too, but
"textStrokeWidth > 0" seems a lot more obvious to a reader than "true".

PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


[webkit-dev] Fwd: Bools are strictly worse than enums

2010-12-03 Thread Ryosuke Niwa
On Fri, Dec 3, 2010 at 2:30 PM, Darin Adler  wrote:
>
>  > I think in general the rule should be "Keep your call sites readable,
> and convert to enums if you find that the call sites are becoming
> inscrutable."
>
> Beyond the enum guidelines, it’s also true that functions with these
> boolean/enum arguments often indicate minor design mistakes. Many times we
> would be better off with multiple functions rather than a single function
> with an enum argument.


I agree.  I prefer to have multiple public functions that are named
accordingly than having a single public function with enums.  However, we
could have a private function with enum that implements these public
functions to share code.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread David Hyatt
An example of nice boolean usage is:

paintTextWithShadows(context, font, textRun, 0, length, length, textOrigin, 
boxRect, textShadow, textStrokeWidth > 0, isHorizontal()); GOOD!

The last parameter is a boolean indicating whether or not the text run is 
vertical text or horizontal text.  It would be unreadable if you passed raw 
true or raw false to the function, but all the call sites pass isHorizontal(), 
so it's perfectly fine.

paintTextWithShadows(context, font, textRun, 0, length, length, textOrigin, 
boxRect, textShadow, textStrokeWidth > 0, true);  BAD!

dave
(hy...@apple.com)

On Dec 3, 2010, at 4:30 PM, Darin Adler wrote:

> On Dec 3, 2010, at 1:37 PM, David Hyatt wrote:
> 
>> The only exception I would make to this rule is if all the call sites use 
>> variables and never pass in raw true or false. In that case there's no loss 
>> of readability, and whether you use an enum vs. a bool is irrelevant.
> 
> That’s right. There are cases where the boolean value passed in for a 
> particular argument is typically the result of a boolean expression or 
> boolean-returning function. We have definitely seen these in patch review.
> 
>> I think in general the rule should be "Keep your call sites readable, and 
>> convert to enums if you find that the call sites are becoming inscrutable."
> 
> Beyond the enum guidelines, it’s also true that functions with these 
> boolean/enum arguments often indicate minor design mistakes. Many times we 
> would be better off with multiple functions rather than a single function 
> with an enum argument.
> 
>-- Darin
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Darin Adler
On Dec 3, 2010, at 1:37 PM, David Hyatt wrote:

> The only exception I would make to this rule is if all the call sites use 
> variables and never pass in raw true or false. In that case there's no loss 
> of readability, and whether you use an enum vs. a bool is irrelevant.

That’s right. There are cases where the boolean value passed in for a 
particular argument is typically the result of a boolean expression or 
boolean-returning function. We have definitely seen these in patch review.

> I think in general the rule should be "Keep your call sites readable, and 
> convert to enums if you find that the call sites are becoming inscrutable."

Beyond the enum guidelines, it’s also true that functions with these 
boolean/enum arguments often indicate minor design mistakes. Many times we 
would be better off with multiple functions rather than a single function with 
an enum argument.

-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Ryosuke Niwa
On Fri, Dec 3, 2010 at 2:08 PM, Alexey Proskuryakov  wrote:
>
> I think that Dave's rule is best here
>

I second that.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Ryosuke Niwa
On Fri, Dec 3, 2010 at 1:55 PM, Nico Weber  wrote:

> Out of curiosity, what do people think of
>
> doSomethingElse(/*paramName=*/true);
>
> when calling an existing function that takes a bool?
>

Why don't we just change it to take enum instead?  Adding a comment to
repeat the argument name in call sites seems to contradict all other coding
principles we have in WebKit.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Alexey Proskuryakov

03.12.2010, в 13:54, Eric Seidel написал(а):

> I'm not sure we have any examples of bool passing like that in real code.


We do, although I can't provide one now. I just remember this being discussed 
in bug review.

I think that Dave's rule is best here, even if it meant that we couldn't check 
it automatically. But I think that we can try - just complain each time true or 
false is passed into a function. That may prove to be too chatty though.

03.12.2010, в 13:55, Nico Weber написал(а):

> Out of curiosity, what do people think of
> 
> doSomethingElse(/*paramName=*/true);

There is a horrible failure case - if the comment is wrong (or gets wrong after 
a refactoring made elsewhere), it gets much worse than an unannotated boolean 
argument.

- WBR, Alexey Proskuryakov

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Antonio Gomes
Again, I think enum is better. I am just saying that method(true /*
xxx */) is not as bad as method(true, true, true, false);

On Fri, Dec 3, 2010 at 5:01 PM, Eric Seidel  wrote:
> I think /* explanation of what I'm doing */ is strictly worse than
> readableCode(UnderstandableParameter).
> I'd rather have readable code than comments attempting to excuse unreadable
> code.
> -eric
> On Fri, Dec 3, 2010 at 1:57 PM, Antonio Gomes  wrote:
>>
>> I do not think that XXX:repaint(true /* immediate */) is so bad
>> either, if I understood Hyatt's comment correctly, and I agree with him on
>> it.
>>
>> Having a enum is ideal, but no need for it to be mandatory, as other
>> also pointed out.
>>
>> On Fri, Dec 3, 2010 at 4:54 PM, Eric Seidel  wrote:
>> > I'm not sure we have any examples of bool passing like that in real
>> > code.
>> > The case I'm concerned about is not one of single argument bools:
>> > doSoemthing(bool)
>> > but more of multi-argument functions:
>> > doSomething(something, bool)
>> > I'm trying to write a rule which can be easily automated by
>> > check-webkit-style.
>> > It's possible we could tighten the rule further to only allow
>> > single-argument bools where "set" is in the function name.
>> > It sounds like most folks are in agreement.  We should add a rule like
>> > this
>> > to check-webkit-style.  Sounds like Dave Levin may already have
>> > something
>> > partial in the works.
>> > -eric
>> > On Fri, Dec 3, 2010 at 1:45 PM, Ryosuke Niwa  wrote:
>> >>
>> >> On Fri, Dec 3, 2010 at 1:37 PM, David Hyatt  wrote:
>> >>>
>> >>> The only exception I would make to this rule is if all the call sites
>> >>> use
>> >>> variables and never pass in raw true or false.  In that case there's
>> >>> no loss
>> >>> of readability, and whether you use an enum vs. a bool is irrelevant.
>> >>> I think in general the rule should be "Keep your call sites readable,
>> >>> and
>> >>> convert to enums if you find that the call sites are becoming
>> >>> inscrutable."
>> >>
>> >> That rule makes sense to me.
>> >> On Fri, Dec 3, 2010 at 1:40 PM, Eric Seidel  wrote:
>> >>>
>> >>> Dave, I'm not sure I understand your exception.  Could you give an
>> >>> example?
>> >>
>> >> I think what he means is that
>> >> bool doSomething();
>> >> void doSomethingElse(bool);
>> >> and the only case we always call doSomethingElse with a return value of
>> >> some function or with a variable:
>> >> doSomethingElse(doSomething());
>> >> doSomethingElse(shouldNotDoSomethingElse);
>> >> etc...
>> >> and we never call it with raw true/false:
>> >> doSomethingElse(true)
>> >> doSomethingElse(false)
>> >> - Ryosuke
>> >>
>> >> ___
>> >> webkit-dev mailing list
>> >> webkit-dev@lists.webkit.org
>> >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>> >>
>> >
>> >
>> > ___
>> > webkit-dev mailing list
>> > webkit-dev@lists.webkit.org
>> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>> >
>> >
>>
>>
>>
>> --
>> --Antonio Gomes
>
>



-- 
--Antonio Gomes
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Eric Seidel
I think /* explanation of what I'm doing */ is strictly worse than
readableCode(UnderstandableParameter).

I'd rather have readable code than comments attempting to excuse unreadable
code.

-eric

On Fri, Dec 3, 2010 at 1:57 PM, Antonio Gomes  wrote:

> I do not think that XXX:repaint(true /* immediate */) is so bad
> either, if I understood Hyatt's comment correctly, and I agree with him on
> it.
>
> Having a enum is ideal, but no need for it to be mandatory, as other
> also pointed out.
>
> On Fri, Dec 3, 2010 at 4:54 PM, Eric Seidel  wrote:
> > I'm not sure we have any examples of bool passing like that in real code.
> > The case I'm concerned about is not one of single argument bools:
> > doSoemthing(bool)
> > but more of multi-argument functions:
> > doSomething(something, bool)
> > I'm trying to write a rule which can be easily automated by
> > check-webkit-style.
> > It's possible we could tighten the rule further to only allow
> > single-argument bools where "set" is in the function name.
> > It sounds like most folks are in agreement.  We should add a rule like
> this
> > to check-webkit-style.  Sounds like Dave Levin may already have something
> > partial in the works.
> > -eric
> > On Fri, Dec 3, 2010 at 1:45 PM, Ryosuke Niwa  wrote:
> >>
> >> On Fri, Dec 3, 2010 at 1:37 PM, David Hyatt  wrote:
> >>>
> >>> The only exception I would make to this rule is if all the call sites
> use
> >>> variables and never pass in raw true or false.  In that case there's no
> loss
> >>> of readability, and whether you use an enum vs. a bool is irrelevant.
> >>> I think in general the rule should be "Keep your call sites readable,
> and
> >>> convert to enums if you find that the call sites are becoming
> inscrutable."
> >>
> >> That rule makes sense to me.
> >> On Fri, Dec 3, 2010 at 1:40 PM, Eric Seidel  wrote:
> >>>
> >>> Dave, I'm not sure I understand your exception.  Could you give an
> >>> example?
> >>
> >> I think what he means is that
> >> bool doSomething();
> >> void doSomethingElse(bool);
> >> and the only case we always call doSomethingElse with a return value of
> >> some function or with a variable:
> >> doSomethingElse(doSomething());
> >> doSomethingElse(shouldNotDoSomethingElse);
> >> etc...
> >> and we never call it with raw true/false:
> >> doSomethingElse(true)
> >> doSomethingElse(false)
> >> - Ryosuke
> >>
> >> ___
> >> webkit-dev mailing list
> >> webkit-dev@lists.webkit.org
> >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
> >>
> >
> >
> > ___
> > webkit-dev mailing list
> > webkit-dev@lists.webkit.org
> > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
> >
> >
>
>
>
> --
> --Antonio Gomes
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Antonio Gomes
I do not think that XXX:repaint(true /* immediate */) is so bad
either, if I understood Hyatt's comment correctly, and I agree with him on it.

Having a enum is ideal, but no need for it to be mandatory, as other
also pointed out.

On Fri, Dec 3, 2010 at 4:54 PM, Eric Seidel  wrote:
> I'm not sure we have any examples of bool passing like that in real code.
> The case I'm concerned about is not one of single argument bools:
> doSoemthing(bool)
> but more of multi-argument functions:
> doSomething(something, bool)
> I'm trying to write a rule which can be easily automated by
> check-webkit-style.
> It's possible we could tighten the rule further to only allow
> single-argument bools where "set" is in the function name.
> It sounds like most folks are in agreement.  We should add a rule like this
> to check-webkit-style.  Sounds like Dave Levin may already have something
> partial in the works.
> -eric
> On Fri, Dec 3, 2010 at 1:45 PM, Ryosuke Niwa  wrote:
>>
>> On Fri, Dec 3, 2010 at 1:37 PM, David Hyatt  wrote:
>>>
>>> The only exception I would make to this rule is if all the call sites use
>>> variables and never pass in raw true or false.  In that case there's no loss
>>> of readability, and whether you use an enum vs. a bool is irrelevant.
>>> I think in general the rule should be "Keep your call sites readable, and
>>> convert to enums if you find that the call sites are becoming inscrutable."
>>
>> That rule makes sense to me.
>> On Fri, Dec 3, 2010 at 1:40 PM, Eric Seidel  wrote:
>>>
>>> Dave, I'm not sure I understand your exception.  Could you give an
>>> example?
>>
>> I think what he means is that
>> bool doSomething();
>> void doSomethingElse(bool);
>> and the only case we always call doSomethingElse with a return value of
>> some function or with a variable:
>> doSomethingElse(doSomething());
>> doSomethingElse(shouldNotDoSomethingElse);
>> etc...
>> and we never call it with raw true/false:
>> doSomethingElse(true)
>> doSomethingElse(false)
>> - Ryosuke
>>
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>



-- 
--Antonio Gomes
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Nico Weber
On Fri, Dec 3, 2010 at 1:45 PM, Ryosuke Niwa  wrote:
> On Fri, Dec 3, 2010 at 1:37 PM, David Hyatt  wrote:
>>
>> The only exception I would make to this rule is if all the call sites use
>> variables and never pass in raw true or false.  In that case there's no loss
>> of readability, and whether you use an enum vs. a bool is irrelevant.
>> I think in general the rule should be "Keep your call sites readable, and
>> convert to enums if you find that the call sites are becoming inscrutable."
>
> That rule makes sense to me.
> On Fri, Dec 3, 2010 at 1:40 PM, Eric Seidel  wrote:
>>
>> Dave, I'm not sure I understand your exception.  Could you give an
>> example?
>
> I think what he means is that
> bool doSomething();
> void doSomethingElse(bool);
> and the only case we always call doSomethingElse with a return value of some
> function or with a variable:
> doSomethingElse(doSomething());
> doSomethingElse(shouldNotDoSomethingElse);
> etc...
> and we never call it with raw true/false:
> doSomethingElse(true)
> doSomethingElse(false)

Out of curiosity, what do people think of

doSomethingElse(/*paramName=*/true);

when calling an existing function that takes a bool?

Nico

> - Ryosuke
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Eric Seidel
I'm not sure we have any examples of bool passing like that in real code.

The case I'm concerned about is not one of single argument bools:

doSoemthing(bool)

but more of multi-argument functions:
doSomething(something, bool)

I'm trying to write a rule which can be easily automated by
check-webkit-style.

It's possible we could tighten the rule further to only allow
single-argument bools where "set" is in the function name.

It sounds like most folks are in agreement.  We should add a rule like this
to check-webkit-style.  Sounds like Dave Levin may already have something
partial in the works.

-eric

On Fri, Dec 3, 2010 at 1:45 PM, Ryosuke Niwa  wrote:

> On Fri, Dec 3, 2010 at 1:37 PM, David Hyatt  wrote:
>
>> The only exception I would make to this rule is if all the call sites use
>> variables and never pass in raw true or false.  In that case there's no loss
>> of readability, and whether you use an enum vs. a bool is irrelevant.
>>
>> I think in general the rule should be "Keep your call sites readable, and
>> convert to enums if you find that the call sites are becoming inscrutable."
>>
>
> That rule makes sense to me.
>
> On Fri, Dec 3, 2010 at 1:40 PM, Eric Seidel  wrote:
>
> Dave, I'm not sure I understand your exception.  Could you give an example?
>
>
> I think what he means is that
> bool doSomething();
> void doSomethingElse(bool);
>
> and the only case we always call doSomethingElse with a return value of
> some function or with a variable:
> doSomethingElse(doSomething());
> doSomethingElse(shouldNotDoSomethingElse);
> etc...
>
> and we never call it with raw true/false:
> doSomethingElse(true)
> doSomethingElse(false)
>
> - Ryosuke
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Ryosuke Niwa
On Fri, Dec 3, 2010 at 1:37 PM, David Hyatt  wrote:
>
> The only exception I would make to this rule is if all the call sites use
> variables and never pass in raw true or false.  In that case there's no loss
> of readability, and whether you use an enum vs. a bool is irrelevant.
>
> I think in general the rule should be "Keep your call sites readable, and
> convert to enums if you find that the call sites are becoming inscrutable."
>

That rule makes sense to me.

On Fri, Dec 3, 2010 at 1:40 PM, Eric Seidel  wrote:

> Dave, I'm not sure I understand your exception.  Could you give an example?


I think what he means is that
bool doSomething();
void doSomethingElse(bool);

and the only case we always call doSomethingElse with a return value of some
function or with a variable:
doSomethingElse(doSomething());
doSomethingElse(shouldNotDoSomethingElse);
etc...

and we never call it with raw true/false:
doSomethingElse(true)
doSomethingElse(false)

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Eric Seidel
That would be so unbelievably fantastic!

On Fri, Dec 3, 2010 at 1:35 PM, David Levin  wrote:

>
>
> On Fri, Dec 3, 2010 at 1:28 PM, Eric Seidel  wrote:
>
>> It seems to me, that using bool types for function arguments is strictly
>> worse than using an enum.  An enum is always clearer and can be easily
>> casted to a bool if needed.
>>
>> doSomething(something, false);
>>
>> Is much less readable than:
>>
>> doSomething(something, AllowNetworkLoads);
>>
>>
>> Do any C++ gurus have further information to add here?  Is my (simple)
>> analysis here incorrect?  If not, seems we should forbid boolean values in
>> multi-argument methods/constructors in our style and add checks to
>> check-webkit-style
>>
>
> I'm pretty close to being able to do this in check-webkit-style. (I was
> actually working on something else.  Flagging unnecessary param names "void
> foo(ScriptExecutionContext* context);" because I get tired of mentioning
> this in code reviews.)
>
>
>> to prevent further introduction of these confusing callsites.
>>
>> -eric
>>
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>
>>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Eric Seidel
Dave, I'm not sure I understand your exception.  Could you give an example?

On Fri, Dec 3, 2010 at 1:37 PM, David Hyatt  wrote:

> On Dec 3, 2010, at 3:33 PM, Darin Fisher wrote:
>
> On Fri, Dec 3, 2010 at 1:28 PM, Eric Seidel  wrote:
>
>> It seems to me, that using bool types for function arguments is strictly
>> worse than using an enum.  An enum is always clearer and can be easily
>> casted to a bool if needed.
>>
>> doSomething(something, false);
>>
>> Is much less readable than:
>>
>> doSomething(something, AllowNetworkLoads);
>>
>>
>> Do any C++ gurus have further information to add here?  Is my (simple)
>> analysis here incorrect?  If not, seems we should forbid boolean values in
>> multi-argument methods/constructors in our style and add checks to
>> check-webkit-style to prevent further introduction of these confusing
>> callsites.
>>
>> -eric
>>
>
>
> I was under the impression that this was already an encouraged style in
> WebKit code.  At least, I really like that is makes call-sites more
> self-documenting.
>
>
> The only exception I would make to this rule is if all the call sites use
> variables and never pass in raw true or false.  In that case there's no loss
> of readability, and whether you use an enum vs. a bool is irrelevant.
>
> I think in general the rule should be "Keep your call sites readable, and
> convert to enums if you find that the call sites are becoming inscrutable."
>
> dave
> (hy...@apple.com)
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread David Hyatt
On Dec 3, 2010, at 3:33 PM, Darin Fisher wrote:

> On Fri, Dec 3, 2010 at 1:28 PM, Eric Seidel  wrote:
> It seems to me, that using bool types for function arguments is strictly 
> worse than using an enum.  An enum is always clearer and can be easily casted 
> to a bool if needed.
> 
> doSomething(something, false);
> 
> Is much less readable than:
> 
> doSomething(something, AllowNetworkLoads);
> 
> 
> Do any C++ gurus have further information to add here?  Is my (simple) 
> analysis here incorrect?  If not, seems we should forbid boolean values in 
> multi-argument methods/constructors in our style and add checks to 
> check-webkit-style to prevent further introduction of these confusing 
> callsites.
> 
> -eric
> 
> 
> I was under the impression that this was already an encouraged style in 
> WebKit code.  At least, I really like that is makes call-sites more 
> self-documenting.

The only exception I would make to this rule is if all the call sites use 
variables and never pass in raw true or false.  In that case there's no loss of 
readability, and whether you use an enum vs. a bool is irrelevant.

I think in general the rule should be "Keep your call sites readable, and 
convert to enums if you find that the call sites are becoming inscrutable."

dave
(hy...@apple.com)

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Peter Kasting
On Fri, Dec 3, 2010 at 1:28 PM, Eric Seidel  wrote:

> It seems to me, that using bool types for function arguments is strictly
> worse than using an enum.  An enum is always clearer and can be easily
> casted to a bool if needed.
>
> doSomething(something, false);
>
> Is much less readable than:
>
> doSomething(something, AllowNetworkLoads);
>

I agree that in general, the latter is better.

Do any C++ gurus have further information to add here?
>

Restricting bools can limit your ability to pass results of one function as
arguments to another.  It is plausible to have function A return a different
"enumerated boolean" type than function B takes, for example, if one type is
a special-case of the other.  This means you'll need to add explicit ?: ops,
conditionals, or casts, all of which make me more unhappy than the original
bools did.

This is probably an issue in a minority of cases, but it might be reason to
use "should" rather than "must" language here.

PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Darin Fisher
On Fri, Dec 3, 2010 at 1:28 PM, Eric Seidel  wrote:

> It seems to me, that using bool types for function arguments is strictly
> worse than using an enum.  An enum is always clearer and can be easily
> casted to a bool if needed.
>
> doSomething(something, false);
>
> Is much less readable than:
>
> doSomething(something, AllowNetworkLoads);
>
>
> Do any C++ gurus have further information to add here?  Is my (simple)
> analysis here incorrect?  If not, seems we should forbid boolean values in
> multi-argument methods/constructors in our style and add checks to
> check-webkit-style to prevent further introduction of these confusing
> callsites.
>
> -eric
>


I was under the impression that this was already an encouraged style in
WebKit code.  At least, I really like that is makes call-sites more
self-documenting.

-Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


[webkit-dev] Bools are strictly worse than enums

2010-12-03 Thread Eric Seidel
It seems to me, that using bool types for function arguments is strictly
worse than using an enum.  An enum is always clearer and can be easily
casted to a bool if needed.

doSomething(something, false);

Is much less readable than:

doSomething(something, AllowNetworkLoads);


Do any C++ gurus have further information to add here?  Is my (simple)
analysis here incorrect?  If not, seems we should forbid boolean values in
multi-argument methods/constructors in our style and add checks to
check-webkit-style to prevent further introduction of these confusing
callsites.

-eric
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


[webkit-dev] how to write layout test to test mouse wheeling on document

2010-12-03 Thread Xiaomei Ji
Webkit now supports horizontal scrolling for pages with left overflow.
But it has a bug in cross-platform that the backward mouse wheel does not
work (you can not scroll backward below (0,0)).
https://bugs.webkit.org/show_bug.cgi?id=50370

I tested the fix posted in the above bug, and I can manually mouse wheel a
RTL page backward and forward horizontally and vertically.

But I am not able to write a layout test on it.
Following is what I wrote. The test passes with and without the fix. It
should fail without the fix because horizontal backward mouse wheel is not
working without the fix.
I tested in Chromium Linux, and I looked at EventSender.cpp in Chromium's
DRT and did not see any problems there.

I am not sure whether I am writing the test correctly. Appreciate your help!








if (window.layoutTestController) {
layoutTestController.waitUntilDone();
}

function dispatchWheelEvent()
{
window.addEventListener("mousewheel", mousewheelHandler,
false);

window.scrollTo(-100, 100);
if (window.eventSender) {
eventSender.continuousMouseScrollBy(-10, -10);
eventSender.continuousMouseScrollBy(30, 30);
}

setTimeout('checkOffsets();', 100);
}

function checkOffsets()
{
shouldBe("window.scrollX", "-120");
shouldBe("window.scrollY", "80");

if (window.layoutTestController)
window.layoutTestController.notifyDone();
}

function mousewheelHandler(e)
{
}





Lorem ipsum dolor sit amet, consectetur
adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna
aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris
nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in
reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla
pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui
officia deserunt mollit anim id est laborum."





Thanks,
Xiaomei
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Webkit-Javascript-Objc problem

2010-12-03 Thread Darin Adler
This is the wrong mailing list for this question. This is a question for 
discussion of the development of WebKit, not discussion of programming with 
WebKit. This page  lists some options and 
places you might ask this question.

If you suspect this is a bug and want to file a bug report, you can make a 
reduced test application demonstrating the problem and include it in the bug 
report. This page  talks about how to 
report bugs.

-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


[webkit-dev] Webkit-Javascript-Objc problem

2010-12-03 Thread Robert Vasvari
Hi All,

I have a strange problem using the Javascript-Objc bridge. I wrote a
simple webkit plugin, using the sample provided. I properly
implemented isKeyExcludedFromWebScript: , webScriptNameForSelector and
all the others in the protocol. I have a test page with javascript in
it that passes values into my objc plugin, both as setting properties
and calling functions with arguments. It all works. But then when I
connect to the real site
which is an ASP website running on Windows get a weird error. The
plugin loads properly, the property-methods do get called, but with
nil argument. I have a simple NSString ivar "stringVar" in the plugin
that I am trying to set from javascript like this:

myplugin.stringVar = "someValue";

somehow, the argument gets nulled out in the Javascript-Objc bridge. I
do get the method
setStringVar:  called with nil as an argument.

Any ideas welcome...

Robert
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] [chromium] using WEBKIT_API properly

2010-12-03 Thread Darin Fisher
Yes, indeed.  Thanks Jeremy!

On Fri, Dec 3, 2010 at 3:13 AM, Jeremy Orlow  wrote:

> You forgot to mention virtual functions, which is another case where you do
> _not_ use WEBKIT_API.
>
> J
>
> On Thu, Dec 2, 2010 at 9:27 PM, Darin Fisher  wrote:
>
>> If you do not work on the Chromium port of WebKit, you can stop reading
>> now.
>>
>> I've noticed that there is some confusion about how to use WEBKIT_API
>> properly.
>> WEBKIT_API causes a function to be exported from WebKit when it is built
>> as a DLL,
>> allowing Chromium to call the function.
>>
>> The rule is actually quite simple:
>>
>>WEBKIT_API should be affixed to any public, non-inline function that is
>> intended
>>for the embedder (Chromium) to call.
>>
>> Put another way:
>> -- Do not apply WEBKIT_API to inline functions.
>> -- Do not apply WEBKIT_API to private functions.
>> -- Do not apply WEBKIT_API to public functions within a #if
>> WEBKIT_IMPLEMENTATION block.
>>
>> (Of related note, we never put WEBKIT_API on public constructors and
>> destructors.
>> Instead, we have constructors call an initialize method and destructors
>> call a reset
>> method.  Those then end up having the WEBKIT_API prefix applied.)
>>
>> Thanks!
>> -Darin
>>
>>
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>
>>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] [chromium] using WEBKIT_API properly

2010-12-03 Thread Jeremy Orlow
You forgot to mention virtual functions, which is another case where you do
_not_ use WEBKIT_API.

J

On Thu, Dec 2, 2010 at 9:27 PM, Darin Fisher  wrote:

> If you do not work on the Chromium port of WebKit, you can stop reading
> now.
>
> I've noticed that there is some confusion about how to use WEBKIT_API
> properly.
> WEBKIT_API causes a function to be exported from WebKit when it is built as
> a DLL,
> allowing Chromium to call the function.
>
> The rule is actually quite simple:
>
>WEBKIT_API should be affixed to any public, non-inline function that is
> intended
>for the embedder (Chromium) to call.
>
> Put another way:
> -- Do not apply WEBKIT_API to inline functions.
> -- Do not apply WEBKIT_API to private functions.
> -- Do not apply WEBKIT_API to public functions within a #if
> WEBKIT_IMPLEMENTATION block.
>
> (Of related note, we never put WEBKIT_API on public constructors and
> destructors.
> Instead, we have constructors call an initialize method and destructors
> call a reset
> method.  Those then end up having the WEBKIT_API prefix applied.)
>
> Thanks!
> -Darin
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev