Add conversion traits to Builder #12

Ghost · 2020-11-30T04:36:19Z

Ghost commented

2020-11-30 04:36:19 +00:00

First-time contributor

This adds several traits for converting a Builder into different things, such as:

A String via ToString,
&[Node] via AsRef<[Node]>
&mut [Node] via AsMut<[Node]>
Vec<Node> via From<Builder> for Vec<Node>

Some of these changes are pretty small as far as what some users might see (e.g. you might as well call .build() instead of .into(), but when writing methods that work with Nodes, it might be very nice to be able to accept Into<Vec<Node>> or AsRef<[Node]>, which would allow you to take in either a Builder or a finished Vec<Node>.

The ToString method also adds some very nice functionality in that it makes rendering a document to a String much easier and readable, as compared to constructing a buffer, rendering to a mutable reference of the buffer, and converting the buffer back to a String, all the while handling a mess of errors

Major Changes

Implementation of the str::ToString, AsRef<[Node]>, and AsMut<[Node]> for Builder
Accepting any AsRef<[Node]> in render() (including accepting the old Vec<Node>, so not breaking)
Addition of estimate_len() to Node, used to pre-allocate the correct size of the String buffer
Implement From<Builder> on Vec<Node>

Design Decisions

estimate_len() has some quick doctests and examples. I know most of the rest of the project uses test methods, but I hope this is alright given that the tests may add some more clarity to the purpose and function of the method.
to_string() has a single line of unsafe code. As the associated comment explains, this is provably safe, and exists just to avoid having to choose between having a bunch of duplicate code or inefficiently performing a UTF-8 check on a whole bunch bytes that we already know are safe. That said, I totally get it if you're just generally against unsafe code and will change it to be an alternative if you so wish
ToString is implemented instead of Display. This is to discourage users from directly using this in a println!() or write!() macro, which would not be a thing you would normally expect to do with this. It also gives us the advantage of being able to pre-allocate a buffer size, meaning less expensive String resizing.
I couldn't think of a clever way to get render() to work with both io::Writes or fmt::Writes without duplicating the code, but I'm dumb and might be missing something, so if there's a way to do that instead of doing my funky unsafe hack that's cool and I can do that instead.

Also hi! Sorry for barging in on your nice gitea instance without asking first I just kinda wanted to submit this PR. If that wasn't cool I can go tho. I'm a singlet but not like the mean kind I can be nice I promise but also I totally get it if you don't want me here sorry thanks

This adds several traits for converting a Builder into different things, such as: - A `String` via `ToString`, - `&[Node]` via `AsRef<[Node]>` - `&mut [Node]` via `AsMut<[Node]>` - `Vec<Node>` via `From<Builder> for Vec<Node>` Some of these changes are pretty small as far as what some users might see (e.g. you might as well call `.build()` instead of `.into()`, but when writing methods that work with Nodes, it might be very nice to be able to accept `Into<Vec<Node>>` or `AsRef<[Node]>`, which would allow you to take in *either* a `Builder` or a finished `Vec<Node>`. The `ToString` method also adds some very nice functionality in that it makes rendering a document to a String *much* easier and readable, as compared to constructing a buffer, rendering to a mutable reference of the buffer, and converting the buffer back to a `String`, all the while handling a mess of errors ## Major Changes * Implementation of the `str::ToString`, `AsRef<[Node]>`, and `AsMut<[Node]>` for `Builder` * Accepting any `AsRef<[Node]>` in `render()` (including accepting the old `Vec<Node>`, so not breaking) * Addition of `estimate_len()` to Node, used to pre-allocate the correct size of the `String` buffer * Implement `From<Builder>` on `Vec<Node>` ## Design Decisions * `estimate_len()` has some quick doctests and examples. I know most of the rest of the project uses test methods, but I hope this is alright given that the tests may add some more clarity to the purpose and function of the method. * `to_string()` has a single line of unsafe code. As the associated comment explains, this is provably safe, and exists just to avoid having to choose between having a bunch of duplicate code or inefficiently performing a UTF-8 check on a whole bunch bytes that we already know are safe. That said, I totally get it if you're just generally against unsafe code and will change it to be an alternative if you so wish * `ToString` is implemented instead of `Display`. This is to discourage users from directly using this in a `println!()` or `write!()` macro, which would not be a thing you would normally expect to do with this. It also gives us the advantage of being able to pre-allocate a buffer size, meaning less expensive String resizing. * I couldn't think of a clever way to get `render()` to work with both `io::Write`s or `fmt::Write`s without duplicating the code, but I'm dumb and might be missing something, so if there's a way to do that instead of doing my funky unsafe hack that's cool and I can do that instead. Also hi! Sorry for barging in on your nice gitea instance without asking first I just kinda wanted to submit this PR. If that wasn't cool I can go tho. I'm a singlet but not like the mean kind I can be nice I promise but also I totally get it if you don't want me here sorry thanks

Ghost commented

2020-11-30 04:38:25 +00:00

First-time contributor

also uhh is that failing test my fault or is it just like that sometimes

Ghost referenced this pull request

2020-11-30 06:20:47 +00:00

Add a `blank_line()` method to `Builder` #13

Ghost changed title from ~~Impl ToString for Builder, accept AsRef<[Node]> in `render()`~~ to Add conversion traits to Builedr

2020-11-30 06:43:04 +00:00

Ghost changed title from ~~Add conversion traits to Builedr~~ to Add conversion traits to Builder

2020-11-30 06:53:38 +00:00

~~Ghost referenced this pull request 2020-11-30 19:09:07 +00:00~~