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

feat(plugins): add Glance.nvim #920

Merged
merged 11 commits into from
Aug 6, 2023
Merged

Conversation

Jint-lzxy
Copy link
Collaborator

@vollowx
Copy link
Contributor

vollowx commented Aug 2, 2023

Maybe a better LSP functions redirecting implementation

---@diagnostic disable: duplicate-set-field
-- Override LSP handler functions
-- stylua: ignore start
vim.lsp.buf.references = function() glance.open('references') end
vim.lsp.buf.definition = function() glance.open('definitions') end
vim.lsp.buf.type_definition = function() glance.open('type_definitions') end
vim.lsp.buf.implementations = function() glance.open('implementations') end
-- stylua: ignore end
---@diagnostic enable: duplicate-set-field

Copy link
Contributor

@vollowx vollowx left a comment

Choose a reason for hiding this comment

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

A little fix of appearance.


glance.setup({
height = 20,
zindex = 50,
Copy link
Contributor

Choose a reason for hiding this comment

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

Catppuccin use darker colors for higher layers.

theme = {
  mode = 'darken',
},

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But IMO it would be better to make this slightly brighter cause it would help distinguish the main window from other floating ones.

@fecet
Copy link
Contributor

fecet commented Aug 2, 2023

If the definition target are the current buffer, will glance destroy all keymap in the buffer? I fix lspsaga for this nvimdev/lspsaga.nvim#1188?

@vollowx
Copy link
Contributor

vollowx commented Aug 2, 2023

If the definition target are the current buffer, will glance destroy all keymap in the buffer? I fix lspsaga for this nvimdev/lspsaga.nvim#1188?

I have used glance for weeks, and didn't meet this issue yet.

@fecet
Copy link
Contributor

fecet commented Aug 2, 2023

That will cause troulbe only in certain case, say you are using a repl should contact to your current buffer or you have some keymap conflict with it, like I set <CR> to do incremental selection and this was defined in peek_definition in lspsaga.

@ayamir
Copy link
Owner

ayamir commented Aug 2, 2023

One problem is that glance.nvim doesn't have the same ability on lsp goto definition.
I can't goto the definition place when it is defined in other files.
It should be the bug of glance.nvim b/c even the default lsp handler support this.

image

What's more, the way of overriding lsp handler breaks Lspsaga goto_definition defined in keymap/completion.lua.

@Jint-lzxy
Copy link
Collaborator Author

One problem is that glance.nvim doesn't have the same ability on lsp goto definition. I can't goto the definition place when it is defined in other files. It should be the bug of glance.nvim b/c even the default lsp handler support this.

image What's more, the way of overriding lsp handler breaks `Lspsaga goto_definition` defined in `keymap/completion.lua`.

Ah sorry I forgot to mention this - I made this change intentionally. But after cfeea4f, indeed it's better to bring the original functionality back. Will push a fix for this.

@Jint-lzxy
Copy link
Collaborator Author

313b49a

@Jint-lzxy
Copy link
Collaborator Author

If the definition target are the current buffer, will glance destroy all keymap in the buffer?

@fecet I scanned the source code while setting up this plugin, and I don't think it's an issue b/c Glance works quite similarly to our utils.keymap module (i.e., "fixing", not "replacing" the keymap). To verify this, Q should echo out MSG before and after Glance gets opened.

:noremap <silent> <nowait> <buffer> Q <Cmd>echo 'MSG'<CR>

@fecet
Copy link
Contributor

fecet commented Aug 2, 2023

Great, I will test if it works will with my repl later

@Jint-lzxy Jint-lzxy changed the title feat: add Glance.nvim feat(plugins): add Glance.nvim Aug 2, 2023
Jint-lzxy and others added 3 commits August 3, 2023 01:30
Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: 冷酔閑吟 <50296129+Jint-lzxy@users.noreply.github.com>
@fecet
Copy link
Contributor

fecet commented Aug 3, 2023

I originally thought we were going to use glance to replace the peek function of lspsaga, so my previous concerns were irrelevant. But why not?

@ayamir
Copy link
Owner

ayamir commented Aug 3, 2023

I originally thought we were going to use glance to replace the peek function of lspsaga, so my previous concerns were irrelevant. But why not?

Does it support definition preview?

@fecet
Copy link
Contributor

fecet commented Aug 3, 2023

Doesn't know now but I used glance to preview definition before switching to lspsaga

@Jint-lzxy
Copy link
Collaborator Author

I originally thought we were going to use glance to replace the peek function of lspsaga, so my previous concerns were irrelevant. But why not?

3aae05b

@fecet
Copy link
Contributor

fecet commented Aug 3, 2023

glance's gd works fine for me

Copy link
Owner

@ayamir ayamir left a comment

Choose a reason for hiding this comment

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

LGTM now

Copy link
Collaborator

@aarnphm aarnphm left a comment

Choose a reason for hiding this comment

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

Works on my end

@ayamir ayamir merged commit ac877da into ayamir:main Aug 6, 2023
2 checks passed
singlemancombat pushed a commit to singlemancombat/nvim-config that referenced this pull request Aug 7, 2023
* feat: add `Glance.nvim`

* feat(glance): override lsp handler functions

* fixup! the spec is missing :'(

* fix(Glance): resolve target URI correctly

* fix(glance-override): future proof arg from all buf lsp handler

Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: 冷酔閑吟 <50296129+Jint-lzxy@users.noreply.github.com>

* fix CI

* fix formatting

* feat: use glance to preview definitions

* cleanup

* style(catppuccin): cleanup

---------

Signed-off-by: 冷酔閑吟 <50296129+Jint-lzxy@users.noreply.github.com>
Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
@CharlesChiuGit
Copy link
Collaborator

Works for me too.

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.

6 participants