[webkit-reviews] review denied: [Bug 25494] add PositionConstructors.h in preparation for adding more position constructor functions : [Attachment 29924] Add PositionConstructors.h

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 22 01:35:40 PDT 2009


Maciej Stachowiak <mjs at apple.com> has denied Eric Seidel <eric at webkit.org>'s
request for review:
Bug 25494: add PositionConstructors.h in preparation for adding more position
constructor functions
https://bugs.webkit.org/show_bug.cgi?id=25494

Attachment 29924: Add PositionConstructors.h
https://bugs.webkit.org/attachment.cgi?id=29924&action=review

------- Additional Comments from Maciej Stachowiak <mjs at apple.com>
I don't think calling this header PositionConstructors.h is right.
"Constructor" means a specific thing in C++, and these aren't it, so it's kind
of confusing.

I actually think it's fine for these functions to be in Position.h, since they
are effectively part of the interface to Position, but if you can think of a
good alternate header name, that's ok too.

Also, while I'm nitpicking, I don't think the names of functions that take
nodes necessarily need Node in them.It could just be:

static inline Position positionBefore(const Node* node)
static inline Position positionAfter(const Node* node)
static inline Position firstDeepEditingPosition(Node* anchorNode)
static inline Position lastDeepEditingPosition(Node* anchorNode)

r- for the header name. Function renames are optional.


More information about the webkit-reviews mailing list