[chromium-dev] Re: Extending Value with std::string support

2008-12-08 Thread Peter Kasting
On Mon, Dec 8, 2008 at 5:28 PM, Andrew Scherkus <[EMAIL PROTECTED]>wrote: > 1) Overload CreateStringValue, GetAsString, etc.. to also accept > std::string. Add TYPE_UTF8_STRING to ValueType enum. > 2) Overload CreateStringValue, GetAsString, etc.. to also accept > std::string. TYPE_STRING becomes

[chromium-dev] Re: Extending Value with std::string support

2008-12-08 Thread Andrew Scherkus
Darin touched upon this, who said to document that std::string should refer to UTF-8 strings. How about: - CreateStringValue creates a StringValue object that returns TYPE_UTF8_STRING and has a DCHECK(IsStringUTF8(foo)) in the constructor - CreateWideStringValue creates a WideStringValue object t

[chromium-dev] Re: Extending Value with std::string support

2008-12-08 Thread Peter Kasting
On Mon, Dec 8, 2008 at 6:41 PM, Andrew Scherkus <[EMAIL PROTECTED]>wrote: > Darin touched upon this, who said to document that std::string should refer > to UTF-8 strings. > How about: > - CreateStringValue creates a StringValue object that returns > TYPE_UTF8_STRING and has a DCHECK(IsStringUTF8

[chromium-dev] Re: Extending Value with std::string support

2008-12-08 Thread Brett Wilson
On Mon, Dec 8, 2008 at 7:50 PM, Peter Kasting <[EMAIL PROTECTED]> wrote: > On Mon, Dec 8, 2008 at 6:41 PM, Andrew Scherkus <[EMAIL PROTECTED]> > wrote: >> >> Darin touched upon this, who said to document that std::string should >> refer to UTF-8 strings. >> How about: >> - CreateStringValue creat

[chromium-dev] Re: Extending Value with std::string support

2008-12-08 Thread Andrew Scherkus
Somewhat in line with the Google style guide, the overloaded CreateStringValue/GetString do accomplish the same thing (variant string type), just with different encodings. I did some partial implementations of #3 and as Peter highlighted, writing GetWideString everywhere started looking really sill

[chromium-dev] Re: Extending Value with std::string support

2008-12-08 Thread Brett Wilson
On Mon, Dec 8, 2008 at 8:32 PM, Andrew Scherkus <[EMAIL PROTECTED]> wrote: > Somewhat in line with the Google style guide, the overloaded > CreateStringValue/GetString do accomplish the same thing (variant string > type), just with different encodings. > I did some partial implementations of #3 an

[chromium-dev] Re: Extending Value with std::string support

2008-12-08 Thread Andrew Scherkus
On Mon, Dec 8, 2008 at 8:53 PM, Brett Wilson <[EMAIL PROTECTED]> wrote: > > On Mon, Dec 8, 2008 at 8:32 PM, Andrew Scherkus <[EMAIL PROTECTED]> > wrote: > > Somewhat in line with the Google style guide, the overloaded > > CreateStringValue/GetString do accomplish the same thing (variant string > >

[chromium-dev] Re: Extending Value with std::string support

2008-12-09 Thread Peter Kasting
On Mon, Dec 8, 2008 at 8:09 PM, Brett Wilson <[EMAIL PROTECTED]> wrote: > Be careful because wstring != UTF16String. > Yes, hence the need to be very explicit about what these functions actually do (since you can't infer things from types alone). In other places of the code, we use GetWString, w

[chromium-dev] Re: Extending Value with std::string support

2008-12-09 Thread tony
So there's a single StringValue class, but sometimes its type is TYPE_UTF8_STRING and sometimes its type is TYPE_UTF16_STRING? So calling Value::IsType would require two checks to see what type of string it is? I think it would be easier if there was only one string type and internally it keeps

[chromium-dev] Re: Extending Value with std::string support

2008-12-09 Thread Brett Wilson
On Tue, Dec 9, 2008 at 10:38 AM, <[EMAIL PROTECTED]> wrote: > > So there's a single StringValue class, but sometimes its type is > TYPE_UTF8_STRING and sometimes its type is TYPE_UTF16_STRING? So calling > Value::IsType would require two checks to see what type of string it is? > > I think it wo

[chromium-dev] Re: Extending Value with std::string support

2008-12-09 Thread tony
That's fine with me as well. Internally, it's always utf8 but we would add helper methods on StringValue so you can CreateStringValue with a wstring or call GetWString. On Tue, 9 Dec 2008, Brett Wilson wrote: > > On Tue, Dec 9, 2008 at 10:38 AM, <[EMAIL PROTECTED]> wrote: > > > > So there's a

[chromium-dev] Re: Extending Value with std::string support

2008-12-09 Thread Andrew Scherkus
Changing the callers would be a much bigger job. If switching to std::string is something we're really interested in, then I'd view this patch as a stepping stone towards reaching that goal. I've got a change ready, but it gives you a good idea on who's using StringValue right now: http://coderev

[chromium-dev] Re: Extending Value with std::string support

2008-12-09 Thread tony
I think it would be ok to have GetString and GetAsString both be overloaded to work with std::string and std::wstring while we transition the callers. It's the same as having a single CreateStringValue that takes std::string and std::wstring. Having 2 string types is really confusing and I think

[chromium-dev] Re: Extending Value with std::string support

2008-12-09 Thread Peter Kasting
On Tue, Dec 9, 2008 at 1:17 PM, <[EMAIL PROTECTED]> wrote: > > I think it would be ok to have GetString and GetAsString both be > overloaded to work with std::string and std::wstring while we transition > the callers. It's the same as having a single CreateStringValue that > takes std::string and

[chromium-dev] Re: Extending Value with std::string support

2008-12-09 Thread Andrew Scherkus
Looking through some of the code again it gets a bit scary when there's code checking for TYPE_WSTRING but not the other. So how about: CreateStringValue accepts std::string and std::wstring SetString accepts std::string and std::wstring GetString can return std::string or std::wstring and uses laz

[chromium-dev] Re: Extending Value with std::string support

2008-12-09 Thread Evan Martin
On Tue, Dec 9, 2008 at 1:58 PM, Andrew Scherkus <[EMAIL PROTECTED]> wrote: > Looking through some of the code again it gets a bit scary when there's code > checking for TYPE_WSTRING but not the other. > So how about: > CreateStringValue accepts std::string and std::wstring > SetString accepts std: