Thanks for doing this.

The .to_owned method can also be removed and replaced by .to_strbuf... which should really be renamed to .to_string.

Yup, thats my plan. I'll get it together tonight.
On 24/05/2014 10:24 pm, "Huon Wilson" notifications@github.com wrote:

Thanks for doing this.

The .to_owned method can also be removed and replaced by .to_strbuf...
which should really be renamed to .to_string.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/14414#issuecomment-44112988
.

ok, so I'm making good progress on this, however just as FUD that seems worth raising, having to do

"foobar".to_string()

everywhere seems weird and confusing. We should look seriously at how to fix this.

ok, patches updated. Ignore the commits, if the diff looks OK I'll rebase them into something mergable.

Seems ok, but yeah, maybe it would be nice to have a few more opinions on the .to_string name.

I also think that we need to reconcile the to_str method from the ToStr trait with the to_string method for getting an instance of String.

I believe that the name to_string is appropriate because in rust a string literal will be a "str" and the concept of a String is different. That's what we bought into with the names we chose.

I don't see why to_str wouldn't be the way you get a String, and we could name it to_string.

OK, this is enough to be actionable. Its going to be another monstrous pain
to get merged I suspect so thanks in advance for bearing with me :)
On 25/05/2014 1:36 pm, "Corey Richardson" notifications@github.com wrote:

I don't see why to_str wouldn't be the way you get a String, and we could
name it to_string.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/14414#issuecomment-44144851
.

BTW, any removed functions/methods should be not removed right now, but marked with #[deprecated = "use ...], as described here.

The stdlibs should still be updated to avoid the old functions, but keeping a dummy implementation like the following is good:

#[deprecated = "use `.to_string`"]
fn to_owned(&self) -> String { self.to_string() }

(This includes from_owned_str etc.)

I think this should be good to go now.

I rebased on master, and deprecated instead of removing those string functions.

The travis failure is relevant.

Yup, fixed some but not all (I botched the rebase)

Will ping when its green
On 26/05/2014 11:38 pm, "Huon Wilson" notifications@github.com wrote:

The travis failure is relevant.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/14414#issuecomment-44240278
.

This includes a bunch of places where to_string is actually called on a String, I'm going to treat this as an excuse to play with static analysis a little more to detect these cases, however I tend to this that this should be merged asap (When the build gets green) because rebasing this on master all the time is painful.

make check-stage1 NO_REBUILD=1 NO_BENCH=1 just passed locally for me. Just waiting on Travis.

confirmed tests pass locally. r?