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

Implement nvim_create_user_command #922

Merged
merged 1 commit into from
May 2, 2022
Merged

Conversation

mehalter
Copy link
Contributor

This moves the creation of the :CmpStatus command to the new vim.api.nvim_create_user_command API

This requires neovim 0.7 so it should be done along with #844

plugin/cmp.lua Outdated Show resolved Hide resolved
@hrsh7th
Copy link
Owner

hrsh7th commented Apr 20, 2022

LGTM.

plugin/cmp.lua Outdated
vim.cmd [[command! CmpStatus lua require('cmp').status()]]
vim.api.nvim_create_user_command(
'CmpStatus',
require('cmp').status,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please be aware that it will require cmp at the time that command is created. To avoid that it should be wrapped in lambda/anonymous function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking this as well. By the time this file is loaded doesn't that imply that cmp is available at the time? Is there a case where this plugin/cmp.lua file will be loaded but the plugin itself will not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested this nvim_create_user_command with some pretty heavy lazy loading in different cases and couldn't get it in a state where this caused issues when the vim.api.nvim_create_user_command was called.

Copy link
Contributor

Choose a reason for hiding this comment

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

True, but plugin/cmp.lua will be sourced by nvim as a startup script (if added to rtp). So those requires are happening any way https://github.com/mehalter/nvim-cmp/blob/command_api/plugin/cmp.lua#L6-L10.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, so just to clarify, should I update it to wrap it in an anonymous function?

Copy link
Contributor

Choose a reason for hiding this comment

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

@hrsh7th Have you got an opinion here?

Copy link
Owner

Choose a reason for hiding this comment

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

For my personal taste, I think it's better to wrap it in an anonymous function. (This is because we can eliminate the assumption that the arguments given to the anonymous function implicitly match the status function.)

google translated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a great point @hrsh7th I have updated the commit to wrap it in an anonymous function.

@hrsh7th
Copy link
Owner

hrsh7th commented May 2, 2022

LGTM. Thank you!

@hrsh7th hrsh7th merged commit 7115894 into hrsh7th:dev May 2, 2022
hrsh7th pushed a commit that referenced this pull request May 2, 2022
hrsh7th added a commit that referenced this pull request May 3, 2022
* Adjust empty line count

* Implement vim.api.nvim_create_autocmd (#844)

* Implement vim.api.nvim_create_autocmd

* Only use vim.api.nvim_create_autocmd on nighly

* Cleanup

* cleanup

* Rename autos.lua to autocmds.lua

* Fix forgot to rename autos to autocmds

* Remove legacy autocmd

* Add descriptions on autocmds

* Update descriptions on autocmds

* Update CmpStatus command to lua API (#922)

Signed-off-by: Micah Halter <[email protected]>

* Move highlights to nvim_set_hl lua API (#925)

Signed-off-by: Micah Halter <[email protected]>

* Add default to highlight

* Refactor autocmds

* fmt

* Improve performance

* Fix bug

* Improve matching logic
Fixes #954

* Fix format

* Improve performance
Fix #825

* Fix cmdline redraw bug

* Fix event

Co-authored-by: hrsh7th <>
Co-authored-by: zer09 <[email protected]>
Co-authored-by: Micah Halter <[email protected]>
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.

4 participants