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

refactor: rewrite forge doc server using axum #6324

Merged
merged 3 commits into from
Nov 16, 2023
Merged

Conversation

DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented Nov 15, 2023

Motivation

We're already using axum for Anvil, let's not use another one for no reason.

Solution

  • Rewrite the simple HTTP server with axum and tower-http.
  • The WebSocket autoreload stuff did not do anything, so I removed it.
  • Move it to forge binary crate since it's pretty heavy dependency-wise.
  • While I'm at it, added a --open to open the docs in a browser after serving them.

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm,
no need for warp, yes

for hotreloading, we could install watchexec with -w option

use tower_http::services::{ServeDir, ServeFile};

/// The HTTP endpoint for the websocket used to trigger reloads when a file changes.
const LIVE_RELOAD_ENDPOINT: &str = "/__livereload";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh hell yeah,
I think for reloading we'd need a watch service that runs forge doc?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was half-implemented. I can open a follow up implementing this

Copy link
Member

@mattsse mattsse Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sweet,

I can't really remember how the watchexec API works, but I believe it was registering an action via a closure, maybe the forge watch command is helpful a reference, perhaps not

Comment on lines +39 to +41
/// Open the documentation in a browser after serving.
#[clap(long, requires = "serve")]
open: bool,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amazing,
works

@DaniPopes DaniPopes merged commit befa571 into master Nov 16, 2023
20 checks passed
@DaniPopes DaniPopes deleted the dani/doc-axum branch November 16, 2023 00:52
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

Successfully merging this pull request may close these issues.

3 participants