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 themes, language & files by score & then name #2675

Merged

Conversation

michaeljones
Copy link
Contributor

@michaeljones michaeljones commented Jun 4, 2022

I haven't contributed before. I'm not attached to this change. I realise it might not be considered worth while. Just excited by the project and noticed this minor thing.

Previously the themes were appearing unordered after typing :theme .
This sorts them first by fuzzy score and then by name so that they
generally appear in a more ordered fashion in the initial list.

The sort by name does not really pay off when there is a score so an
alternative approach would be to sort by score if there is string to
fuzzy match against and otherwise sort by name.

I've lowercased the names as that avoids lower case & upper case letters
being sorted into separate groups. There might be a preferable approach
to that though.

Previously the themes were appearing unordered after typing ':theme '.
This sorts them first by fuzzy score and then by name so that they
generally appear in a more ordered fashion in the initial list.

The sort by name does not really pay off when there is a score so an
alternative approach would be to sort by name if there is string to
fuzzy match against and otherwise sort by score.

I've lowercased the names as that avoids lower case & upper case letters
being sorted into separate groups. There might be a preferable approach
to that though.
helix-term/src/ui/mod.rs Outdated Show resolved Hide resolved
@michaeljones
Copy link
Contributor Author

There are also other parts of that source code that might have the same change. We sort settings, language and filename_impl by reverse score and not alphabetically. I haven't looked into triggering those though so I'm not sure what the user experience is like.

@the-mikedavis
Copy link
Member

It looks like settings (:set ) already happens to be alphabetically sorted. language (:lang ) is in the order in languages.toml which is a little unorganized, I think that could be sorted alphabetically too. For filename_impl (:open ) it looks like the original ordering comes from however ignore::WalkBuilder is sourcing the files, alphabetical could be good there too.

And change to use sort_unstable_by instead of sort_unstable_by_key as it
allows us to avoid some allocations.

I don't fully understand the flow of the 'filename_impl' function but
this seems to deliver the desired results.
Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
@michaeljones
Copy link
Contributor Author

As you've noted, I've added a commit which changes to the non-allocating sorting approach and adds sorting to the language & files code paths.

@michaeljones michaeljones changed the title Sort themes by score & then name Sort themes, language & files by score & then name Jun 7, 2022
Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Thanks!

n0s4 pushed a commit to n0s4/helix that referenced this pull request Jul 1, 2022
* Sort themes by score & then name

Previously the themes were appearing unordered after typing ':theme '.
This sorts them first by fuzzy score and then by name so that they
generally appear in a more ordered fashion in the initial list.

The sort by name does not really pay off when there is a score so an
alternative approach would be to sort by name if there is string to
fuzzy match against and otherwise sort by score.

I've lowercased the names as that avoids lower case & upper case letters
being sorted into separate groups. There might be a preferable approach
to that though.

* Sort language & files by score then name

And change to use sort_unstable_by instead of sort_unstable_by_key as it
allows us to avoid some allocations.

I don't fully understand the flow of the 'filename_impl' function but
this seems to deliver the desired results.

* Remove unnecessary reference

Co-authored-by: Michael Davis <mcarsondavis@gmail.com>

Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
@the-mikedavis the-mikedavis merged commit 1fde77a into helix-editor:master Jul 1, 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.

3 participants