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

Website v2 #143

Closed
mockersf opened this issue May 3, 2021 · 21 comments
Closed

Website v2 #143

mockersf opened this issue May 3, 2021 · 21 comments

Comments

@mockersf
Copy link
Member

mockersf commented May 3, 2021

Website would be made of several parts:

Awesome Bevy part would pull from another repo files describing all the community contributions and display them integrated in the website (bevyengine/bevy-assets#71)

Work needed for that:

  • theming mdbook to the website theme
  • setting up new Awesome Bevy
  • setting up CI

Downsides:

  • CI to deploy will be quite more complex
  • Different tools to build different part of the website, making it harder to contribute / maintain

Other options:

What do you think?

@alice-i-cecile
Copy link
Member

I like this proposal a lot: I think it's substantially superior to all three of the other options listed.

Integrating Awesome Bevy directly into the website would be lovely; I think it deserves both a nicer UI and better visibility at this point. I'm not sure if it should be in the same repo or another.

@cart
Copy link
Member

cart commented May 4, 2021

Awesome Bevy

I agree that awesome-bevy should be built into the main bevy website.

I really like having awesome-bevy in another repo for "separation of concerns" reasons. We can treat it like a database to be queried and we don't conflate "core website changes" with "new community plugins/games/etc", which will likely continue to increase in frequency (and would therefore create a bunch of noise in the bevy-website repo). The biggest downside is ensuring the website is updated when awesome-bevy is updated. Using a single repo would trivially ensure that awesome-bevy has valid content and that every change is automatically deployed.

But that seems like a solvable problem:

  1. make awesome-bevy CI trigger a bevy-website rebuild on merged changes
  2. add format validation to awesome-bevy

Bevy Book Tech Stack

I'm not yet convinced that the addition of mdbook is a net win. Zola-everywhere has a number of benefits:

  1. zola serve is the only command required to work on all of bevy-website
  2. theming mdbook to match the website means we need to maintain two sets of "identical styles" but with different implementations: one for bevy-website theming "zola markdown / normal content" and the other for mdbook, which will almost certainly have its own quirks / elements. This will likely be a constant struggle as bevy-website evolves and mdbook styles change.
  3. Using zola for the blog and the book has synergies. Improvements to one are inherently improvements to the other. Some real world examples:
    • I implemented "rich rust type links" and now both the blog and the book can use them, the api for declaring them is the same, and they "look the same" by default.
    • We just merged "anchor links for headers", which can look/behave the same way in the book and the blog.

The biggest missing piece that mdbook provides but bevy-website is missing is rust code-block validation. But that actually feels relatively straightforward to me:

  1. enumerate list of files to be validated (either just scan for all .md files or use a filter)
  2. run each markdown file through rustdoc. ex: rustdoc --test content/learn/book/getting-started/setup/_index.md works after a few tweaks:
    • Markdown files must start with # or %. The "zola header" with metadata is generally the first thing in book md. This can be worked around by just inserting # FILE_PATH_HERE at the top of each file (when validating the file). Duplicate h1s don't break rustdoc so the only downside here is off-by-one errors for line numbers. And adding the file path makes debugging easier (see example below)
    • adjust for the fact that code blocks are assumed to be rust if no language is specified
    • The rs language type in code blocks wont work. It needs to be rust (which is still compatible with zola's syntax highlighting).

The command above properly validates the "hello world" codeblock in the getting started guide (first run is valid content, second run introduces a bug):
image

@cart
Copy link
Member

cart commented May 4, 2021

I'm now realizing the file path is already part of the error message. So we would probably just use # bevy_website as the auto-inserted header.

@cart
Copy link
Member

cart commented May 4, 2021

There is also the matter of letting rustdoc know where to find dependencies (such as the bevy crate). This is possible with -L and --extern flags. So far I've gotten it to respect --extern bevy=path_to_bevy_rlib, but you also need to pull in each dependency, so thats a bit nasty. However it looks like mdbook also just farms out validation to rustdoc and it also looks like mdBook doesn't handle dependency resolution. That issue references skeptic which appears to be exactly what we need.

@cart
Copy link
Member

cart commented May 4, 2021

It looks like @alice-i-cecile's "Understanding Bevy" book and @jamadazi's "Bevy Cheatbook" actually both avoid using the rustdoc validation, in favor of just embedding content from rust crates validated using normal cargo build.

I think we should support both patterns. By default, we should favor directly-embedded code as this keeps the code close to the context and avoids the "line number file filter" errors introduced by something like `{{ import ../my_crate/foo.rs lines 0-10 }}``. But embedding content from external crates avoids code duplication for large examples introduced across many code snippets.

Therefore I propose:

  1. Use skeptic if possible to validate embedded code (or write our own rustdoc abstraction that properly handles dependency management)
  2. Find an existing Zola feature or write a "zola shortcode" that allows us to import content from other files. I don't expect this to be complicated. Worst case scenario we can add a pre-pass script.

@alice-i-cecile
Copy link
Member

It looks like @alice-i-cecile's "Understanding Bevy" book and @jamadazi's "Bevy Cheatbook" actually both avoid using the rustdoc validation, in favor of just embedding content from rust crates validated using normal cargo build.

This was courtesy of @mockersf's setup in my case <3

If we're extending Zola to do this, we should try to upstream it as well. Less maintenance burden in the long-run, and I'm sure it's a feature others will appreciate.

@cart
Copy link
Member

cart commented May 4, 2021

Lol well (2) was really easy:

image

@cart
Copy link
Member

cart commented May 4, 2021

image

@mockersf
Copy link
Member Author

mockersf commented May 4, 2021

The idea was not to snipe you into making it 😄

So you feel strongly about keeping everything with Zola.

It is possible, but will require more elbow grease in the CI to build it 👍

  • for full crate examples
    • validation is easy, just have to find all Cargo.toml files and run cargo test
    • include would need to be able to filter by line or code block as mdbook, should be doable
  • for snippets
    • skeptic parses the specified md files, extract code blocks, put them in a test file and run the tests. Its functions for doing it are private without issuing cargo commands, but we can build an empty crate that would just run skeptic

Are you ok with that plan?

It looks like @alice-i-cecile's "Understanding Bevy" book and @jamadazi's "Bevy Cheatbook" actually both avoid using the rustdoc validation, in favor of just embedding content from rust crates validated using normal cargo build.

I like this pattern as it provides a complete context on how to run a snippet, and is nice to split a tutorial into step by step

@cart
Copy link
Member

cart commented May 4, 2021

The idea was not to snipe you into making it

No worries! I definitely didn't interpret it that way. I just had enough Zola context to know it should be doable and enough motivation to compare that type of implementation to what mdBook does.

for full crate examples

  • validation is easy, just have to find all Cargo.toml files and run cargo test
  • include would need to be able to filter by line or code block as mdbook, should be doable

Yeah I agree with all of that. Just gotta be a bit careful about "accidental offset errors" when someone makes a change to the code (or even just runs cargo fmt) and moves the desired line numbers to a position outside of the range referenced. Code blocks using this approach will require extra diligence on our part (which is why inline should be the default).

for snippets

  • skeptic parses the specified md files, extract code blocks, put them in a test file and run the tests. Its functions for doing it are private without issuing cargo commands, but we can build an empty crate that would just run skeptic

Yeah before committing to skeptic someone needs to do a quick proof of concept (in the context of the Bevy Book) to make sure the workflow is ideal. I'm a little afraid the templates will be overly limiting, whereas rustdoc is pretty flexible. Solving the rustdoc dependency problem in a clean way will be challenging, but probably not impossible (and maybe solved by someone already).

Also both skeptic and rustdoc will rely on us solving the "ignored line" problem. Most inline code blocks will require some "context" that isn't rendered, but helps the example compile. rustdoc normally supports filtering out "hashtag lines" like # use bevy::prelude::*. But we'll need to do that ourselves because rustdoc isn't generating html for us. Skeptic will also compile "hashtag lines", but we still need to filter them out during rendering (which i have confirmed zola does not do).

Doing hashtag filtering would require a pre-pass of some kind:

  • Maybe possible using some permutation of zola-native features like shortcodes, disabling markdown rendering, and manual markdown rendering from within templates. but this will almost certainly be ugly if it works at all
  • We could make a custom zola build that adds support for filtering hashtag lines out of rust code blocks (and try to upstream it if possible). I don't want to deal with maintaining a custom zola build, so we should get this pre-approved if we decide to go this route.
  • A script/program that runs before zola in CI, parses markdown, filters out hashtags in rust code blocks, and dumps the output somewhere to be rendered. This is also pretty hackey / complicates the build situation more than I'd like.

We could also just rely on skeptic templates for "hidden content". But unfortunately using the skeptic template syntax breaks syntax highlighting in zola. Ex: starting a code block with ```rust,skt-foo breaks syntax highlighting, presumably because it is looking for a language named rust,skt-foo.

If we can't find some way to solve the "hidden code for inline blocks" problem cleanly (or we're blocked by Zola upstreaming a feature we need), we might need to just rely on "external code blocks" completely when code validation is required.

Pulling in @Keats (creator of Zola) for input here as they have contributed to the Bevy Book in the past / seem interested in helping us out 😄

@mockersf
Copy link
Member Author

mockersf commented May 4, 2021

Yeah I agree with all of that. Just gotta be a bit careful about "accidental offset errors" when someone makes a change to the code (or even just runs cargo fmt) and moves the desired line numbers to a position outside of the range referenced. Code blocks using this approach will require extra diligence on our part (which is why inline should be the default).

Better than filtering by line number, there is filtering by code block delimited by anchors (see the part that starts with To avoid breaking your book when modifying included files in https://rust-lang.github.io/mdBook/format/mdbook.html#including-portions-of-a-file). I really like anchors for that.

We could also just rely on skeptic templates for "hidden content". But unfortunately using the skeptic template syntax breaks syntax highlighting in zola. Ex: starting a code block with ```rust,skt-foo breaks syntax highlighting, presumably because it is looking for a language named rust,skt-foo.

I used skeptic in the past and read it's source recently to check if we could use it, I think it's the best way forward with Zola. We will need to change Zola language resolution anyway because we may want to have code blocks annotated with no-run, should-panic, and so on. I think it's a small enough change that it may be accepted: instead of takin everything of the same line as ``` as the language, split on the first ,

@cart
Copy link
Member

cart commented May 4, 2021

Better than filtering by line number, there is filtering by code block delimited by anchors (see the part that starts with To avoid breaking your book when modifying included files in https://rust-lang.github.io/mdBook/format/mdbook.html#including-portions-of-a-file). I really like anchors for that.

Ooh yeah I'm sold. Implementing this should be possible will zola/tera templates using a combination of the split/starting_with functions (ex: split the file by newlines, iterate over lines and find those starting with // ANCHOR: split on that, check if name matches requested anchor, if no continue, if yes try to find ANCHOR_END, combine range of lines back into single string (stripping out all anchor comment lines), and finally pipe string through markdown renderer. I won't pretend that writing these shortcodes will be fun, but it should be possible.

I think it's a small enough change that it may be accepted: instead of takin everything of the same line as ``` as the language, split on the first ,

Yup this doesn't seem particularly contentious and seems like a very scoped investment / quick win.

We'll also want to see if skeptic handles zola's "markdown header metadata" syntax properly, otherwise we'll need to add a preprocessor to strip it out.

@inodentry
Copy link
Contributor

Oooh so you guys want to implement mdbook anchor syntax in Zola? 👀 👀 👀

So, this should allow the official book to work kinda like Cheatbook, but in Zola instead of mdbook? 👀

@mockersf
Copy link
Member Author

mockersf commented May 5, 2021

Yup this doesn't seem particularly contentious and seems like a very scoped investment / quick win.

Zola actually already more or less support that, but right now it ignores tokens it doesn't know and consider the last token to be the language. if skeptic doesn't care about the ordering it should work already 👍
https://github.com/getzola/zola/blob/master/components/rendering/src/markdown/fence.rs

it also has a hl_lines (for highlight) that accepts range that I didn't see in the documentation, will have to try it

@Keats
Copy link
Contributor

Keats commented May 5, 2021

Anything you need from Zola?

@cart
Copy link
Member

cart commented May 5, 2021

Short term I don't think so. Soon we should know whether the zola and skeptic code block language formats are compatible with each other. If they aren't, then we'll need to sort out the best way to resolve that (ex: we might request that zola defaults to the first language in a comma separated list instead of the last language, or we might do the reverse for skeptic). Not sure if theres a spec for that / what sort of precedent the wider markdown ecosystem has set.

@mockersf
Copy link
Member Author

mockersf commented May 8, 2021

tried integrating with skeptic, should work well but build issue with native libs: #148

@mockersf
Copy link
Member Author

mockersf commented May 9, 2021

and other version including code blocks from a validated crate: #150

@cart
Copy link
Member

cart commented May 9, 2021

Ooh you implemented anchors. Very nice :)

@mockersf
Copy link
Member Author

mockersf commented May 9, 2021

yup I wanted to have the two options for comparison

@alice-i-cecile
Copy link
Member

I think all of the main questions in this are now resolved, and we're sticking with Zola. Any remaining nits should go in their own issues, so we can address them properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants