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

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

Merged
merged 4 commits into from
Jun 26, 2021

Conversation

vv9k
Copy link
Contributor

@vv9k vv9k commented Jun 21, 2021

So this might not be very optimized but it's a big quality of life improvement. I use file picker a lot and sometimes I just want to see what I edited last without having to remember the exact filename. I tested this change on a quiet big repository - https://fuchsia.googlesource.com/fuchsia/ and the additional time to sort the entries was negligible even in debug mode.

I'm ok not to push it either if that will be the general consensus.

EDIT:
Actually meant access time not modification time

@vv9k vv9k changed the title Sort files in file picker by modification date Sort files in file picker by access date Jun 21, 2021
@pickfire
Copy link
Contributor

I think it would be good to have an option to change sorting priority, this could be a default but I don't know which is better. What if other users search?

helix-term/src/ui/mod.rs Outdated Show resolved Hide resolved
Copy link
Member

@sudormrfbin sudormrfbin left a comment

Choose a reason for hiding this comment

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

I tested this locally, but normal files (without an lsp running for them) only seem to be updated when opening and closing the editor. Rust files for example show up at the top right after saving them.

helix-term/src/ui/mod.rs Outdated Show resolved Hide resolved
helix-term/src/ui/mod.rs Outdated Show resolved Hide resolved
@vv9k vv9k changed the title Sort files in file picker by access date Sort files in file picker by access, modification and creation date Jun 22, 2021
Comment on lines 115 to 117
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

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

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.

@cessen
Copy link
Contributor

cessen commented Jun 22, 2021

Definitely a fan of this. I have a related feature I'd like to implement that I think will complement this: sorting open buffers in the space-b command by most-recently-viewed. That way your working set of documents (the ones you've most recently accessed) are always at the top of the buffer list.

@archseer also mentioned the possibility of merging the space-f file and space-b buffer lists into a single command, in which case the buffers (sorted by most-recently-viewed) could be at the top, and files that aren't opened in buffers (sorted by filesystem access time) go below that. That would give a single file/buffer navigation command that always has the most relevant stuff at the top.

@sudormrfbin
Copy link
Member

Merging space-f and space-b is something I've been thinking about too. It's unnecessary cognitive overload to think where the particular file can be opened from - if it's outside the cwd then you'd have to use the buffer picker else the file picker works fine. Definitely a quality of life improvement.

@pickfire
Copy link
Contributor

The other idea I have is sort of space space in doom emacs which we can navigate between directories easily and show only the current directory so that is at least easier compared to :e xxx/<tab>.

helix-term/src/ui/mod.rs Outdated Show resolved Hide resolved
files.collect()
} else {
const MAX: usize = 8192;
files.take(MAX).collect()
};

files.sort_by_key(|file| file.1);
Copy link
Member

Choose a reason for hiding this comment

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

Note: this makes it harder to pass input lazily later

@archseer archseer merged commit eb6fb63 into helix-editor:master Jun 26, 2021
cessen pushed a commit to cessen/helix that referenced this pull request Jun 26, 2021
…elix-editor#336)

* Sort files in file picker by access date

* Fallback file time to modified then created then UNIX_EPOCH

* Use `sort_by_key`

* Refactor
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.

5 participants