Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regenerate images in documentation #179

Open
14 tasks
sunjay opened this issue May 26, 2020 · 2 comments
Open
14 tasks

Regenerate images in documentation #179

sunjay opened this issue May 26, 2020 · 2 comments

Comments

@sunjay
Copy link
Owner

sunjay commented May 26, 2020

This crate has changed a lot in the last 3 years. The images in the documentation probably aren't super accurate anymore. If you haven't seen the documentation yet, it contains images to help people understand what the example code produces. Take the set_pen_color method for example:


image


After the changes in #173, this image now looks like the following:

image

It's a subtle change, but you can see that the pen thickness is about half of what it used to be, and the lines now have circular ends instead of rectangular ends. (The thickness change is actually because the set_pen_size calls in the documentation examples weren't updated after #173...oops! Feel free to double the pen thickness as part of your changes. The images probably look better that way!)

If any of the images are significantly different or have changed in undesirable ways, we should probably fix the example instead of just updating the image. Feel free to leave a comment and we can discuss what to do.

Mentoring Instructions

The image for the documentation shown above is in docs/assets/images/docs/colored_circle.png.

The image URL is hard-coded to a permanent link in the documentation comment:

/// ![turtle pen color](https://github.com/sunjay/turtle/raw/9240f8890d1032a0033ec5c5338a10ffa942dc21/docs/assets/images/docs/colored_circle.png)

(Notice that the path I just told you is in this code.)

You won't be able to update that link until after the PR is merged, but I would still really appreciate some help updating the images.

Here's what you need to do to help:

  1. Clone the repository
  2. Open examples/circle.rs in your editor
    • Important: We are going to be running the code using this file, but don't actually commit any of the changes we make! This is very easy to do accidentally with git.
  3. Copy and paste the code from the documentation into examples/circle.rs
    • For example, if you were going to do this for the set_pen_color method shown above, you would start by opening your local copy of the turtle.rs file
    • The code on docs.rs may be out of date, so make sure to use your local copy so you don't accidentally generate the wrong image!
    • Once you have turtle.rs open, find the set_pen_color method and copy and paste the lines of example code in the comment above the method
    • Delete the leading /// and you should get some valid Rust code
    • Another potentially easier method is to generate the documentation and copy and paste the code from there
  4. Run the example with cargo run --features unstable --example circle
  5. Wait for it to complete
  6. Take a screenshot of the drawing without the titlebar
    • The screenshots we have had so far all have the titlebar, but this isn't great because titlebars look different on every platform/OS/theme
    • Only include the titlebar if it is relevant (e.g. the image for set_title would need to include the titlebar)
  7. Look for the right filename for your image in the markdown of the documentation you're trying to update the image for
  8. Replace that file in the repo
  9. Commit the new image

As I mentioned, you won't be able to update the URLs to their permanent links until after the PR to update the images has been merged. Please feel free to come back and help out with that in a follow-up PR! Instructions for doing this can be found in the issue where we originally made all the links into permanent links.

Checklist

  • docs/assets/images/docs/changed_title.png
  • docs/assets/images/docs/orange_background.png
  • docs/assets/images/docs/circle.png
  • docs/assets/images/docs/circle_offset_center.png
  • docs/assets/images/docs/circle.png
  • docs/assets/images/docs/small_drawing.png
  • docs/assets/images/docs/squares.svg
    • Updated URL needs ?sanitize=true at the end
  • docs/assets/images/docs/color_mixing.png
  • docs/assets/images/docs/pen_thickness.png
  • docs/assets/images/docs/colored_circle.png
  • docs/assets/images/docs/red_circle.png
  • docs/assets/images/docs/clear_before_click.png
  • docs/assets/images/docs/clear_after_click.png
@JosephLing
Copy link

Would it be possible to have the images be generated from the docs? as you can have hidden code at the end of each example to save it to a docs/assets/.... path. The complication would be that svgs don't render in md so maybe some svg to png crate could be used (nsvg could work). As well as only having this run on a release not every time someone ran the tests.

I would happy to help with original task and to help work on a more automated version (although I know that is a little over the top for the orignal issue).

@sunjay
Copy link
Owner Author

sunjay commented Aug 10, 2020

Would it be possible to have the images be generated from the docs? as you can have hidden code at the end of each example

This is a very interesting idea! I would have to see the results before committing to having it in the crate, but I really like the idea of this being automated. One of the tricky things about this would be that some of the examples do need the titlebar included in the screenshot. I suppose for now we could simply leave those examples to be manually updated and automate the rest.

Would you be willing to do some exploratory work on this? It's possible that we might not end up merging it if the solution is too difficult to maintain, but I would be interested in trying it out to see what happens.

Mentoring Instructions

  • create a crate in the repository root directory called turtle_docs_helpers or something
  • add a path dev-dependency to the turtle crate so it can use the turtle_docs_helpers crate but only builds it for tests and stuff
  • the turtle_docs_helpers crate will define a function with the following signature:
     /// Saves the currently drawn image to `docs/assets/images/docs/{output_name}`
     pub fn save_docs_image<T: SaveSvg>(drawing: &T, output_name: &str) {
     	let svg_path = Path::new("docs/assets/images/docs").join(output_name).with_extension("svg");
     	// ...
     }
  • The SaveSvg trait exists because turtle_docs_helpers can't depend on the turtle crate (circular dependency)
     /// 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 SaveSvg {
         fn save_svg(&self, path: &Path);
     }
  • In the turtle crate, we'll want to put any code involving turtle_docs_helpers behind a flag like #[cfg(docs_images)] or something (this is what makes it possible to only run this for a release and not every time we test)
  • This trait normally wouldn't be implementable for Turtle or AsyncTurtle since those have no save_svg method, but we'll use the internal APIs of both to force it to be possible
     //TODO: Figure out the best place to put these impls
     
     #[cfg(docs_images)]
     impl turtle_docs_helpers::SaveSvg for ProtocolClient {
     	fn save_svg(&self, path: &Path) {
     		block_on(self.export_svg(path))
     			.expect("unable to save docs image");
     	}
     }
     
     #[cfg(docs_images)]
     impl turtle_docs_helpers::SaveSvg for AsyncDrawing {
     	fn save_svg(&self, path: &Path) {
     		self.client.save_svg(path);
     	}
     }
     
     #[cfg(docs_images)]
     impl turtle_docs_helpers::SaveSvg for Drawing {
     	fn save_svg(&self, path: &Path) {
     		self.drawing.save_svg(path);
     	}
     }
     
     //TODO: Similar impls for AsyncTurtle and Turtle
  • add # #[cfg(docs_images)] turtle_docs_helpers::save_docs_image(&turtle, "..."); to the bottom of every example that needs it (the # ensures that the line is not outputted when generating documentation)
  • Check the images and make sure the right ones are in the right places
    • Take note of any that are majorly different because of the way the library has changed, we can always update those images later once the examples are fixed

Let me know if you have any questions about any of this! This is just off the top of my head, so I may have missed some things or made some mistakes. These instructions should get you most of the way there. 😄

Sounds like a fun thing to work on! 🎉

@JosephLing JosephLing mentioned this issue Aug 11, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants