Add company-cider backend #1910

alexander-yakushev · 2017-01-06T19:05:18Z

#1909 follow-up.

This patch adds company-cider backend that for the most part calls directly to the already existing CIDER functions. An update to docs was also made to describe how to enable the backend.

I've made a function that sets company-cider a sole completion backend. Is it OK? Maybe it is enough to drop company-capf from the list?

Regarding the docs, I also request comments. I took the liberty to drop the mentions of company-flx, but I feel there is a better way to explain the new setup.

bbatsov · 2017-01-06T19:21:47Z

I've made a function that sets company-cider a sole completion backend. Is it OK? Maybe it is enough to drop company-capf from the list?

I think that'd be better. Some people might be using a mixture of backends and this would affect them.

bbatsov · 2017-01-06T19:22:44Z

doc/code_completion.md

 (add-hook 'cider-mode-hook #'company-mode)
 ```

+To enable CIDER-specific company backend, add the following hooks:


a CIDER-specific

use the following code (you're actually adding hook functions to the hooks)

bbatsov · 2017-01-06T19:23:53Z

doc/code_completion.md

+(add-hook 'cider-mode-hook #'company-cider-make-exclusive)
+```
+
 When `company-mode` is thus enabled, it will receive completion information


I'd keep the original paragraph before this code and add some explanations about the different behaviour afterwards - with a link to "Fuzzy candidate matching" (or you can just move that section here).

bbatsov · 2017-01-06T19:24:17Z

cider-interaction.el

+(defun company-cider (command &optional arg &rest ignored)
+  "`company-mode' completion backend for CIDER."
+  (interactive (list 'interactive))
+  (cl-case command


Why not use pcase here?

Mindlessly copied from some other backend. Point taken.

bbatsov · 2017-01-06T19:25:05Z

Looks good to me overall. Apart from my small remarks you should mention this in the changelog.

alexander-yakushev · 2017-01-06T19:41:51Z

I would like anyone to try this out to verify it doesn't only work on my setup:).

Will rebase when ready.

bbatsov · 2017-01-06T20:04:15Z

OK, I'll try to source you some testers from Twitter as I'm short on time to testing myself right now.

xiongtx · 2017-01-06T20:06:05Z

If company-backends is turned into a buffer-local variable, wouldn't user modifications of it apply to only a single buffer? If the user tries to change company-backends in a CIDER buffer, it won't apply to other buffers, even other CIDER buffers.

Might a mode-local variable be a better idea? mode-local.el is a part of GNU Emacs.

alexander-yakushev · 2017-01-06T20:13:47Z

To be honest, I've stolen the idea of a buffer-local variable from company-eclim. If you say that it is better to make the variable mode-local, so be it.

alexander-yakushev · 2017-01-06T20:23:50Z

@xiongtx Is this what you have in mind?

dgutov · 2017-01-06T20:57:56Z

mode-local.el is a part of GNU Emacs

It's part of CEDET, and not used too widely.

On the other hand, changing the buffer-local value of company-backends is a perfectly common practice.

xiongtx · 2017-01-06T21:01:53Z

I'll defer to the author of company on how company-backends should be overridden 🙂 .

alexander-yakushev · 2017-01-06T21:08:39Z

Thanks for chiming in, Dmitry!

dgutov · 2017-01-06T22:50:21Z

Not a problem. But also see the last comment in the original issue. If what I'm suggesting is right, it should lead to a smaller, less disruptive change.

bbatsov · 2017-01-14T01:54:36Z

@alexander-yakushev Seems no one volunteered to test this, but we can merge this non-the-less. Squash and rebase and ping me when this is good to go.

julienfantin · 2017-01-21T05:55:33Z

doc/code_completion.md

-additional package [company-flx](https://github.com/PythonNut/company-flx).
-This is powered internally by [flx](https://github.com/lewang/flx).
+```el
+(add-hook 'cider-repl-mode-hook #'company-cider-enable-backend)


Should be cider-company-enable-backend

julienfantin · 2017-01-21T06:05:50Z

This is not working for me.

I tried the exclusive config to make sure nothing else was interfering in my setup but I did not get any completion results nor errors?

In GNU Emacs 26.0.50.2 (x86_64-apple-darwin15.6.0, NS appkit-1404.47 Version 10.11.6 (Build 15G1004))

Using latest packages on melpa

stardiviner · 2017-01-21T10:31:58Z

I have tested the code, it works. But will break the behavior of mixed backends users. Like grouped backends with company-yasnippet. So I suggest a better solution. Or just don't override the company-backends by default, let user decide. Like me, I will mix backend company-yasnippet with function:

(defun my-company-add-backend-locally (backend)
    "Add a backend in my custom way.

\(my-company-add-backend-locally 'company-robe\)
"
    (if (local-variable-if-set-p 'company-backends)
        (add-to-list 'company-backends `(,backend :with company-yasnippet))
      (add-to-list (make-local-variable 'company-backends)
                   `(,backend :with company-yasnippet))
      ))

alexander-yakushev · 2017-01-22T00:27:20Z

@stardiviner Apparently, @dgutov was correct after all (to nobody's surprise) suggesting to use a custom completion style rather than a custom backend. I have rewritten the PR. Could you please test if this solution doesn't break your setup?

@julienfantin Thanks for pointing out a mistake! Could you please try if the current version works for you?

dgutov · 2017-01-22T00:39:25Z

Sorry, right about what? I never said a custom backend won't work with fuzzy-matching. There are such third-party backends for other languages already.

I suggested the completion-styles approach because it takes less code, and because it might work with C-M-i as well. But nobody has tested that hypothesis, so far.

alexander-yakushev · 2017-01-22T00:52:35Z

Correct about it being a "better" way, if it actually will help stardiviner's usecase.

C-M-i seems to work fuzzily for me now.

dgutov · 2017-01-22T01:01:39Z

C-M-i seems to work fuzzily for me now.

Cool. This UI has a few peculiar behaviors, such as moving point to the position where disambiguation is required, which probably won't work with fuzzy. But still, having the basics work is a good result.

bbatsov · 2017-01-22T02:50:40Z

cider-interaction.el

+;; Fuzzy completion for company-mode
+
+(defun cider-company-compliment-candidates (string table predicate point)
+  "Return Compliment candidates as is, without filtering them."


Just one thing about the use of compliment in the names - this calls into the middleware which uses compliment for clj connections and cljs-tooling for cljs connections. Don't think cljs-tooling supports the fuzzy matching, so some mentions of this might be useful.

bbatsov · 2017-01-22T02:52:13Z

Looks OK to me. I wonder if we shouldn't move all the completion stuff to a dedicated file as there's already quite a lot of completions-related code.

bbatsov · 2017-01-22T02:53:14Z

cider-interaction.el

+               cider-company-compliment-candidates
+               "CIDER backend-driven completion style."))
+
+(defun cider-company-enable-fuzzy-completion ()


There should be a matching disable command I guess.

This is supposed to be put on a hook and enables itself per-buffer. Does it still warrant a disable?

Well, it's not a big deal but I can imagine some peoople will use this without a hook and it's generally a good idea to have a way to undo the effect of enabling this.

stardiviner · 2017-01-22T09:43:19Z

@alexander-yakushev It works with the completion-styles.

bbatsov · 2017-01-22T11:14:16Z

Guess this is in a good enough shape now. Thanks, @alexander-yakushev!

alexander-yakushev changed the title ~~Company backend~~ Add company-cider backend Jan 6, 2017

bbatsov reviewed Jan 6, 2017

View reviewed changes

julienfantin reviewed Jan 21, 2017

View reviewed changes

bbatsov reviewed Jan 22, 2017

View reviewed changes

alexander-yakushev added 2 commits January 22, 2017 12:33

Add fuzzy completion-style for company

3971561

Add info on custom completion style to docs

a8840d2

bbatsov merged commit 04e428b into clojure-emacs:master Jan 22, 2017

bbatsov mentioned this pull request Jan 22, 2017

Fuzzy completion woes #1909

Closed

Uh oh!

Add company-cider backend #1910

Add company-cider backend #1910

Uh oh!

Conversation

alexander-yakushev commented Jan 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bbatsov commented Jan 6, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bbatsov commented Jan 6, 2017

Uh oh!

alexander-yakushev commented Jan 6, 2017

Uh oh!

bbatsov commented Jan 6, 2017

Uh oh!

xiongtx commented Jan 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexander-yakushev commented Jan 6, 2017

Uh oh!

alexander-yakushev commented Jan 6, 2017

Uh oh!

dgutov commented Jan 6, 2017

Uh oh!

xiongtx commented Jan 6, 2017

Uh oh!

alexander-yakushev commented Jan 6, 2017

Uh oh!

dgutov commented Jan 6, 2017

Uh oh!

bbatsov commented Jan 14, 2017

Uh oh!

julienfantin Jan 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

julienfantin commented Jan 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stardiviner commented Jan 21, 2017

Uh oh!

alexander-yakushev commented Jan 22, 2017

Uh oh!

dgutov commented Jan 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexander-yakushev commented Jan 22, 2017

Uh oh!

dgutov commented Jan 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bbatsov commented Jan 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stardiviner commented Jan 22, 2017

Uh oh!

bbatsov commented Jan 22, 2017

Uh oh!

Uh oh!

alexander-yakushev commented Jan 6, 2017 •

edited

Loading

xiongtx commented Jan 6, 2017 •

edited

Loading

julienfantin Jan 21, 2017 •

edited

Loading

julienfantin commented Jan 21, 2017 •

edited

Loading

dgutov commented Jan 22, 2017 •

edited

Loading

dgutov commented Jan 22, 2017 •

edited

Loading