Issue 20484: Adds a method to sort the elements of a vector by invoking a method on...

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 20484: Adds a method to sort the elements of a vector by invoking a method on... (Closed)

Created:
11 years, 10 months ago by sky

Modified:
9 years, 7 months ago

Reviewers:
tony

CC:
chromium-reviews_googlegroups.com

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Adds a method to sort the elements of a vector by invoking a method on each that returns a string. The strings are then compared using a collator, or operator < if no collator is found. I'm going to use this for sorting bookmarks. BUG=1750 TEST=none yet Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=10252

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 2

Patch Set 4 : '' #

Patch Set 5 : '' #

Patch Set 6 : '' #

Created: 11 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+127 lines, -35 lines)			Patch
M	chrome/common/l10n_util.h	View	1 2 3	2 chunks	+69 lines, -1 line	0 comments	Download
M	chrome/common/l10n_util.cc	View	1 2 3 4	3 chunks	+29 lines, -31 lines	0 comments	Download
M	chrome/common/l10n_util_unittest.cc	View	1 2 3 4 5	4 chunks	+29 lines, -3 lines	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

tony

11 years, 10 months ago (2009-02-20 02:10:46 UTC) #2

LGTM.

http://codereview.chromium.org/20484/diff/10/13
File chrome/common/l10n_util.h (right):

http://codereview.chromium.org/20484/diff/10/13#newcode23
Line 23: #include "third_party/icu38/public/common/unicode/ubidi.h"
Nit: While you're here, can you convert this include to just "unicode/ubidi.h"?
I think that's more consistent with other code that uses icu headers.

http://codereview.chromium.org/20484/diff/10/13#newcode184
Line 184: const std::wstring& lhs,
How hard would it be to make all this use string16 instead of wstring?  In the
end, that's what we want to do (dsh is working on replacing all wstrings with
string16).  If this results in lots of changes to bookmarks code, nevermind.

Expand Messages | Collapse Messages