nvim-telescope / telescope-fzf-native.nvim

FZF sorter for telescope written in c
1.38k stars 45 forks source link

bug: items with the same score should consider length #30

Closed ajitid closed 3 years ago

ajitid commented 3 years ago

FZF uses string length as a tiebreaker by default when the score is tied between two results. Would it be possible to implement it in telescope-fzf-native too?

In FZF:

image

In telescope-fzf-native:

image

Here in junegunn/fzf the tiebreaker logic is implemented.

Conni2461 commented 3 years ago

I would actually say this is a bug, and i am aware of this for quite some time. The problem is, i currently only do calculating of the score and telescope core handles inserting (sorting) into a list. I talked with tj about this the last 2 weeks or so how we could tackle this issue. The first idea was we just add that check in telescope core (if score == next_item.score then prio len). But since then i had a different (maybe better idea) what if we could replace the inserting into a data structure similar to how we can currently replace the score calculation. The latter might give us performance improvements if i would implement it in c.

I currently on a telescope break till August 4th so i dont have time to do it this week. I will look into it as soon as i am back.

Thanks for reporting :) now people have an issue to track progress :laughing:

ajitid commented 3 years ago

Unrelated but, what data structure are you guys using to sort items with respect to their score? Is it a priority queue or something else? Also, if possible, could you point me to the code where it is implemented?

Conni2461 commented 3 years ago

Just a linked list and combination with entry_manager. We currently only sort the first n (shown) items. This might change when we implemented https://github.com/nvim-telescope/telescope.nvim/issues/42

We add new entries here. https://github.com/nvim-telescope/telescope.nvim/blob/master/lua/telescope/pickers.lua#L1020

And this diff should fix this issue.

diff --git a/lua/telescope/entry_manager.lua b/lua/telescope/entry_manager.lua
index ff8b9ba..8fbf297 100644
--- a/lua/telescope/entry_manager.lua
+++ b/lua/telescope/entry_manager.lua
@@ -155,7 +155,10 @@ function EntryManager:add_entry(picker, score, entry)
     info.looped = info.looped + 1

     if container[2] > score then
-      -- print("Inserting: ", picker, index, node, new_container)
+      return self:_insert_container_before(picker, index, node, new_container)
+    end
+
+    if score < 1 and container[2] == score and #entry.ordinal < #container[1].ordinal then
       return self:_insert_container_before(picker, index, node, new_container)
     end

I need to think about this more.

Conni2461 commented 3 years ago

addressed in https://github.com/nvim-telescope/telescope.nvim/pull/987

Conni2461 commented 3 years ago

fixed in latest telescope version

windwp commented 2 years ago

@Conni2461 hi, I want to disable that options. can you help a bit :smile:

Conni2461 commented 2 years ago

the score sorting is currently hardcoded here. https://github.com/nvim-telescope/telescope.nvim/blob/e7aa63db8fea4983658d03b5000d59f75259638b/lua/telescope/entry_manager.lua#L154-L170

It would be cool if that would be made configurable. Like either by a sorter or by a user. Then we could implement a sorter that tries to keep the original sorting. PR welcome, i haven't had the time to do it myself :)