[Webkit-unassigned] [Bug 61495] Make RenderStyle visuallyOrdered property use an enum instead of a bool.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=61495


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #95088|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #9 from Eric Seidel <eric at webkit.org>  2011-06-06 17:40:38 PST ---
(From update of attachment 95088)
View in context: https://bugs.webkit.org/attachment.cgi?id=95088&action=review

> Source/WebCore/rendering/style/RenderStyle.h:362
> +    Order visuallyOrdered() const { return static_cast<Order>(inherited_flags._visuallyOrdered); }
> +    void setVisuallyOrdered(Order o) { inherited_flags._visuallyOrdered = o; }

The naming gets confused here.  The current names (mostly) look like they take/return bools.  Technically it should be isVisuallyOrdered(), but some old parts of the code don't follow the "is" style.  If we're changing thsi to an enum, it makes better sense to name it something like rtlOrdering().  Which I would argue is still better than the old code because it's not immediately clear that the opposite of visual ordering is logical ordering.

If it's not too much work, I would rename this to rtlOrdering and setRTLOrdering and we can be done. :)

As is, I'm not sure the bool -> enum conversion is worth it in this case, as it doesnt' really add clarity.  The clarity comes when replacing callsites which pass "true" or "false" to functions which take more than one argument.  In this case, isFoo and setFoo are pretty clear when passed true/false.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.