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

initial directory listing support #328

Merged
merged 11 commits into from
Oct 13, 2022
Merged

initial directory listing support #328

merged 11 commits into from
Oct 13, 2022

Conversation

b5
Copy link
Member

@b5 b5 commented Oct 12, 2022

  • now with actual HTML
  • breadcrumb headers
  • icons for 64 file extensions (matching kubo)
  • human readable file sizes (via a new iroh-util func, cc @faassen for CLI usage)
  • sizes are in SI units (MiB, KiB, GiB), too nerdy?
  • all data prep handled on the rust side, with no helper functions in template code
  • use include_str! for easier asset maintenance

re-interprets the kubo template with iroh colors (I don't think we should be screwing with end-user expectations too much for this first release), but help links point to iroh to avoid bombing kubo maintainers with iroh issues:

Screen_Shot_2022-10-12_at_4 24 08_PM

punted until later:

  • root directory size
  • load time indicator
  • proper tests to ensure directory template doesn't break everything

closes n0-computer/beetle#258

@b5 b5 marked this pull request as ready for review October 12, 2022 15:10
@b5 b5 requested review from Arqu and ramfox October 12, 2022 15:10
@b5 b5 self-assigned this Oct 12, 2022
@b5 b5 added c-gateway feat New feature or request labels Oct 12, 2022
let path = match accum.last() {
Some(prev) => {
let base = prev.get("path");
format!("/{}/{}", base.unwrap_or(&"".to_string()), encode(path_el))
Copy link
Contributor

Choose a reason for hiding this comment

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

this will not do what you want, if base is empty this will produce //<path_el>.

match prev.get("path") {
  Some(base) => format!("/{}/{}", base, encode(path_el)),
  None => format!("/{}", encode(path_el)),
}

should be closer to what you want

Copy link
Contributor

Choose a reason for hiding this comment

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

actually looking at the whole structure this can be simplified to

match accum.last().and_then(|prev| prev.get("path")) {
    Some(base) => format!("/{}/{}", base, encode(path_el)),
    None => format!("/{}", encode(path_el)),
}

@@ -550,16 +572,59 @@ async fn serve_fs_dir<T: ContentLoader + std::marker::Unpin>(
if !root_path.ends_with('/') {
root_path.push('/');
}

let mut breadcrumbs: Vec<HashMap<&str, String>> = Vec::new();
root_path
Copy link
Contributor

Choose a reason for hiding this comment

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

can I get a todo at least for writing a test for this?

dignifiedquire
dignifiedquire previously approved these changes Oct 13, 2022
@b5 b5 merged commit 3316a53 into main Oct 13, 2022
@b5 b5 deleted the b5/gateway_templates branch October 13, 2022 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

gateway(unixfs): improve dir listing
3 participants