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.

Sign up for GitHub

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

Jump to bottom

feat(cmp): reimplement comparators for completion entries #746

Merged
merged 3 commits into from
May 16, 2023
Merged

feat(cmp): reimplement comparators for completion entries #746

merged 3 commits into from
May 16, 2023

Conversation

ayamir
Copy link
Owner

@ayamir ayamir commented May 16, 2023 •
edited
Loading

@ayamir
Copy link
Owner Author

ayamir commented May 16, 2023

The sort logic for nvim-cmp is required to update to provide a better completion list.
Currently I just set the sort logic same with nvim-cmp's default.

@ayamir ayamir linked an issue May 16, 2023 that may be closed by this pull request
can't toggle the recently suggestion #744
Closed
@Jint-lzxy
Copy link
Collaborator

#744 (comment)

@Jint-lzxy
Copy link
Collaborator

Done. Also set window.completion.scrollbar = false, @ayamir I can't quite understand why scrollbar was disabled manually 🤔

@Jint-lzxy Jint-lzxy marked this pull request as ready for review May 16, 2023 13:24
@Jint-lzxy Jint-lzxy changed the title feat(cmp): update sort logic. feat(cmp): reimplement comparators for completion entries May 16, 2023
@ayamir
Copy link
Owner Author

ayamir commented May 16, 2023

I can't quite understand why scrollbar was disabled manually

Just keep the ui more concise like nvchad do lol.

@ayamir
Copy link
Owner Author

ayamir commented May 16, 2023 •
edited
Loading

-- compare.locality, -- This conflicts with offset

why?

@Jint-lzxy
Copy link
Collaborator

I can't quite understand why scrollbar was disabled manually

Just keep the ui more concise like nvchad do lol.

Got it 😄 btw, I replace manual restriction of scrollable with window.completion.scrollbar = false, did this break anything?

@ayamir
Copy link
Owner Author

ayamir commented May 16, 2023

did this break anything?

No, works as expect.

@Jint-lzxy
Copy link
Collaborator

-- compare.locality, -- This conflicts with offset

why?

These two comparators AFAIU have the following traits:

  • compare.offset
    • Based on items' offset in the text buffer. Items closer to the cursor position will have lower priority.
  • compare.locality
    • <...>. Items closer to the cursor will have higher priority.

@ayamir
Copy link
Owner Author

ayamir commented May 16, 2023

-- compare.locality, -- This conflicts with offset

why?

These two comparators AFAIU have the following traits:

  • compare.offset

    • Based on items' offset in the text buffer. Items closer to the cursor position will have lower priority.

  • compare.locality

    • <...>. Items closer to the cursor will have higher priority.

OK, maybe it's better to provide a brief intro for each comparator to help user decide what to use?

@Jint-lzxy
Copy link
Collaborator

Done :)

Jint-lzxy
Jint-lzxy approved these changes May 16, 2023
View reviewed changes
Copy link
Collaborator

@Jint-lzxy Jint-lzxy left a comment

Choose a reason for hiding this comment

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

OK, maybe it's better to provide a brief intro for each comparator to help user decide what to use?

Just found an upstream PR that provides such documentation 👍 So this PR is good.

@ayamir
Copy link
Owner Author

ayamir commented May 16, 2023

OK, maybe it's better to provide a brief intro for each comparator to help user decide what to use?

Just found an upstream PR that provides such documentation 👍 So this PR is good.

OK, let's merge it now.

@ayamir ayamir merged commit a860c4b into main May 16, 2023
@ayamir ayamir deleted the feat/cmp-sort branch May 16, 2023 14:22
singlemancombat pushed a commit to singlemancombat/nvim-config that referenced this pull request May 16, 2023
@ayamir @Jint-lzxy @singlemancombat
feat(cmp): reimplement comparators for completion entries (ayamir#746)
4262b3b 
* feat(cmp): update sort logic.

* fixup! feat(cmp): update sort logic

* fixup commit

---------

Co-authored-by: Jint-lzxy <[email protected]>
@TonyWu20 TonyWu20 mentioned this pull request May 22, 2023
Merged
boomker pushed a commit to boomker/uvcode-nvim that referenced this pull request May 24, 2023
@ayamir @boomker
feat(cmp): reimplement comparators for completion entries (ayamir#746)
faab394 
* feat(cmp): update sort logic.

* fixup! feat(cmp): update sort logic

* fixup commit

---------

Co-authored-by: Jint-lzxy <[email protected]>
(cherry picked from commit a860c4b)
bleedingfight pushed a commit to bleedingfight/nvimdots that referenced this pull request Jun 22, 2023
@ayamir @Jint-lzxy @bleedingfight
feat(cmp): reimplement comparators for completion entries (ayamir#746)
d54e61d 
* feat(cmp): update sort logic.

* fixup! feat(cmp): update sort logic

* fixup commit

---------

Co-authored-by: Jint-lzxy <[email protected]>
@Jint-lzxy Jint-lzxy mentioned this pull request Aug 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jint-lzxy Jint-lzxy Jint-lzxy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

can't toggle the recently suggestion
2 participants
@ayamir @Jint-lzxy