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

Add completion-at-point implementation ocaml-community/utop#261 #406

Merged
merged 1 commit into from
Apr 19, 2023

Conversation

j-shilling
Copy link
Contributor

After poking around for ways to use my normal completion-at-point workflow in the utop buffer, I came accross issue #261, which is a feature request to provide a completion-at-point-fuction.

This is an implementation I came up with that works for me and I wanted to share it here in case anyone else found it useful or had any feedback.

@j-shilling j-shilling requested a review from rgrinberg as a code owner December 1, 2022 19:40
@rgrinberg
Copy link
Collaborator

@bbatsov could you review?

src/top/utop.el Outdated Show resolved Hide resolved
src/top/utop.el Outdated Show resolved Hide resolved
src/top/utop.el Outdated Show resolved Hide resolved
@bbatsov
Copy link
Contributor

bbatsov commented Dec 2, 2022

Overall the code looks good to me. In general I think this should be enabled by default - probably unconditionally, given it's the more generic method of completion in Emacs. I see very little point to have to enable manually a capf function. Given how basic the company-backend is, probably at some point it can go away, as company has a capf backend anyways.

@bbatsov
Copy link
Contributor

bbatsov commented Dec 2, 2022

While you're at this - I'd also suggest incorporating those doc improvements proposed a while ago #326

@j-shilling
Copy link
Contributor Author

Overall the code looks good to me. In general I think this should be enabled by default - probably unconditionally, given it's the more generic method of completion in Emacs. I see very little point to have to enable manually a capf function. Given how basic the company-backend is, probably at some point it can go away, as company has a capf backend anyways.

As a temporary fix, I added a dispatch function that can be bound to TAB that either calls completion-at-point or starts the company workflow. I haven't used company in a long time, but is there any more to this than just removing the current backend and adding company-capf backend instead in utop-mode?

If it's that simple, I can do that here and then merge in #326 with changes that explain which backend to use. It might also be worth removing the complete command and renaming company-complete along with removing the complete-word response. I'm very new to OCAML, so I might not be the right guy to do that.

@bbatsov
Copy link
Contributor

bbatsov commented Dec 2, 2022

I haven't used company in a long time, but is there any more to this than just removing the current backend and adding company-capf backend instead in utop-mode?

You don't really need to do anything in utop-mode for company-capf to work, provided it registers some completions-at-point function. The purpose of the special company backends is to provide some enhanced functionality, more metadata, etc. Obviously that wasn't the case with the existing backend, that's why I think it becomes kind of redundant.

It might also be worth removing the complete command and renaming company-complete along with removing the complete-word response.

Yeah, I'm not sure why it was made company-specific in utop itself. I guess the author of the functionality was a company user and this was before capf became popular (and company alternatives like corfu).

@bbatsov
Copy link
Contributor

bbatsov commented Apr 18, 2023

@rgrinberg @emillon Seems we forgot about this PR, which is good to merge IMO. (especially given that company-mode as not as popular as it used to be)

@emillon
Copy link
Collaborator

emillon commented Apr 19, 2023

ok, thanks!

@emillon emillon linked an issue Apr 19, 2023 that may be closed by this pull request
@emillon emillon merged commit 0e9f0c6 into ocaml-community:master Apr 19, 2023
@bbatsov
Copy link
Contributor

bbatsov commented Apr 19, 2023

Thanks!

emillon added a commit to emillon/opam-repository that referenced this pull request Apr 21, 2023
CHANGES:

* Fix regression with unit qualification when a `Unit` module is in scope with
  no `()` constructor (ocaml-community/utop#429, fixes ocaml-community/utop#428, @emillon)

* emacs: add completion-at-point implementation (ocaml-community/utop#406, fixes ocaml-community/utop#261, @j-shilling)
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.

utop emacs completion-at-point-functions support?
4 participants