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

autopairs integration #118

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

HyperAfnan
Copy link

No description provided.

@HyperAfnan
Copy link
Author

Hey Max, Sorry it took me a while because I got some urgent work yesterday.
I have added autopairs with the use of pcall() for testing 
And if it goes well. We can make an integration.autopairs field in the care.config that can be used to toggle the autopairs integration code and later it can be used to add integrations for more plugins like tabout or any icon plugin .

@HyperAfnan
Copy link
Author

hey @max397574 any update? does it work for you?

@max397574
Copy link
Owner

max397574 commented Oct 31, 2024

I will test this out later
the code looks good and it is the correct place to add it

I'm not sure if we want plugin-specific code in the codebase
I'm not directly against it but I don't like it in this location

I think the best solution would be to have a module somewhere where these integrations are stored and we also create a builtin one (which just adds the ()). I could also add the builtin one myself later

configuration would then either be an option which takes a callback (so a function, to which the entry gets passed)
or we just tell users to create an autocmd (because there already are autocommands (not sure if there are at the end of confirming but if not it's easy to add))

Copy link
Owner

@max397574 max397574 left a comment

Choose a reason for hiding this comment

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

looks good so far
see my comment for more information

@@ -173,6 +173,11 @@
--- Auto will prefer right if there is enough space
---@field position? "auto"|"left"|"right"

--- Configuration of integrations with other plugin
---@class care.config.integration
Copy link
Owner

Choose a reason for hiding this comment

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

note to myself: add to docgen so it gets link after merging

@@ -51,6 +51,28 @@ return function(entry)
local is_snippet = completion_item.insertTextFormat == 2
local snippet_text

-- Integrates nvim-autopairs with care.nvim
if config.integration.autopairs then
local autopairs = require("nvim-autopairs")
Copy link
Owner

Choose a reason for hiding this comment

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

still use pcall and use vim.notify to give a warning if autopairs is not available

because some people always manage to mess things up like this and I'd rather not get people reporting errors

Copy link
Author

Choose a reason for hiding this comment

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

okey ill use pcall and vim.notify

@HyperAfnan
Copy link
Author

I think the best solution would be to have a module somewhere where these integrations are stored and we also create a builtin one (which just adds the ()). I could also add the builtin one myself later

okey I'll the integration code into a module

configuration would then either be an option which takes a callback (so a function, to which the entry gets passed)
or we just tell users to create an autocmd (because there already are autocommands (not sure if there are at the end of confirming but if not it's easy to add))

Are you saying to make it like this for the autocmd part?

cmp.event:on(
				"confirm_done",
				cmp_autopairs.on_confirm_done({
					filetypes = {
						["*"] = {
							["("] = {
								kind = {
									cmp.lsp.CompletionItemKind.Function,
									cmp.lsp.CompletionItemKind.Method,
								},
								handler = handlers["*"],
							},
						},
					},
				})
			)

this is the code to integrate nvim-cmp with nvim-autopairs

@max397574
Copy link
Owner

yes similar like what cmp does

except that we use autocommands and not a weird self-built even system

what do you think?
should we rather use a callback in the config or an autocmd?
I'd personally say autocmd because 1. there are a ton of config options already and 2. we don't know if more callback will exist in the future

@HyperAfnan
Copy link
Author

Yeah I also think adding autocmd would be a great idea
it will also allow people to use other plugins like nvim-autopairs such as ultimate-autopairs or mini.pairs

@max397574
Copy link
Owner

@HyperAfnan do you think you'll find time to finish this soon?

Otherwise I could take this over

@HyperAfnan
Copy link
Author

Sorry, this week was too hectic for me, but tomorrow is Sunday, so I'll complete this by tomorrow.

@max397574
Copy link
Owner

No worries
I myself I'm often quite busy and just would have picked this up if you wouldn't have been able to work on it during the next few weeks

@HyperAfnan
Copy link
Author

Hey max , I have created an event listener for the event called confirm_done which is called in the care/menu/confirm.lua file

@max397574
Copy link
Owner

#118 (comment)

except that we use autocommands and not a weird self-built even system

😳

@HyperAfnan
Copy link
Author

can you tell me how should it look like ? I'm out of ideas

@max397574
Copy link
Owner

well just how it was before
with a user autocmd

and then just add the code for the autopairs logic somewhere and a configuration recipe with how to setup the autocmd

@HyperAfnan
Copy link
Author

okey i gotcha

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.

2 participants