Update documentation to include write examples #100
Conversation
|
Can someone review? 🙂 cc @lnicola |
README.md
Outdated
| }); | ||
|
|
||
| // Create file at path | ||
| let gpx_file = File::create(out_path); |
There was a problem hiding this comment.
| let gpx_file = File::create(out_path); | |
| let gpx_file = File::create(out_path)?; |
README.md
Outdated
| gpx.tracks[0].segments[0].points.push(Waypoint::new(geo_point)); | ||
|
|
||
| // Write to file | ||
| write(&gpx, gpx_file?)?; |
There was a problem hiding this comment.
| write(&gpx, gpx_file?)?; | |
| gpx::write(&gpx, gpx_file)?; |
README.md
Outdated
| `write` will write the GPX output to anything that implements `std::io::Write`. To save the output to a string, use a mutable `BufWriter` to write it to a vector, and then convert the vector to a string. | ||
| ```rust | ||
| let mut buf = BufWriter::new(Vec::new()); | ||
| write(&gpx, &mut buf)?; // writes to buf |
There was a problem hiding this comment.
| write(&gpx, &mut buf)?; // writes to buf | |
| gpx::write(&gpx, &mut buf)?; // writes to buf |
README.md
Outdated
| ``` | ||
|
|
||
| ### Write to string | ||
| `write` will write the GPX output to anything that implements `std::io::Write`. To save the output to a string, use a mutable `BufWriter` to write it to a vector, and then convert the vector to a string. |
There was a problem hiding this comment.
I don't think you need to wrap a Vec in a BufWriter. But it might be a good idea to do it for files, since write might cause very short writes. So can you update both examples?
README.md
Outdated
|
|
||
| pub fn to_gpx<P: AsRef<Path>>(out_path: P) -> Result<String, Box<dyn Error>> { | ||
| // Instantiate Gpx struct | ||
| let mut gpx: Gpx = Gpx { |
There was a problem hiding this comment.
Let's build this in the other order (let track_segment = ...; let track = ...; let gpx = ...). That way wou don't need indexing and can do directly e.g. vec![track].
README.md
Outdated
| use gpx::{Gpx, Track, TrackSegment, Waypoint, Route}; | ||
| use geo_types::{Point, coord}; | ||
|
|
||
| pub fn to_gpx<P: AsRef<Path>>(out_path: P) -> Result<String, Box<dyn Error>> { |
There was a problem hiding this comment.
| pub fn to_gpx<P: AsRef<Path>>(out_path: P) -> Result<String, Box<dyn Error>> { | |
| pub fn to_gpx<P: AsRef<Path>>(out_path: P) -> Result<(), Box<dyn Error>> { |
|
Thanks for the review, great suggestions. I'm new to this library and Rust generally, so I'll defer to your suggestions. I'll push the changes in the next couple of days, hopefully by the end of this coming weekend. |
README.md
Outdated
| pub fn to_gpx<P: AsRef<Path>>(out_path: P) -> Result<String, Box<dyn Error>> { | ||
| // Instantiate Gpx struct | ||
| let mut gpx: Gpx = Gpx { | ||
| version: GpxVersion::Gpx11, // or Gpx10 |
There was a problem hiding this comment.
I'm personally not a fan of these type annotations / comments here. If you're just skimming the README, it's enough to know you can set e.g. the waypoints, but the exact type won't matter too much. And if you've pasted the code into your editor, you'll hopefully get enough info from it.
|
Ok, code has been updated! May you take a look when you get a chance, @lnicola? |
| @@ -2,6 +2,7 @@ | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| - [#100](https://github.com/georust/gpx/pull/100): Add write examples to `README.md` | |||
There was a problem hiding this comment.
Oh, my, do we really have so many unreleased changes?
README.md
Outdated
| ```rust | ||
| use std::path::Path; | ||
| use gpx::{Gpx, Track, TrackSegment, Waypoint, Route}; | ||
| use geo_types::{Point, coord}; |
There was a problem hiding this comment.
You're missing a couple of imports:
use geo_types::{coord, Point};
use gpx::{Gpx, GpxVersion, Track, TrackSegment, Waypoint};
use std::{error::Error, fs::File, io::BufWriter, path::Path};
README.md
Outdated
| // Write to file | ||
| gpx::write(&gpx, buf)?; | ||
|
|
||
| Ok(()); |
There was a problem hiding this comment.
| Ok(()); | |
| Ok(()) |
README.md
Outdated
| ``` | ||
|
|
||
| ### Write to string | ||
| `write` will write the GPX output to anything that implements `std::io::Write`. To save the output to a string, write it to a UTF-8 vector, and then convert the vector to a string. |
There was a problem hiding this comment.
| `write` will write the GPX output to anything that implements `std::io::Write`. To save the output to a string, write it to a UTF-8 vector, and then convert the vector to a string. | |
| `write` will write the GPX output to anything that implements `std::io::Write`. To save the output to a string, write it to an `u8` vector, and then convert the vector to a `String`. |
README.md
Outdated
| ### Write to string | ||
| `write` will write the GPX output to anything that implements `std::io::Write`. To save the output to a string, write it to a UTF-8 vector, and then convert the vector to a string. | ||
| ```rust | ||
| let mut vec: Vec<u8> = Vec::new(); |
There was a problem hiding this comment.
| let mut vec: Vec<u8> = Vec::new(); | |
| let mut vec = Vec::new(); |
|
Done :-) ... can you take a look again @lnicola ? |
|
Thanks and sorry for the back and forth! bors r+ |
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
No worries, thanks for the comments, it's helping me learn good practices in Rust 🙂 |
CHANGELOG.mdif knowledge of this change could be valuable to users.Fixes #98