Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(966)

Issue 6130046: Add SkString.contains() (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 7 months ago by epoger
Modified:
12 years, 7 months ago
Reviewers:
reed1
CC:
skia-review_googlegroups.com
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Add SkString.contains() Committed: https://code.google.com/p/skia/source/detail?r=3781

Patch Set 1 #

Total comments: 2

Patch Set 2 : moved SkStrContains() definition into header #

Patch Set 3 : add unittests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -7 lines) Patch
M include/core/SkString.h View 1 2 chunks +13 lines, -1 line 0 comments Download
M src/core/SkString.cpp View 1 1 chunk +0 lines, -6 lines 0 comments Download
M tests/StringTest.cpp View 1 2 1 chunk +13 lines, -0 lines 0 comments Download

Messages

Total messages: 4
epoger
12 years, 7 months ago (2012-04-27 15:49:02 UTC) #1
reed1
I think we can/should put the impl of this new function inlined in the header, ...
12 years, 7 months ago (2012-04-27 15:57:46 UTC) #2
epoger
https://codereview.appspot.com/6130046/diff/1/src/core/SkString.cpp File src/core/SkString.cpp (right): https://codereview.appspot.com/6130046/diff/1/src/core/SkString.cpp#newcode69 src/core/SkString.cpp:69: SkASSERT(substring); On 2012/04/27 15:57:47, reed1 wrote: > return NULL ...
12 years, 7 months ago (2012-04-27 16:08:32 UTC) #3
reed1
12 years, 7 months ago (2012-04-27 16:48:07 UTC) #4
lgtm -- lets add a couple of unittests either now or in a follow-up CL
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b