Bug 85154 – Add String::startsWith() and endsWith() for string literals

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Benjamin Poulain]

Add String::startsWith() and endsWith() for string literals

[Benjamin Poulain]

Created attachment 139396 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 139396 [details]
Patch

Attachment 139396 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12557841

[Build Bot]

Comment on attachment 139396 [details]
Patch

Attachment 139396 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12557838

[Early Warning System Bot]

Comment on attachment 139396 [details]
Patch

Attachment 139396 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12559830

[Early Warning System Bot]

Comment on attachment 139396 [details]
Patch

Attachment 139396 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12550823

[Benjamin Poulain]

Created attachment 139397 [details]
Patch

[Benjamin Poulain]

Created attachment 139398 [details]
Patch

[Darin Adler]

Comment on attachment 139398 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=139398&action=review

> Source/WTF/ChangeLog:9
> +        a string litteral, a new String was constructed implicitely, allocating

litteral -> literal
implicitely -> implicitly

> Source/WTF/ChangeLog:10
> +        a new StringImpl and copying the caracters for the operation.

caracters -> characters

> Source/WTF/ChangeLog:12
> +        This patch adds a version of those methods for single character and

single character -> single characters
litterals -> literals

> Source/WTF/wtf/text/AtomicString.h:87
> +    bool startsWith(const char* s, unsigned matchLength, bool caseSensitive = true) const
> +        { return m_string.startsWith(s, matchLength, caseSensitive); }

Is this overload used anywhere?

> Source/WTF/wtf/text/AtomicString.h:97
> +    bool endsWith(const char* s, unsigned matchLength, bool caseSensitive = true) const
> +        { return m_string.endsWith(s, matchLength, caseSensitive); }

Same question.

> Source/WTF/wtf/text/StringImpl.cpp:1118
> +    if (length())
> +        return this->operator[](0) == character;
> +    return false;

I would write this differently:

    return m_length && (*this)[0] == character;

> Source/WTF/wtf/text/StringImpl.cpp:1143
> +    if (length())
> +        return this->operator[](length() - 1) == character;
> +    return false;

I would write this differently:

    return m_length && (*this)[m_length - 1] == character;

> Source/WTF/wtf/text/StringImpl.cpp:1153
> +    ASSERT(matchLength);
> +    if (matchLength > length())
> +        return false;
> +    unsigned startOffset = length() - matchLength;
> +
> +    return equalInner(this, startOffset, matchString, matchLength, caseSensitive);

The paragraphing here seems strange. I’d leave out the blank line.

[Benjamin Poulain]

Thanks for the review. I should use a spellchecker for my change logs...

> > Source/WTF/wtf/text/AtomicString.h:87
> > +    bool startsWith(const char* s, unsigned matchLength, bool caseSensitive = true) const
> > +        { return m_string.startsWith(s, matchLength, caseSensitive); }
> 
> Is this overload used anywhere?
> 
> > Source/WTF/wtf/text/AtomicString.h:97
> > +    bool endsWith(const char* s, unsigned matchLength, bool caseSensitive = true) const
> > +        { return m_string.endsWith(s, matchLength, caseSensitive); }
> 
> Same question.

They are not, I just figured that could be useful in the future.

[Benjamin Poulain]

Committed r115669: <http://trac.webkit.org/changeset/115669>