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

Removed sorting from CLI + added sorting from HTML #44

Merged
merged 16 commits into from Mar 6, 2019
Merged

Removed sorting from CLI + added sorting from HTML #44

merged 16 commits into from Mar 6, 2019

Conversation

ghost
Copy link

@ghost ghost commented Feb 25, 2019

So this is my WIP for solving #37

Right now, I can sort by name/size/date, in ascending and descending order by clicking the corresponding header. I'll clean up some parts of the code which, I think, could be simplified. Sorting by name is done using natural sort, does that seem fine to you ?

@svenstaro
Copy link
Owner

It would be optimal if you could try using unicode chars first. Then it will also look fine in lynx. :)

@svenstaro
Copy link
Owner

Once you're happy with your code, remove WIP and I'll do a review.

@svenstaro
Copy link
Owner

svenstaro commented Mar 4, 2019

The default order should be to implicitly sort by name, right? It doesn't seem to be the case for me. I honestly can't tell which order the default is. It seems random.

@svenstaro
Copy link
Owner

Apart from that, this actually seems really good and it's super fast. Great job!

BTW what do you think about highlighting the currently hovered line? Should be a breeze in CSS.

@svenstaro
Copy link
Owner

Also I just took a look and turns out actix-web now has typed URL params: https://docs.rs/actix-web/0.7.18/actix_web/struct.Query.html

So we should probably use those.

@ghost ghost changed the title WIP: Removed sorting from CLI + added sorting from HTML Removed sorting from CLI + added sorting from HTML Mar 4, 2019
@ghost ghost mentioned this pull request Mar 4, 2019
src/listing.rs Outdated Show resolved Hide resolved
src/listing.rs Outdated Show resolved Hide resolved
@svenstaro svenstaro merged commit 6340e9e into svenstaro:master Mar 6, 2019
@svenstaro
Copy link
Owner

Merging as is. Great work!

vojta7 pushed a commit to vojta7/miniserve that referenced this pull request Apr 3, 2019
Removed sorting from CLI + added sorting from HTML
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.

None yet

1 participant