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

feat/issue-306-pr #965

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ socket2 = "0.4"
strum = { version = "0.24", features = ["derive"] }
tar = "0.4"
thiserror = "1"
walkdir = "2.3.2"
yansi = "0.5"
zip = { version = "0.6.2", default-features = false }

Expand Down
44 changes: 36 additions & 8 deletions src/listing.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![allow(clippy::format_push_string)]
use std::fmt::Display;
use std::io;
use std::path::{Component, Path, PathBuf};
use std::time::SystemTime;
Expand All @@ -10,6 +11,7 @@ use percent_encoding::{percent_decode_str, utf8_percent_encode};
use regex::Regex;
use serde::Deserialize;
use strum::{Display, EnumString};
use walkdir::WalkDir;

use crate::archive::ArchiveMethod;
use crate::auth::CurrentUser;
Expand Down Expand Up @@ -78,6 +80,24 @@ pub enum EntryType {
File,
}

#[derive(Clone, Copy, PartialEq, Eq)]
/// Possible entry size types
pub enum EntrySize {
/// EntryCount is number of entries in a directory
EntryCount(usize),
/// Bytes is number of bytes in a file
Bytes(bytesize::ByteSize),
}

impl Display for EntrySize {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
EntrySize::EntryCount(count) => write!(f, "{}", count),
EntrySize::Bytes(bytes) => write!(f, "{}", bytes),
}
}
}

Copy link
Owner

Choose a reason for hiding this comment

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

Wouldn't it be cool to also show the combined directory file size?

Copy link
Author

Choose a reason for hiding this comment

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

Not sure what you are referring to by "combined directory file size". Are you referring to the total size in bytes of a directory? That might be a bit expensive and probably would require a persistent cache to be anywhere near fast.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, the byte sizes. I recognize it might be too slow. Hm. If you like to, you could try to experiment with this.

/// Entry
pub struct Entry {
/// Name of the entry
Expand All @@ -89,8 +109,8 @@ pub struct Entry {
/// URL of the entry
pub link: String,

/// Size in byte of the entry. Only available for EntryType::File
pub size: Option<bytesize::ByteSize>,
/// Size of the entry
pub size: EntrySize,

/// Last modification date
pub last_modification_date: Option<SystemTime>,
Expand All @@ -104,7 +124,7 @@ impl Entry {
name: String,
entry_type: EntryType,
link: String,
size: Option<bytesize::ByteSize>,
size: EntrySize,
last_modification_date: Option<SystemTime>,
symlink_info: Option<String>,
) -> Self {
Expand Down Expand Up @@ -256,12 +276,14 @@ pub fn directory_listing(
Err(_) => None,
};

let size = WalkDir::new(entry.path()).into_iter().count();
Copy link
Author

Choose a reason for hiding this comment

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

Thinking about it, we might be able to just do .take(UPPER_LIMIT).count() instead of .count() to shortcut execution time. So, we might have UPPER_LIMIT be something like 10001 and if it's more than 10000 we have the Impl Display for EntrySize print >10000 or something along those lines.

Copy link
Author

Choose a reason for hiding this comment

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

The other option would have been to add an actual timeout, but I'm not really sure how to best do that in a non-async context.


if metadata.is_dir() {
entries.push(Entry::new(
file_name,
EntryType::Directory,
file_url,
None,
EntrySize::EntryCount(size),
last_modification_date,
symlink_dest,
));
Expand All @@ -270,7 +292,7 @@ pub fn directory_listing(
file_name.clone(),
EntryType::File,
file_url,
Some(ByteSize::b(metadata.len())),
EntrySize::Bytes(ByteSize::b(metadata.len())),
last_modification_date,
symlink_dest,
));
Expand Down Expand Up @@ -302,9 +324,15 @@ pub fn directory_listing(
SortingMethod::Size => entries.sort_by(|e1, e2| {
// If we can't get the size of the entry (directory for instance)
// let's consider it's 0b
e2.size
.unwrap_or_else(|| ByteSize::b(0))
.cmp(&e1.size.unwrap_or_else(|| ByteSize::b(0)))
match (e1.size, e2.size) {
(EntrySize::EntryCount(ref e1_count), EntrySize::EntryCount(ref e2_count)) => e2_count.cmp(e1_count),
(EntrySize::Bytes(ref e1_bytes), EntrySize::Bytes(ref e2_bytes)) => e2_bytes.cmp(e1_bytes),
(EntrySize::EntryCount(_), EntrySize::Bytes(_)) => std::cmp::Ordering::Greater,
(EntrySize::Bytes(_), EntrySize::EntryCount(_)) => std::cmp::Ordering::Less,
}
// e2.size
// .unwrap_or_else(|| ByteSize::b(0))
// .cmp(&e1.size.unwrap_or_else(|| ByteSize::b(0)))
Comment on lines +364 to +366
Copy link
Owner

Choose a reason for hiding this comment

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

Also this.

}),
SortingMethod::Date => entries.sort_by(|e1, e2| {
// If, for some reason, we can't get the last modification date of an entry
Expand Down
18 changes: 11 additions & 7 deletions src/renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,19 +511,23 @@ fn entry_row(
}

@if !raw {
@if let Some(size) = entry.size {
span.mobile-info.size {
(maud::display(size))
}
span.mobile-info.size {
(maud::display(&entry.size))
}
// @if let size = entry.size {
// span.mobile-info.size {
// (maud::display(size))
// }
// }
}
}
}
}
td.size-cell {
@if let Some(size) = entry.size {
(maud::display(size))
}
(maud::display(&entry.size))
// @if let size = entry.size {
// (maud::display(size))
// }
Comment on lines +521 to +534
Copy link
Owner

Choose a reason for hiding this comment

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

What's with the commented code here?

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I didn't remove that since it was the code that would need to be switched on if we were going to throw everything behind a feature flag.

}
td.date-cell {
@if let Some(modification_date) = convert_to_utc(entry.last_modification_date) {
Expand Down