syl20bnr / spacemacs

A community-driven Emacs distribution - The best editor is neither Emacs nor Vim, it's Emacs *and* Vim!
http://spacemacs.org
GNU General Public License v3.0
23.69k stars 4.9k forks source link

Proper way to set up company-mode backends #924

Closed trishume closed 9 years ago

trishume commented 9 years ago

The company-mode layer refactor in b1adef06263e75249a9ed704ba72865649f33adb introduced a new method for layers to add company mode integration. This new method involves use of a macro, which clears company-backends buffer locally and adds just the specific backend for that mode.

However company-mode already has logic for choosing and activating the correct backend for the buffer from company-backends. This system is pretty cool and dynamic and allows cool things like automatically choosing ycmd over company-clang if you have it installed, without having to remove any previous hooks and things.

One disadvantage is It requires more code to implement and that code is not the same as the normal method of adding company backends that package Readme's will tell layer authors to use. See diff of #916

The new method is okay though, I guess I would be perfectly happy with it if I had better justification for its goodness. I'm creating this issue to discuss the relative merits of both systems. It would be great if @syl20bnr could provide examples of the cases he thought the new system might be better in.

syl20bnr commented 9 years ago

This is more a matter of design choice and flexibility than anything else.

In a personal configuration the auto-selection of relevant backends is very handy, in a distribution like Spacemacs I want full control over them to tailor them for each major mode.

It is some sort of ODT layer where we can cherry-pick the relevant fields for a domain. I guess that the most important of all is to be able to easily control the order of the backends. I prefer to unknot very small balls than a big aggregate of all of them.

I also think that an application wide company-backend does not scale, not in time complexity, but in dependency complexity.

To support the refactoring there is a the documentation of the yasnippet backend [1] and also a paragraph in the emacs wiki [2].

In the end we have a little more work to do in the configuration but I think it worth it.

[1] https://github.com/company-mode/company-mode/blob/master/company-yasnippet.el#L63-81 [2] http://www.emacswiki.org/emacs/CompanyMode (section Issues & Questions)

trishume commented 9 years ago

I'm not sure this really addresses those problems. The yasnippet problem is not solved by this, it is based on the orthogonal fact that company selects one backend at a time unless you group them. Either way you have to use backend-with-yas. The emacs wiki section skips the fact that the company-backends list is ordered and thus the example problem it gives is solved by default by the fact that company-dabbrev is after all the language-specific backends.

In terms of not scaling with dependency complexity, I think I need an example to understand what you are getting at.

The one specific example I can think of where this approach works better is when in general company-dabbrev is activated before/instead of company-ispell but you wan't ispell in markdown mode. In this case you want to add company-ispell buffer locally in the markdown mode hook. However, this can be done on a case-by-case basis when needed and I think it's better not to complicate everything else for this rare case.

For your example of ordering, I don't think hooks solve this. When you cons onto company-backends you have whichever one is consed on last taking precedence. So if you have two python backends the later loaded one wins. To control ordering you can just use hooks to cons it on after another package loads. With hooks you have both packages hooking on and trying to set the backends to only them, how do you decide which one wins? This is something admittedly the global list does poorly, but I don't think hooks do any better.

syl20bnr commented 9 years ago

In this case you want to add company-ispell buffer locally in the markdown mode hook.

Right, so I'm for a generalisation of this.

For your example of ordering, I don't think hooks solve this. When you cons onto company-backends you have whichever one is consed on last taking precedence.

Right this is why I wrote that I prefer to unknot small balls of whool than a unique big one. In the scope of a layer it will be easy to correctly construct the list.

syl20bnr commented 9 years ago

To resume my feeling about this, major-mode layer specific backends offer a better separation of concerns.

Edit: well we may have several major-modes per layer, but you get the idea :-)

syl20bnr commented 9 years ago

I forgot one important thing, kind of. It is never all black or all white. It it makes sense we can include default backends in the macro that make company-backends buffer local and reset it.

trishume commented 9 years ago

I now understand the value of having the macro around for rare cases like the markdown example. However, I don't think it should be used as a default practice, it works just as well or better if everywhere else uses the normal way and the override is only used when strictly necessary.

Kinda like you were saying about never all black and white but just a different method. Layers use the normal method in the 90% of cases where it is easier and does the same thing. In the rare case the specific override is used.

In the scope of a layer it will be easy to correctly construct the list

This is already easy and the new method is just more code. The backends already know which mode you want them for so if in the go layer you cons on company-go it will be used with 100% certainty in exactly the same situations as the hook would use it.

The only scenario where the two methods function any differently is when there is a backend that by default supports multiple modes (like company-ispell) but you only want it to activate in one mode.

What I'm getting at is that the fact that company-backends is ordered already gives you layer specific backends. It is only rare cases (none exist yet in Spacemacs) where this doesn't work. I don't understand why layers like go and python should use this method.

syl20bnr commented 9 years ago

I will put it another way.

Imagine that I designed the dotfile of spacemacs to contain a list of all layers and tell the users to give them a nil or t value to ignore or use each of them. Now tell me which layers you are effectively using. Is it easy to do so ? If yes, will it be always true as we add layers ? Is it easy to maintain ? Why the user should care about all the layers ?

In the backends scope, why all the backends should be visible from a major-mode ? How do I know which backends are effectively used ?

Instead of being passive and wait for a bug in a major-mode which will compel us to make the company-backends variable buffer local, I prefer to embrace from the beginning the clean way to do it and make it easy to use. I'm sure you can see the flexibility it gives us in a context of a layered design where each layer is isolated from the others as much as possible.

trishume commented 9 years ago

These are all very good points, but there are other ways of doing these things that use the existing company infrastructure. The key fact is that company-backends is both ordered and only activates one element of the list at a time.

Your example of wanting to know what backend is used is a good one. It's important to know for debugging purposes. The thing is part of what makes company-mode awesome is you can have company-c-headers in include tags, file completion in strings, c completion in code. This means as you say you can't just see what backend is active in a buffer.

However, at the time of completion company only has one backend active and you can query it. You just have to start typing until the completion box pops up then M-x eval-expression RET company-backend. Tada! And this gives you better debugging info than a small buffer local list would since it tells you exactly what Company is using. You do have to know about this variable, but you have to know about the company-backends variable in the other scenario anyway.

Also I agree that layer design would be a horrible design, but Company isn't like that. It's an ordered list of human-readable symbols that you can read. The difference with your example is that as I mentioned Company only has one backend active at a time unlike the many layers. Just read down the list and see which one would apply first.

Buffer locals with hooks also doesn't make it so that "each layer is isolated", it works the same way as the list (since the list is ordered and only one thing is activated). In fact it might be harder to debug with hooks.

For example: Suppose you have both company-semantic and company-ycmd in separate layers. Which one is activated? With buffer locals you open a C++ buffer and look at company-backends and you see only the hook that beat the other one based on the order they were added. With a global you eval the list and see if company-semantic or company-ycmd is first.

So far the same, but it gets better: What if the company-ycmd won (it's later in alphabetical loading) but the user copied their dotfiles to a VPS without the ycmd server? With hooks they just don't get completion in C++, with a global list the ycmd backend fails over gracefully to company-semantic.

Also with the current hook macro if some company user wants to add include file completion for example, they would have to explicitly add it to both the c++ layer and the ycmd layer, or refactor some abstraction. With the global list, it applies when needed with only adding it in one place, even if the user has added their own company-irony private layer for c++ completion.

syl20bnr commented 9 years ago

Your point are valid and thank you for the explanation. This is what I understood about the company design so there is no misunderstanding here.

But I cannot understand how you can say that it is better to pick the backend in a unique list containing all the backends of the world. It makes no sense for me, especially when order matters in a lazy loaded environment. Reducing the scope can only be a good thing, in some use case we even have no choice which is a good hint about he limit of the convenience introduced by the unique list.

The order issue is no different in both approaches and will provoke headaches, this is intrinsic to the design of company. But I prefer to think about this issue in a restricted environment. If I'm configuring the backends for doing C++ why should I care about the other backends because I have to care about them, see the warning all over the place about the blocking backends.

For the duplication of hooks it is resolved with a function accepting several hooks, and C and C++ is an edge case anyway.

For the user it does not change a lot, it is a matter of wrapping the hook in a function.

What other property that I like about the cherry-picking approach is that the default value of company-backends is not altered. So we can just use this default value unaltered in some modes with no specific backends.

trishume commented 9 years ago

I guess in forming my perspective I'm thinking more of pro and con examples of the outcomes of each solution. I guess I haven't been swayed by the architectural appeals because I don't see the reason behind them.

You have tried to give me these reasons but I just don't find the provided ones solid enough. For example:

syl20bnr commented 9 years ago

The flaw in the design is here, no matter how you try to minimize it, there are backends which can block the other because they are almost always relevant, this is a guaranteed source of headache, maybe not so much in a private configuration but certainly more likely in a big configuration like spacemacs. It is not a coincidence that the first question in the emacs wiki is about separation of concern and address this specific blind spot in the backends election. There is no "spacemacs specific" things to google about this flaw. And using the default method will still work so I don't see how the user can be confused and need to google something, at best he is modifying the global backend list free of all the other mode specific backends.

The best way to create a layer in spacemacs is to read how the others are done, the more layers we have, the more consistent they are, and the easiest is this process. See the PR for the go layer: I did not have to say how to add properly the backend to get a PR (even if the error was about the lazy loading of company and not about the way we manage the backends).

syl20bnr commented 9 years ago

Oh by the way, the cherry-picking allows to lazy-load company as well.

Edit: No scratch that, the error was that it was added in a :init instead of a :config.

trishume commented 9 years ago

Ok thanks a lot. That's good enough reasoning for me, especially now that I know there is a specific measurable benefit (lazy loading).

One thing I do suggest though is that the macro be better than the existing one, it shouldn't take 6 lines (#916) to add a company backend in the proper way. How about one macro/function that takes a mode and a backend list as a parameter? I can submit a PR (probably will have time tomorrow) to implement this and switch my layers (ycmd, auctex) to this method.

Thanks for discussing this.