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.

Sign up for GitHub

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

Jump to bottom

Sort files in file picker by access, modification and creation date #336

Merged
merged 4 commits into from
Jun 26, 2021
Merged

Sort files in file picker by access, modification and creation date #336

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7618660
Sort files in file picker by access date
vv9k Jun 21, 2021
25ec013
Fallback file time to modified then created then UNIX_EPOCH
vv9k Jun 22, 2021
96e8561
Use `sort_by_key`
vv9k Jun 22, 2021
454e44f
Refactor
vv9k Jun 25, 2021
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
23 changes: 21 additions & 2 deletions helix-term/src/ui/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,25 +78,44 @@ pub fn regex_prompt(

pub fn file_picker(root: PathBuf) -> Picker<PathBuf> {
use ignore::Walk;
use std::time;
let files = Walk::new(root.clone()).filter_map(|entry| match entry {
Ok(entry) => {
// filter dirs, but we might need special handling for symlinks!
if !entry.file_type().map_or(false, |entry| entry.is_dir()) {
Some(entry.into_path())
let time = if let Ok(metadata) = entry.metadata() {
if let Ok(atime) = metadata.accessed() {
vv9k marked this conversation as resolved.
Show resolved Hide resolved
atime
} else if let Ok(mtime) = metadata.modified() {
mtime
} else if let Ok(ctime) = metadata.created() {
ctime
} else {
time::UNIX_EPOCH
}
vv9k marked this conversation as resolved.
Show resolved Hide resolved
} else {
time::UNIX_EPOCH
};

Some((entry.into_path(), time))
} else {
None
}
}
Err(_err) => None,
});

let files = if root.join(".git").is_dir() {
let mut files: Vec<_> = if root.join(".git").is_dir() {
files.collect()
} else {
const MAX: usize = 8192;
files.take(MAX).collect()
};

files.sort_by(|(_, time1), (_, time2)| time1.cmp(time2));

let files = files.into_iter().map(|(path, _)| path).collect();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we collect into two vec using unzip. Then, zip it into an iterator and use sort_by_key from the second iterator. So with this method we can remove the map and collect to files. What do you think?

Both methods collect twice, but the one I suggested collects beforehand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you sort an iterator though? I was sure that the methods are not available.

Copy link
Contributor Author

@vv9k vv9k Jun 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Also, as a side note, I think this approach is a bit easier to read, but I'll let others chime in to decide.

Copy link
Contributor

@pickfire pickfire Jun 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Wait no, sorry. Don't zip the iterator but just sort using the slice. Both approach seemed similar to me with the one I suggested taking less lines of code.

Also, both approach I will recommend using sort_by_key rather than sort_by.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, now we can use sort_by_key. Done.


Picker::new(
files,
move |path: &PathBuf| {
Expand Down