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

Make system.path_compare more case-aware #1457

Merged
merged 2 commits into from
Apr 7, 2023

Conversation

Guldoman
Copy link
Member

@Guldoman Guldoman commented Apr 5, 2023

Before, strings like README.md would be sorted before changelog.md, because we only looked at the raw ascii values.
Now the character case is considered as a secondary sorting key.

Before:
Schermata del 2023-04-05 10-11-57_2

After:
Schermata del 2023-04-05 10-11-41_2

Guldoman added 2 commits April 5, 2023 10:02
Before, strings like `README.md` would be sorted before `changelog.md`, 
because we only looked at the raw ascii values.
Now the character case is considered as a secondary sorting key.
@jgmdev
Copy link
Member

jgmdev commented Apr 7, 2023

Looks good, but it is interesting that github exhibits the same behavior:

github-file-list

I wonder if it is something desirable?

@Guldoman
Copy link
Member Author

Guldoman commented Apr 7, 2023

They probably just use a simple ascii comparison.
Basically every editor and file manager (and ls!) behaves in a similar way to this PR.

@jgmdev
Copy link
Member

jgmdev commented Apr 7, 2023

Basically every editor and file manager (and ls!) behaves in a similar way to this PR.

Fair enough, merging!

@jgmdev jgmdev merged commit 6d4bf4f into lite-xl:master Apr 7, 2023
takase1121 pushed a commit to takase1121/lite-xl that referenced this pull request Aug 19, 2023
* Use Lua-provided string lengths for `system.path_compare`
* Make `system.path_compare` more case-aware
   Before, strings like `README.md` would be sorted before `changelog.md`, 
   because we only looked at the raw ascii values.
   Now the character case is considered as a secondary sorting key.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants