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

tiebreak index does not exist #536

Closed
valpackett opened this issue Aug 7, 2023 · 6 comments · Fixed by #609
Closed

tiebreak index does not exist #536

valpackett opened this issue Aug 7, 2023 · 6 comments · Fixed by #609

Comments

@valpackett
Copy link

The readme says

There are five sort keys for results: score, index, begin, end, length, you can specify how the records are sorted by sk --tiebreak score,index,-begin or any other order you want.

However…

https://github.com/lotabout/skim/blob/291fc34c58b1670a5e8c95f1e8f930b82c030b19/src/item.rs#L200-L209

index does not seem to exist at all??

@lotabout
Copy link
Collaborator

lotabout commented Aug 8, 2023

@valpackett It's a doc error. index was removed in commit 94846e7. Can't remember exactly why, but possibly sorting by index could be achieved by --tac.

@blueforesticarus
Copy link

blueforesticarus commented Feb 29, 2024

Based on the documentation, it seems you are meant to be able to set the priority of tiebreakers by specifying multiple.
From what I can tell, with --tac you cannot sort at all.

This is rather unfortunate, as what I'd really like to do is sort by score, and then by index, so that newest - perfect matches show up first (in this case I am skimming browser history, sorted by timestamp)

@j-lakeman
Copy link

Then --tiebreak=index should probably be changed / removed from all shell binding files right? Like this one:

SKIM_DEFAULT_OPTIONS="--height ${SKIM_TMUX_HEIGHT:-40%} $SKIM_DEFAULT_OPTIONS -n2..,.. --tiebreak=index --bind=ctrl-r:toggle-sort $SKIM_CTRL_R_OPTS --no-multi --read0" $(__skimcmd) --query "$READLINE_LINE"

This would be an ez PR I could provide.

@LoricAndre
Copy link
Contributor

I'd say removing it might not be the best option, but I'd love a PR that readds the index, as long as it includes the necessary tests to ensure that if there was a reason for it to be removed, we find it before it hits the repos.

@LoricAndre
Copy link
Contributor

Note: I think I'll actually try my hand at this one, please tell me if you want to work on it to avoid duplicate PRs !

@j-lakeman
Copy link

Go for it!

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 a pull request may close this issue.

5 participants