rust-analyzer / rust-analyzer.github.io

https://rust-analyzer.github.io/
87 stars 51 forks source link

Add dark mode #216

Closed joshka closed 4 months ago

joshka commented 5 months ago
joshka commented 5 months ago
Light

image

Dark

image

joshka commented 4 months ago

ping @matklad @lnicola

lnicola commented 4 months ago

Sorry about this. I'm not a fan of the extra JS dependency, and I think we should store the files instead of using the CDN (yes, even font-awesome). But it looks great, and we should probably merge it.

joshka commented 4 months ago

Sorry about this.

No problem - I figured this repo is one that rarely gets PRs comparatively to the main repo.

I'm not a fan of the extra JS dependency, and I think we should store the files instead of using the CDN (yes, even font-awesome).

Fixed in https://github.com/rust-analyzer/rust-analyzer.github.io/pull/216/commits/cd876ccc7a1c95e9205bc4eb206a1d39aeee5feb which:

lnicola commented 4 months ago

r? @matklad

matklad commented 4 months ago

Can we just vendor the two files? I’d avoid npm dependency if possible.

joshka commented 4 months ago

You can do that if you'd like. I'd generally advise taking the approach of pulling external sources like this from a package manager instead of vendoring, as it maintains provenance (do you trust that the version of these javascript files is the same as the released versions), and makes it easier to work with any future updates (e.g. enable dependabot and get automatic PRs for updates).

What's the rationale for avoiding installing node.js?

lnicola commented 4 months ago

Avoiding complexity, mostly. If you use npm all day long, you're comfortable with it, if you don't, then adding two JS files to the repo is easier. We don't particularly care about provenance and specific versions here, we'd probably use integrity if we did.

Similarly, matklad is comfortable with Jekyll, while I'm too scared to try updating our outdated version of Ruby.

joshka commented 4 months ago

Moved things into the repo.

matklad commented 4 months ago

Avoiding complexity, mostly.

There's also a thing that you definitely don't want both node&ruby in the same project, that's pure technical debt. Though, it perhaps can be argued that, give that the vs code extension is still very much a part of the project, we should remove ruby in favor of npm. I'd be wary about that though, mostly because the npm ecosystem defaults to overly-complicated solutions.

But, if we want to do some larger refactor here, I think it makes sense to think about the bigger overall question of whether rust-analyzer needs a separate website as a project?

Separate identity definitely made sense while the project was 3rd party. Now that we are official, perhaps all we need is mdbook? My personal answer is that we (as a rust project) should actually do the opposite, and add websites for other tools, like miri, rustc, and cargo. Rust is a federation of teams, and I think it works better if each team has their very own corner of the web.

Which all is completely unrelated to the PR, sorry for pouring this stream-of-conscious onto you! :)

joshka commented 4 months ago

Which all is completely unrelated to the PR, sorry for pouring this stream-of-conscious onto you! :)

No problem - it's an interesting peek behind the curtain.

If it's any consolation, we moved away from mdbook (due to it being fairly constrained) to using starlight for the Ratatui docs website and are pretty happy with the result.