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

Fix: directory sorting #492

Merged
merged 7 commits into from
Jan 31, 2022
Merged

Fix: directory sorting #492

merged 7 commits into from
Jan 31, 2022

Conversation

trapeznikov-ap
Copy link
Contributor

The top directory ".." should always be the first in the list when sorting

@pedrolamas
Copy link
Member

Hi @trapeznikov-ap, thank you for your PR.

From what I can see on my own machine, the ".." always appears on top, no matter what file list I'm looking at, so I was just wondering where did you see this behavior broken?

@trapeznikov-ap
Copy link
Contributor Author

trapeznikov-ap commented Jan 30, 2022

Hi @trapeznikov-ap, thank you for your PR.

From what I can see on my own machine, the ".." always appears on top, no matter what file list I'm looking at, so I was just wondering where did you see this behavior broken?

At me the problem is shown at sorting in descending order by names. Tell me the sort direction and which column are you sorting by? Do you have folders with names that start with an exclamation mark?

@pedrolamas
Copy link
Member

Can you send a screenshot showing this behavior? Are you using cyrillic chars for your filenames and folders?

From me, using regular chars it is working as normal when ordering by Name ascending and descending, so ".." is always on top.

@trapeznikov-ap
Copy link
Contributor Author

image
image
image

@pedrolamas
Copy link
Member

You are correct @trapeznikov-ap , I got a repo of the issue and your fix does mitigate it completely, but I think we can "improve" it a bit, as you should be able to move those 2 new lines outside of the for (so just after the line with sort but before we do the for)

Can you check that please?

@pedrolamas pedrolamas merged commit a2cb15e into fluidd-core:develop Jan 31, 2022
@matmen matmen added this to the Release: 1.17 milestone Feb 22, 2022
matmen pushed a commit to matmen/fluidd that referenced this pull request Jun 27, 2022
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.

4 participants