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

YouCompleteMe-style tab completion? #526

Closed
Valloric opened this issue May 29, 2016 · 41 comments
Closed

YouCompleteMe-style tab completion? #526

Valloric opened this issue May 29, 2016 · 41 comments
Labels

Comments

@Valloric
Copy link

[Full disclosure: I'm the author of YCM so I'm biased. :)]

I've been trying out emacs lately to see can I replicate my uber-customized Vim setup on the dark side :). I'm using company-mode with emacs-ycmd and it's great! Thanks for making company-mode awesome and for continuing to maintain it; OSS dev work is often thankless.

Other than one missing feature, company-mode is doing everything I want it to do.

The best way to describe the feature is with the following gif. (Wait until the part where LongestCommonSubsequenceLength is being completed.)

gif

In the gif, only tab is being used, nothing else (no enter).

In YCM, tab is used to both select a completion candidate from the list and to insert it in the buffer. So the user types and gets completions. Pressing tab selects the first item in the completion menu and inserts it in the buffer. Pressing tab again selects the second item and replaces the inserted item with the second one. This can continue as long as the user wishes to cycle through the menu.

The benefits are the following:

  1. Only one shortcut key to interact with instead of two.
  2. One fewer button press (no need to confirm an entry).

Any chance company-mode can get an option of some sort to behave like this? Something like complete-and-cycle for auto-completion-tab-key-behavior. It would be greatly appreciated, if for any reason then to lessen the cognitive load on those Vim users using Emacs.

@dgutov
Copy link
Member

dgutov commented May 30, 2016

Hi Val,

I'm glad you're happy with company-mode overall.

Thanks for the great explanation; we've discussed this before in the issue #343, and there's a gist linked there that implements this, albeit in a not very clean fashion.

@aaronjensen
Copy link
Contributor

Here's the gist for reference: https://gist.github.com/aaronjensen/a46f88dbd1ab9bb3aa22

I haven't had time to pull request it properly. It has been working well for me, however.

@dgutov
Copy link
Member

dgutov commented May 30, 2016

There's still work to do on the design here. In particular:

  • Advising company-set-selection is not great. Advising the frontends would be better. Having two new frontends would be best; they can wrap the two simpler ones, and do things depending on whether the user has already typed tab. Things like showing the completion inline only after tab, and binding company-selection to -1 locally around company-pseudo-tooltip-frontend when the user hasn't typed tab yet.
  • It seems like company-simple-complete-next should implement an electric loop (maybe using company--electric-do, or Electric-command-loop, or something along these lines). When the user types anything that resolves to an unknown command, you insert the selected completion, exit and call company--unread-last-input.
  • This way, there's no need to change the contents of the buffer until the very end. And all values of company-require-match will be supported.

@aaronjensen
Copy link
Contributor

It seems like company-simple-complete-next should implement an electric loop (maybe using company--electric-do, or Electric-command-loop, or something along these lines). When the user types anything that resolves to an unknown command, you insert the selected completion, exit and call company--unread-last-input.
This way, there's no need to change the contents of the buffer until the very end. And all values of company-require-match will be supported.

I would guess that this would negatively impact the UX unless we could do something like the company-preview-frontend for the selected option. That said, what is the reason to avoid modifying the buffer until the end? Does it cause some problems?

Advising company-set-selection is not great.

Agreed, though from what I could tell, it was the only way I knew how to get this working with the current code. company-set-selection could be changed to allow for -1 as a valid value intrinsically and then not require advising. Until then, anything that called company-set-selection would undo anything that advising the frontends did, unless an entirely different technique of communicating "no selection" was used. My first approach used a different technique, but it was full of bugs.

@dgutov
Copy link
Member

dgutov commented May 30, 2016

we could do something like the company-preview-frontend for the selected option

We would. Hence "two new frontends".

That said, what is the reason to avoid modifying the buffer until the end? Does it cause some problems?

It causes complexity. I don't see why we'd want to do that.

You can't press C-g to quickly abort. If you want the user to be able to undo the insertion in one step (even if they iterated over multiple completions), you now have to handle the undo boundaries. Like mentioned, it also breaks company-require-match.

anything that called company-set-selection would undo anything that advising the frontends did

Advising the frontends would only change the display. Do any of the frontends call company-set-selection?

The "no selection" would be indicated by a new boolean variable.

@dgutov
Copy link
Member

dgutov commented Jun 20, 2016

Not giving up on this request, but in the meantime you could try out the recipe at the bottom of https://github.com/company-mode/company-mode/wiki/Switching-from-AC.

@sickHamm
Copy link

sickHamm commented Jun 26, 2016

@dgutov
A more simple implement company-auto-expand-frontend ; demo gif;
But need some help and TODO

  • manual_ setq `company-prefix' has any negative affect ; Don't need, but have change reality request-complete prefix before run pre-command;
  • Some TODO
  • code maybe could more robust

@dgutov
Copy link
Member

dgutov commented Jun 26, 2016

@sickHamm Sorry, this doesn't look like a good approach, even if it works for now.

Frontends aren't supposed to change anything, they're only for visualization.

Is manual setq `company-prefix' has any negative affect

Yes. But no more than calling private methods, I suppose.

@sergei-dyshel
Copy link

Any possible reason whey the gist by @aaronjensen does not work in Spacemacs?

@aaronjensen
Copy link
Contributor

@sergei-dyshel it works for me. Here's the most up to date version:

https://github.com/aaronjensen/spacemacs.d/blob/master/lisp/company-simple-complete.el

I require it in user-config https://github.com/aaronjensen/spacemacs.d/blob/master/init.el#L397

@sergei-dyshel
Copy link

I see. My problem was that I required it outside of user-config function. I still need to do my homework and learn how to write config for Spacemacs.

@bhipple
Copy link

bhipple commented May 28, 2017

Thank you Aaron and Valloric! I moved from Vim to Spacemacs about 6 months ago, and YouCompleteMe TAB key behavior was the one thing I've been missing. Countless times I'd be at the end of a line typing something like foo<CR>, and instead of getting foo\n I'd end up with foobar. I had tried a bunch of random elisp hacks to try to get company to behave the way I wanted but yours is the first I've found that "just works".

@nikital
Copy link
Contributor

nikital commented Aug 22, 2017

Hi everyone,

I implemented this completion style as a frontend that doesn't modify the buffer during completion and with minimal advice on the internal rendering code.

The frontend creates an overlay at point and updates it every time company selection is updated to make it look as if the text is already inserted into the buffer. It uses comany-selection-changed to know if the user took explicit action on the completion or if the tooltip is just showing up passively. When the frontend detects a command that isn't on company's keymap it completes the selection before the command executes. It also has evil mode integration for proper repeat support.

https://gist.github.com/nikital/0362abcaf003f2b81f0d0f9bda7be1f8

I hope that you will find this useful :)

@dgutov, what do you think about the gist? Would you consider this design clean enough to integrate into company-mode? If yes I can make it into a pull request and if no I'm open for critique.

@aaronjensen
Copy link
Contributor

@nikital nice! I definitely like the overlay. One of the things it's missing from my implementation is the ability to abort--tab and then shift-tab would result in nothing selected. Of course, that's where most of the advice I added comes in, so it may not be easy w/o more advice.

@nikital
Copy link
Contributor

nikital commented Aug 23, 2017

@aaronjensen If I understood the behavior you describe correctly, it is very easy to implement. I updated the gist with the new behavior. Now pressing shift-tab when standing on the first entry will deselect it.

https://gist.github.com/nikital/0362abcaf003f2b81f0d0f9bda7be1f8

The diff is pretty small and has no design changes: https://gist.github.com/nikital/0362abcaf003f2b81f0d0f9bda7be1f8/revisions?diff=split

@aaronjensen
Copy link
Contributor

@nikital great, that seems to work for me. I'll keep giving it a shot and let you know if I run into anything else. One thing is I believe this is required too:

(evil-declare-ignore-repeat 'company-select-previous-then-none)

@nikital
Copy link
Contributor

nikital commented Aug 26, 2017

@aaronjensen Oops, correct, fixed. Thanks for testing it out :)

@dgutov
Copy link
Member

dgutov commented Aug 27, 2017

@nikital It's a clean, clever hack, but the resulting frontend is not a "proper" frontend anyway, with how it's doing a lot of stuff other than visualization.

But I think you can release this as a separate package. I'll try not to break it unnecessarily.

@aaronjensen
Copy link
Contributor

@dgutov Out of curiosity, do you see a path to this being a core capability within company-mode? It's fine if not, it just seems there is a decent amount of interest in it and it would be nice for it to be supported w/o need for advice and as a "proper" frontend.

@dgutov
Copy link
Member

dgutov commented Aug 28, 2017

I'd like for it to be supported, but the way to do it is not obvious. Maybe some new extensibility surface, alongside backends and frontends? Nothing concrete.

As long as you have to rebind several key combinations yourself, as a user, it won't look like a "core capability" to me anyway.

I think we can do away with the necessity to use an advice, though, by using kill-local-variable to reset the company-selection value. This customization would then set it to nil globally.

@aaronjensen
Copy link
Contributor

Could it be a flag that changed the behavior of company-select-next and company-select-previous? It would also cause nothing to be selected by default. One question would still be what to do with RET, you wouldn't necessarily want to change the behavior of that command, so it'd probably require unbinding.

It does seem like it's still more of a "recipe" than anything, but I don't necessarily see anything wrong w/ that.

@dgutov
Copy link
Member

dgutov commented Aug 29, 2017

Maybe you're right. We certainly have some other optional commands that users can bind in the active keymap at their preference.

So I have two main concerns:

  • The frontend isn't "simply" a frontend, and having it in the core might encourage some developers to write some others like it. Perhaps we can avoid that simply by putting a comment in the code, however.

  • The commands only work when this frontend is used. To make this clearer, I think they should live together in a separate file. Hence my suggestion to release a new package. A new sub-package inside company-mode should work almost as well, but we need a name for it. company-insert-selected doesn't sound very distinguishing for me, and company-ycm is not appropriate either. Any suggestions?

@aaronjensen
Copy link
Contributor

The frontend isn't "simply" a frontend, and having it in the core might encourage some developers to write some others like it. Perhaps we can avoid that simply by putting a comment in the code, however.

Could you expand on this? From a purely usage standpoint, I see it as a version of company-preview-frontend that automatically does a company-complete-selection when you press any non-company key. Is it the lack of an company-complete-selection that gives you pause? IMO the frontend is the UI with which users interact. That's what I wanted to change when I opened this issue and the frontend seemed like the place to do it to me. Obviously you have something specific in mind though and/or you may be thinking about some particulars of the implementation that I'm missing.

The commands only work when this frontend is used. To make this clearer, I think they should live together in a separate file. Hence my suggestion to release a new package. A new sub-package inside company-mode should work almost as well, but we need a name for it. company-insert-selected doesn't sound very distinguishing for me, and company-ycm is not appropriate either. Any suggestions?

I'm totally fine with that. I think I'd prefer them to be a separate, cohesive group of things than having to add more codepaths to existing commands. Some name ideas:

  • company-lazy-complete
  • company-quick-complete
  • company-fast-complete
  • company-simple-complete
  • company-tab-and-go 😉

It's probably going to be hard to capture the behavior w/ the name, but I'm sure there are many out there better at naming than I.

@dgutov
Copy link
Member

dgutov commented Aug 29, 2017

IMO the frontend is the UI with which users interact. That's what I wanted to change when I opened this issue and the frontend seemed like the place to do it to me.

That's not how this term is used in this package. See the first sentence in company-frontends docstring. A frontend is a visualization. Of course, you can draw different lines and designate the thing before them as "frontend", like company-mode as a whole is a frontend for completion-at-point-functions.

Is it the lack of an company-complete-selection that gives you pause?

Rather, the fact that calling company-complete-selection from a frontend's pre-command action is not guaranteed to continue working well. It can break in the future, noticeably or somehow subtly. And it's probably incompatible with the default set of commands/bindings.

Advising company-fill-propertize is not great either, but there are ways to avoid that.

there are many out there better at naming than I

Let's wait for those people. 😃 company-tab-and-go isn't too bad, though. I was thinking of company-onlytab, which is basically the same. Maybe call it company-tng for brevity (so that the function names are shorter, too).

@LindyBalboa
Copy link

Just dropping in to say thanks to the guys working on this. It works nicely.

@habamax
Copy link

habamax commented Aug 29, 2017

Yep, thanks! YouCompleteMe style completion is what I did really miss in emacs.

@habamax
Copy link

habamax commented Aug 29, 2017

Let's wait for those people. 😃 company-tab-and-go isn't too bad, though. I was thinking of company-onlytab, which is basically the same. Maybe call it company-tng for brevity (so that the function names are shorter, too).

How about company-with-tab which could be Complete Anything With Tab?

Or company-tab-addict or company-tabber?

@dgutov
Copy link
Member

dgutov commented Aug 29, 2017

OK, I think any of these names is fine.

@nikital Will your sign the copyright assignment papers for Emacs? This is necessary for contributing to this package. The instructions are here: http://orgmode.org/cgit.cgi/org-mode.git/plain/request-assign-future.txt

@nikital
Copy link
Contributor

nikital commented Aug 29, 2017

Yes I can sign the copyright assignment, but first I want to spend a few more evenings to think about a better solution that won't involve calling company-complete-selection from a pre-command. If I won't come up with anything I will make the current package into a pull request.

@dgutov
Copy link
Member

dgutov commented Aug 29, 2017

I'm not sure this is the best use of your time, but please do as you see fit. There are options, like using pre-command-hook, or company-completion-cancelled-hook, but the best option will probably depend on whichever way this particular usage gets broken in the future.

Other things to consider:

  • Is company-insert-selected--complete-func variable really necessary? Most of the code in company runs in pre- and post-command-hook, and yet Evil seems to integrate with it fine.

  • Likewise, the other Evil integration can move to evil/evil-integration.el when this code is merged.

If I won't come up with anything I will make the current package into a pull request.

Please pick one the subpackage name, and put the code in a file with that name, as well as prefix the function names with it. These options sound best to me, unless you have a different idea:

company-tab-and-go
company-tng
company-tab-addict
company-tabber

Also note that I'll only merge the PR after the assignment is complete.

@nikital
Copy link
Contributor

nikital commented Aug 30, 2017

Is company-insert-selected--complete-func variable really necessary? Most of the code in company runs in pre- and post-command-hook, and yet Evil seems to integrate with it fine.

It's because of the order of the hooks.

Evil implements repeat by installing global evil-repeat-pre-hook and evil-repeat-post-hook which subscribe and unsubscribe to after-change-functions to monitor buffer changes. Company installs hooks as buffer-local hooks, which means that the order in which the hooks run is:

  • company-pre-command
  • evil-repeat-pre-hook
  • company-post-command
  • evil-repeat-post-hook

Currently the only code path inside Company that will trigger completion from a hook is company--continue (which can call company-complete-selection) and it's called from company-post-command. Since it runs between Evil's hooks, repeat works.

When we call company-complete-selection from company-pre-command, Evil's hooks didn't run yet and repeat breaks. I added manual calls to evil-repeat-pre-hook and evil-repeat-post-hook to address that.

@dgutov
Copy link
Member

dgutov commented Aug 31, 2017

So evil-repeat doesn't work with company-auto-complete either?

What if the code in your frontend added a new item to pre-command-hook, instead of having that code inside it? The pre-command action of the frontend might save some necessary information, and the new hook function would act on it.

@nikital
Copy link
Contributor

nikital commented Sep 1, 2017

So evil-repeat doesn't work with company-auto-complete either?

It does work, because company-auto-complete is reached from company-post-command. (And I explained in the previous comment why post-commands work)

What if the code in your frontend added a new item to pre-command-hook, instead of having that code inside it?

I don't like it because it would really complicate the code (with adding, removing and enforcing order of hooks), with the only benefit I see is easier Evil integration later on. If we want to make Evil integration easier, it's better to make it explicit. If we don't care about Evil integration, it's better to keep Company clean and separately figure out how to integrate repeat on Evil's side.

However, I came up with a way to avoid directly calling company-complete-selection from the pre-command hook:

  1. (company--unread-last-input)
  2. (setq this-command 'company-complete-selection)

The diff is here: https://gist.github.com/nikital/0362abcaf003f2b81f0d0f9bda7be1f8/revisions

The benefit of this approach is that it runs company-complete-selection through the entire Emacs command loop, which I believe lessens the chance of unrelated things breaking (for example, now Evil repeat works without further tweaks). I'm new to Emacs hacking so at first I was reluctant to change this-command from a pre-command hook, but after consulting the documentation it seems like a legitimate behavior:
https://www.gnu.org/software/emacs/manual/html_node/elisp/Command-Loop-Info.html#Command-Loop-Info

What do you think about this?

@dgutov
Copy link
Member

dgutov commented Sep 1, 2017

It does work, because company-auto-complete is reached from company-post-command.

I see, thanks. But! The idea behind company-auto-complete is very similar to what you are trying to achieve: as soon as the user types something that's not bound to a company-mode command, insert the current completion, and also allow the said command to work, of course. This works fine because it uses company-point to perform the insertion.

So I imagine your code could use post-command-hook in a similar manner. Like in the post-command section of the frontend.

Or even: couldn't you simply set company-auto-complete to an appropriate predicate, and let it do all the work? Try company-explicit-action-p as its value. The main difference will be that it won't finish completion as long as you are typing characters that are word or symbol constituents (or whatever the current backend thinks is continuing the current prefix).

If we don't care about Evil integration, it's better to keep Company clean and separately figure out how to integrate repeat on Evil's side.

Yes, but we'd want to make it possible at least, I believe.

I came up with a way to avoid directly calling company-complete-selection from the pre-command hook

Not sure I understand. Are you setting this-command in pre-command-hook with the expectation that the command loop will use the new value and call that command instead? If that works, and the other options are much harder to implement, fine, but I'm not sure it's guaranteed to work in future Emacs releases.

Does the documentation say anything about this particular usage? You are allowed to change this-command, but usually that doesn't change the actual command invoked (e.g. because when you change it the command has already been called).

@nikital
Copy link
Contributor

nikital commented Sep 2, 2017

couldn't you simply set company-auto-complete to an appropriate predicate, and let it do all the work?

Unfortunately, no. This would be the cleanest solution and it was the first thing I tried. But because company-auto-complete is called from a post-command it's executed after user's command has finished. This breaks some commands that modify the completed text. For example, we want the following to work:

;; | is point
;; Start typing
(this-command|)
;; company-complete-selected
(this-command-keys|)
;; backward-kill-word
(this-command-|)

If we call company-complete-selected from a post-command hook, we will get

;; | is point
;; Start typing
(this-command|)
;; backward-kill-word
(this-|)
;; company-complete-selected from post-command
(this-command-keys|)

Does the documentation say anything about this particular usage?

I couldn't find anything in the documentation about this pattern, but looking through Emacs' source code, it seems to be a known pattern:

@dgutov
Copy link
Member

dgutov commented Sep 2, 2017

Unfortunately, no. This would be the cleanest solution and it was the first thing I tried

OK.

looking through Emacs' source code, it seems to be a known pattern

Curious. It still might change (since it's undocumented), but the odds seem small. So go ahead.

@dgutov
Copy link
Member

dgutov commented Sep 24, 2017

Closing, having merged #706. If it's not enough somehow, please describe why.

@JonatanSahar
Copy link

Hi!
I see that Nikita's code has been merged into company, but how do you go about enabling it?
Thanks for all your work!

@JonatanSahar
Copy link

NVM, I figured it out.
I have a couple of related questions - not sure if this the appropriate place though.

  1. with the preview-frontend the overlay gets a different face which is very comfortable in my opinion. is there a way to replicate this with tng?
  2. I get the completed text inserted into the buffer in a way that takes point to the end of the completion, so that new characters are inserted after it. I thought the idea was for the completion to change as I insert more letters, but that's not the case for me as it is.

@JonatanSahar
Copy link

I'm sorry, I understand that this was meant to be used with the tooltip stytle of completion.
I guess what I was actually asking was if there's a way to integrate this with preview mode - such that the first completion is tentatively overlayed into the buffer, changing as you continue to insert text, then inserted, and cycle-able with TAB.

@dgutov
Copy link
Member

dgutov commented Apr 25, 2023

Hi!

I get the completed text inserted into the buffer in a way that takes point to the end of the completion, so that new characters are inserted after it. I thought the idea was for the completion to change as I insert more letters, but that's not the case for me as it is.

As soon as the completion text is "inserted", you don't get to type more. Any subsequent character keypress is taken to mean you are finished and typing code for after it is inserted. I believe that is the YouCompleteMe workflow anyway.

I guess what I was actually asking was if there's a way to integrate this with preview mode - such that the first completion is tentatively overlayed into the buffer, changing as you continue to insert text, then inserted, and cycle-able with TAB.

I guess you wanted something else than company-tng-mode, then? Have you tried just replacing company-preview-if-just-one-frontend with company-preview-frontend inside the value of company-frontends, keeping everything else the same?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants