sunjay
commented
3 years ago @JosephLing Sorry about the late reply. I really wanted to make sure I get you unstuck, so I checked out the code locally and did some digging starting on the master branch. The patch below works and does everything up to the actual SVG image generation itself. You should just have to finish off the SVG to PNG conversion, test it out, add a call to save_docs_image to the rest of the doctests that need it, and commit the updated images. Please keep in mind my review comments about not adding anything to doctests that require the window titlebar or require the turtle to be drawn. We can deal with those later.
One of the issues is in order for cfg flags to work they are separate for
RUSTFLAGSandRUSTDOCFLAGSso that means you need both set if you want the doctests to run thecfg(docs_images).
Having to set both RUSTFLAGS and RUSTDOCFLAGS is not actually a big downside here. Keep in mind that this command is only meant to be used occasionally (and only by me). It's okay if the command is a bit unruly because we can document it in CONTRIBUTING.md. Cargo features are exposed to the user and they don't actually solve the whole problem anyway--I'll explain more below.
A Better Starting Point
A lot of the issues you ran into were because of my own incomplete understanding of some of Rust's features and cargo. Sorry about that! I've taken the time to fix that and I'm hoping that the patch below will be a much better starting point for your work on this.
The first detail I missed was that dev-dependencies are only available for the actual test files themselves, the crate itself is not recompiled with the dev-dependencies. That means the only way to get this to work is to list turtle_docs_helpers in the [dependencies] section. This is troublesome because we don't want this crate to be visible to users of the turtle crate. We also can't publish a crate to crates.io if it has a path dependency.
Solution: have a commented out line in [dependencies] with an explanation of what's going on. This allows us to manually run the command to generate the images without forcing us to have a permanent path dependency in the [dependencies] section. The reason using cargo features wouldn't solve this problem is because even with a cargo feature, you'd still need to list the path dependency in [dependencies] section. There is no getting around that. The only difference is passing --features ... instead of setting environment variables. However, as explained above, that difference isn't important here.
You can run the code with:
# Don't forget to uncomment the `turtle_docs_helpers` dependency in Cargo.toml
RUSTDOCFLAGS="--cfg docs_images" RUSTFLAGS="--cfg docs_images" cargo test --features "test unstable" --docOn windows, you may need to set the environment variables at the front of the command in advance, then run the rest of the command starting with cargo test ....
diff --git a/Cargo.toml b/Cargo.toml
index 1dcfbca..b103548 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -51,6 +51,11 @@ cfg-if = "0.1"
parking_lot = "0.10"
+# This dependency should be uncommented when rebuilding the images in the API
+# documentation. In all other cases this dependency cannot be used since having
+# a path-only dependency would make it impossible to publish this crate.
+#turtle_docs_helpers = { path = "turtle_docs_helpers" }
+
[dependencies.futures-util]
version = "0.3"
default-features = false
diff --git a/src/async_drawing.rs b/src/async_drawing.rs
index 42931ea..49176fe 100644
--- a/src/async_drawing.rs
+++ b/src/async_drawing.rs
@@ -188,3 +188,10 @@ impl AsyncDrawing {
self.client.debug_drawing().await
}
}
+
+#[cfg(docs_images)]
+impl turtle_docs_helpers::SaveDocsSvg for AsyncDrawing {
+ fn save_docs_svg(&self, path: std::path::PathBuf) {
+ self.client.save_docs_svg(path);
+ }
+}
diff --git a/src/async_turtle.rs b/src/async_turtle.rs
index 2602292..77f1e74 100644
--- a/src/async_turtle.rs
+++ b/src/async_turtle.rs
@@ -324,3 +324,10 @@ impl AsyncTurtle {
self.client.debug_turtle(self.id, self.angle_unit).await
}
}
+
+#[cfg(docs_images)]
+impl turtle_docs_helpers::SaveDocsSvg for AsyncTurtle {
+ fn save_docs_svg(&self, path: std::path::PathBuf) {
+ self.client.save_docs_svg(path);
+ }
+}
diff --git a/src/drawing.rs b/src/drawing.rs
index 014e9eb..6548edf 100644
--- a/src/drawing.rs
+++ b/src/drawing.rs
@@ -629,6 +629,13 @@ impl Drawing {
}
}
+#[cfg(docs_images)]
+impl turtle_docs_helpers::SaveDocsSvg for Drawing {
+ fn save_docs_svg(&self, path: std::path::PathBuf) {
+ self.drawing.save_docs_svg(path);
+ }
+}
+
#[cfg(test)]
mod tests {
use super::*;
diff --git a/src/ipc_protocol/protocol.rs b/src/ipc_protocol/protocol.rs
index 6e13beb..df9a9b3 100644
--- a/src/ipc_protocol/protocol.rs
+++ b/src/ipc_protocol/protocol.rs
@@ -408,3 +408,12 @@ impl ProtocolClient {
}
}
}
+
+#[cfg(docs_images)]
+impl turtle_docs_helpers::SaveDocsSvg for ProtocolClient {
+ fn save_docs_svg(&self, path: std::path::PathBuf) {
+ use crate::sync_runtime::block_on;
+ block_on(self.export_svg(path))
+ .expect("unable to save docs image");
+ }
+}
diff --git a/src/turtle.rs b/src/turtle.rs
index 5019456..a1e1388 100644
--- a/src/turtle.rs
+++ b/src/turtle.rs
@@ -535,7 +535,7 @@ impl Turtle {
///
/// # Example
///
- /// ```rust,no_run
+ /// ```rust
/// use turtle::Turtle;
///
/// fn main() {
@@ -557,6 +557,7 @@ impl Turtle {
/// turtle.set_pen_color("#4CAF50"); // green
/// turtle.set_pen_size(100.0);
/// turtle.forward(200.0);
+ /// # #[cfg(docs_images)] turtle_docs_helpers::save_docs_image(&turtle, "pen_thickness");
/// }
/// ```
///
@@ -891,6 +892,13 @@ impl Turtle {
}
}
+#[cfg(docs_images)]
+impl turtle_docs_helpers::SaveDocsSvg for Turtle {
+ fn save_docs_svg(&self, path: std::path::PathBuf) {
+ self.turtle.save_docs_svg(path);
+ }
+}
+
#[cfg(test)]
mod tests {
use super::*;
diff --git a/turtle_docs_helpers/Cargo.toml b/turtle_docs_helpers/Cargo.toml
new file mode 100644
index 0000000..4a6a129
--- /dev/null
+++ b/turtle_docs_helpers/Cargo.toml
@@ -0,0 +1,9 @@
+[package]
+name = "turtle_docs_helpers"
+version = "0.1.0"
+authors = ["Sunjay Varma <varma.sunjay@gmail.com>"]
+edition = "2018"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
diff --git a/turtle_docs_helpers/src/lib.rs b/turtle_docs_helpers/src/lib.rs
new file mode 100644
index 0000000..40a9937
--- /dev/null
+++ b/turtle_docs_helpers/src/lib.rs
@@ -0,0 +1,17 @@
+use std::path::{Path, PathBuf};
+
+/// Saves the image being drawn as an SVG and panics if an error occurs
+///
+/// This is different from the `save_svg` method on `Drawing` and `AsyncDrawing`
+/// because this is only meant to be used for automation and may need to access
+/// internal APIs.
+pub trait SaveDocsSvg {
+ fn save_docs_svg(&self, path: PathBuf);
+}
+
+/// Saves the currently drawn image to `docs/assets/images/docs/{output_name}`
+pub fn save_docs_image<T: SaveDocsSvg>(drawing: &T, output_name: &str) {
+ let svg_path = Path::new("docs/assets/images/docs").join(output_name).with_extension("svg");
+ drawing.save_docs_svg(svg_path);
+ todo!()
+}Notes:
- renamed the trait and trait method to help avoid confusion (I had asked for this in the review comments but I thought it might be easier for you if I just did it here)
- changed
PathtoPathBufto avoid an unnecessary allocation and sinceself.client.export_svgtakesPathBufanyway - each added impl is at the bottom of its file (but above any tests) because this code isn't that important and won't be updated often
Please make sure you document in CONTRIBUTING.md that you need to uncomment the turtle_docs_helpers dependency before running the command shown above.
I think if you base your work on this, you should be able to get it working. Please don't hesitate to let me know if you have any more questions!
JosephLing
For https://github.com/sunjay/turtle/issues/179 this implements generating the svgs from the doc tests using
docs_imagefeature flag.There was one main slight complication in that a few examples required the user to click on the window. For those tests which where originally
no_runI split them up into separate doc tests but hopefully they should all be one coherent code example...is_visible, for the title bar I am not sure the best way to handle it but it is easy to get the data out.