Refactor URL formatting code into its own module.

[michaelsproul]

I've been attempting to implement Show for Iron's custom URL type, and I want to avoid duplicating all of rust-url's formatting code.

I've made two formatters publicly available in their own module, for URL paths and userinfo (usernames + passwords). This should allow people writing their own extensions to rust-url to implement Show for their types.

I wasn't quite sure where to put the tests, but they seem quite happy in a little sub-module. I figured the tests module was better suited to parsing/library tests.

Related: iron/iron#130

[michaelsproul]

Not sure about the API here. In many ways a &str is neater, but it implies we should switch to Option<&str> for the password field, and &[&str] for the path field in PathFormatter.

The path is what worries me, the shortest expression I have to convert a Vec<String> to a &[&str] is this...

path.iter().map(|s| s.as_slice()).collect::<Vec<&str>>().as_slice()

I expected this to fail the borrow-checker, but it doesn't. Makes me wonder if some sort of map_slice() method would be useful on vectors, as I imagine this is a common pattern.

[SimonSapin]

It doesn’t seem worth the bother.

[SimonSapin]

Could you also move UrlNoFragmentFormatter into the new format module?

[michaelsproul]

I've moved the UrlNoFragmentFormatter and used generics to handle the PathFormatter API problem. It now accepts both &[String] and &[&str] 😄.

[SimonSapin]

Looks good, thanks!

[SimonSapin]

@michaelsproul Here is another possible refactoring for #15, not merged yet: https://github.com/servo/rust-url/compare/textwriters

What do you think? Would this work for Iron?

[michaelsproul]

Yeah, looks like it would work for Iron, no worries.

Do you think the TextWriter and build_string! system offers a significant benefit over using Show and format!? When I wrote the formatting code I imagined that people needing strings could create PathFormatter and UserInfoFormatter objects as required and use them like so:

let string = format!("{}", PathFormatter { path: path });

I worry that introducing more generic formatting code only complicates things further when the Show system is already quite fully featured and widely used.

In Iron at the moment I call the Show::fmt method quite a bit, which is nice to have.

https://github.com/iron/iron/blob/master/src/request/url.rs#L118

[SimonSapin]

I’m not too sure of the design, which is why it’s not merged yet.

The problem with Show is that you need a type to implement it on. If you want re-usable pieces of formatting, you end up with types like PathFormatter that have no other purpose than to implement Show on them. I don’t really like this. You can have various fmt-like functions that can be called from each other or from a Show impl, but you can not create a fmt::Formatter without Show being involved.

fmt::Formatter happens to implement io::Writer, so another approach is to have functions that take a Writer instead of a TextWriter, and have build_string! use a MemWriter. I have another version of this patch that does this. Do you think it would be preferable?

But that version of build_string!, like format!(), includes a redundant UTF-8 check. Hence TextWriter. I have pushed for Unicode streams in the past, and I really think that Show::fmt should be based on them. I’m hoping that someday it will.

So I don’t know. It’s not easy to strike a balance between being in harmony with the ecosystem as it exists now, and pulling it in the direction is think is beneficial by showing the way.

[michaelsproul]

Oooh, I am seeing where you're coming from now.

TextWriter seems like a great idea, I prefer it to using Writer. I think your current approach in the textwriters branch is merge-able.

Would it make sense to create a separate project for TextWriter to encourage wide spread adoption? Or could we try to get it added to rust-encoding?

[SimonSapin]

rust-encoding has something similar already. I’d like to do some work on the rust-encoding API to make it as general as possible (including Unicode streams) and then unify. But you know, only so many hours every day :)