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!: implement auto-insert option #65

Merged
merged 16 commits into from
Oct 11, 2024
Merged

Conversation

noomly
Copy link
Contributor

@noomly noomly commented Oct 9, 2024

Closes #34

Recording 2024-10-09 at 19 36 24

Still left to do:

  • Figure out a less dirty way of identifying whether a character update comes from selecting rather than accepting an item (see trigger.triggered_by). I'm open to suggestions on this one.
  • Enforce preselect = false when auto_insert = true. We could turn preselect off when auto_insert is on and print a message on setup warning the user about the incompatibility, wdyt?
  • Maybe call accept when implicitly accepting a completion item by selecting it and pressing a non-completable character (like ( for ex) as mentioned here Allow auto-inserting completion items #34 (comment).
  • Handle snippets

@noomly noomly marked this pull request as draft October 9, 2024 20:01
@tkapous
Copy link

tkapous commented Oct 9, 2024

Maybe there should be only one option for selection behaviour, e.g select: 'auto_insert' | 'preselect' | nil?

@noomly
Copy link
Contributor Author

noomly commented Oct 10, 2024

Maybe there should be only one option for selection behaviour, e.g select: 'auto_insert' | 'preselect' | nil?

That's a much better idea than having two separate booleans yes! Will implement that tomorrow.

@noomly noomly changed the title feat: implement auto-insert option feat!: implement auto-insert option Oct 10, 2024
@noomly noomly marked this pull request as ready for review October 10, 2024 15:48
@noomly noomly marked this pull request as draft October 10, 2024 15:57
@noomly noomly marked this pull request as ready for review October 10, 2024 16:39
@noomly
Copy link
Contributor Author

noomly commented Oct 10, 2024

I think the PR is ready to be reviewed @Saghen. I'm still not sure about the first checklist point, and I also added another comment.

I tested as much edge cases as I could think of withvtsls, luals and rust_analyzer but it's definitely possible that I missed some which could end up botching up the insertion when auto-insert is on. I'll be watching the issues for related bugs once this PR is merged.

@Saghen
Copy link
Owner

Saghen commented Oct 11, 2024

I've added a wrapper on the trigger module so we can edit the buffer without triggering the sources/fuzzy 19e1312. Wicked job on this PR, thank you!

@Saghen Saghen merged commit 1df7d33 into Saghen:main Oct 11, 2024
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.

Allow auto-inserting completion items
3 participants