tokio-rs / website

Website for the Tokio project
https://tokio.rs
MIT License
227 stars 328 forks source link

Inconsistency of bash scripts in the tutorial #760

Closed timKraeuter closed 3 months ago

timKraeuter commented 3 months ago

The bash scripts are displayed inconsistently across the tutorial, which is a bit confusing and sometimes less ergonomic when following the tutorial.

Problem

For example, in Hello tokio and Spawning the following is used (mind the $, and language=bash):

```bash
$ cargo install mini-redis

In Streams, $ is used but not language=bash, which is fine but could be changed.

However, later in Channels, IO, and Framing scripts are shown as follows (mind the missing $, and language=text not bash):

```text
mkdir src/bin
mv src/main.rs src/bin/server.rs

This is probably because the commands in Channels, IO, and Framing do not create any output, and without $, multiple commands are easier to copy and paste (more ergonomic). In contrast, in Hello tokio there is a command with output:

$ cargo run
got value from the server; result=Some(b"world")

Solutions

Thus, there are multiple solutions:

  1. Use $ consistently
    • Less ergonomic for multiple commands in one block; see the first block in Hello Tokio.
  2. Drop $ consistently
    • More ergonomic, but commands are less evident due to missing $ Means rewriting the command in Hello Tokio shown above to distinguish commands from output. This is already done at the end of the Setup section.
    • We could highlight bash scripts differently (currently, I do not see a difference between text and bash) to distinguish them from other code blocks representing the output (not exactly sure how to do this yet).
  3. Leave it as is.

Let me know what you think!

I can draft a pull request for 1 or 2 if you like.

Darksonn commented 3 months ago

Thanks for pointing this out. I'm leaning towards consistent use of $.

timKraeuter commented 3 months ago

I also think that makes the most sense and seems consistent with what I have seen in other documentation.

I made a minimal pull request (#761) which fixes the issue and subsumes an old pull request (#691).