ui/gfx/text_elider.h - Issue 381953002: New avatar button: Consolidate text elision between Mac and Win/Linux

Side by Side Diff: ui/gfx/text_elider.h

Issue 381953002: New avatar button: Consolidate text elision between Mac and Win/Linux (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: use TruncateString Created 6 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch | Annotate | Revision Log

« chrome/browser/ui/cocoa/profiles/avatar_button_controller.mm ('K') | « chrome/browser/ui/views/profiles/new_avatar_button.cc ('k') | ui/gfx/text_elider.cc » ('j') | no next file with comments »

OLD	NEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.	1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4 //	4 //

5 // This file defines utility functions for eliding and formatting UI text.	5 // This file defines utility functions for eliding and formatting UI text.

6	6

7 #ifndef UI_GFX_TEXT_ELIDER_H_	7 #ifndef UI_GFX_TEXT_ELIDER_H_

8 #define UI_GFX_TEXT_ELIDER_H_	8 #define UI_GFX_TEXT_ELIDER_H_

9	9

10 #include <string>	10 #include <string>

(...skipping 140 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
151 int available_pixel_height,	151 int available_pixel_height,

152 WordWrapBehavior wrap_behavior,	152 WordWrapBehavior wrap_behavior,

153 std::vector<base::string16>* lines);	153 std::vector<base::string16>* lines);

154	154

155 // Truncates the string to length characters. This breaks the string at	155 // Truncates the string to length characters. This breaks the string at

156 // the first word break before length, adding the horizontal ellipsis	156 // the first word break before length, adding the horizontal ellipsis

157 // character (unicode character 0x2026) to render ...	157 // character (unicode character 0x2026) to render ...

158 // The supplied string is returned if the string has length characters or	158 // The supplied string is returned if the string has length characters or

159 // less.	159 // less.

160 GFX_EXPORT base::string16 TruncateString(const base::string16& string,	160 GFX_EXPORT base::string16 TruncateString(const base::string16& string,

161 size_t length);	161 size_t length,

	162 bool only_at_word_breaks);
	msw 2014/07/17 17:18:35 nit: maybe use gfx::BreakType or base::i18n::Break nit: maybe use gfx::BreakType or base::i18n::BreakIterator::BreakType? Marc Treib 2014/07/18 10:05:09 Hm, base::i18n::BreakIterator::BreakType has lots Show quoted text On 2014/07/17 17:18:35, msw wrote: > nit: maybe use gfx::BreakType or base::i18n::BreakIterator::BreakType? Hm, base::i18n::BreakIterator::BreakType has lots of cases that aren't handled separately here, and it might be confusing that the behavior here doesn't quite match that of a corresponding BreakIterator. gfx::BreakType has WORD_BREAK and LINE_BREAK which would probably mean the same thing here? But there's a TODO to merge it with base::i18n::BreakIterator::BreakType, so.. hm. I agree that an enum would be better here than a bool, but neither of these seem to really fit, and I wouldn't like to introduce yet another one. msw 2014/07/18 19:21:48 gfx::BreakType seems appropriate for now. When my Show quoted text On 2014/07/18 10:05:09, treib wrote: > On 2014/07/17 17:18:35, msw wrote: > > nit: maybe use gfx::BreakType or base::i18n::BreakIterator::BreakType? > > Hm, base::i18n::BreakIterator::BreakType has lots of cases that aren't handled > separately here, and it might be confusing that the behavior here doesn't quite > match that of a corresponding BreakIterator. > gfx::BreakType has WORD_BREAK and LINE_BREAK which would probably mean the same > thing here? But there's a TODO to merge it with > base::i18n::BreakIterator::BreakType, so.. hm. > I agree that an enum would be better here than a bool, but neither of these seem > to really fit, and I wouldn't like to introduce yet another one. gfx::BreakType seems appropriate for now. When my TODO is addressed, it can be replaced with a [likely leaner] base::i18n::BreakIterator::BreakType. It already supported WORD_BREAK and now supports CHARACTER_BREAK, and could later support LINE_BREAK (start by finding the first newline before the argument length). If we are adding an argument to support some addition breaking type, it should be an enum, not a bool. Marc Treib 2014/07/21 09:03:00 Done. Show quoted text On 2014/07/18 19:21:48, msw wrote: > On 2014/07/18 10:05:09, treib wrote: > > On 2014/07/17 17:18:35, msw wrote: > > > nit: maybe use gfx::BreakType or base::i18n::BreakIterator::BreakType? > > > > Hm, base::i18n::BreakIterator::BreakType has lots of cases that aren't handled > > separately here, and it might be confusing that the behavior here doesn't > quite > > match that of a corresponding BreakIterator. > > gfx::BreakType has WORD_BREAK and LINE_BREAK which would probably mean the > same > > thing here? But there's a TODO to merge it with > > base::i18n::BreakIterator::BreakType, so.. hm. > > I agree that an enum would be better here than a bool, but neither of these > seem > > to really fit, and I wouldn't like to introduce yet another one. > > gfx::BreakType seems appropriate for now. When my TODO is addressed, it can be > replaced with a [likely leaner] base::i18n::BreakIterator::BreakType. It already > supported WORD_BREAK and now supports CHARACTER_BREAK, and could later support > LINE_BREAK (start by finding the first newline before the argument length). If > we are adding an argument to support some addition breaking type, it should be > an enum, not a bool. Done.
162	163

163 } // namespace gfx	164 } // namespace gfx

164	165

165 #endif // UI_GFX_TEXT_ELIDER_H_	166 #endif // UI_GFX_TEXT_ELIDER_H_

OLD	NEW