mgunyho / tere

Terminal file explorer
European Union Public License 1.2
1.67k stars 36 forks source link

Sorting #64

Closed joshrdane closed 1 year ago

joshrdane commented 2 years ago

Sorting Options

Notes

mgunyho commented 2 years ago

Thanks! I'm busy this weekend but I'll take a look early next week.

As a quick note, --sort X is kind of inconsistent with other similar "cyclable" options (like e.g. case sensitive mode), but I think this is definitely the right way to do it. I'm not sure why I did it the other way with the others tbh, I should think about that again.

joshrdane commented 1 year ago

Yeah, the column header is something that we can put in there and that would also free up the area I allocated for sorting in the footer. I will take some time this weekend to address the issues in the comments.

mgunyho commented 1 year ago

Merging the master made the diff a bit harder to interpret, I'll do a rebase on to develop.

mgunyho commented 1 year ago

Oops, that turned out to be a bit trickier than I thought. I cleaned up the version history a bit, split some things off into separate commits and removed some unnecessary ones (like importing intrinsics::unreachable and then removing the import). You will need to do git fetch and then git reset --hard origin/sorting to have your local version match up with this one, just make sure that you don't lose any work. I won't have time to do the review anymore today, but I'll get back to it soon.

mgunyho commented 1 year ago

Ah, one more thing that is missing: the new feature should be mentioned in the changelog, and also the keyboard shortcut and CLI option should be documented in the readme (there is a test to check that all default shortcuts are listed).

mgunyho commented 1 year ago

Oh yeah and I also tried to create some unit tests for this, but you're right, modifying the timestamp of a Metadata is not really possible, and update_ls_output_buf is hardcoded to read the current folder, which you can't really change inside a test... so let's just rely on manual testing for now.

mgunyho commented 1 year ago

I went ahead and implemented these fixes myself, I hope you don't mind.

I'll do another rebase and then merge this.

mgunyho commented 1 year ago

Thanks a lot!