Sarcasm / company-irony

company-mode completion back-end for irony-mode
118 stars 11 forks source link

Pass control to other company backends when inside string or comment #35

Closed ghost closed 7 years ago

ghost commented 7 years ago

Hey there,

first of all, thank you for providing irony. Awesome work.

I noticed, that company-irony will never return nil to the prefix command, even if point is in a comment or string literal. This blocks other company backends that might provide useful completion candidates in such contexts, e.g. company-files, company-ispell or company-dabbrev.

Of course, one can work around this issue by grouping these backends with company-irony but then one might get unreasonable completion candidates outside strings or comments.

Therefore, I'd like to propose for company-irony to pass control to other backends, when in string literals or comments.

Sarcasm commented 7 years ago

company-irony-prefix returns stop indeed. What backend do you have in mind? I guess this can be made configurable, either stop or nil.

ghost commented 7 years ago

Making it configurable would be very nice. Then one would be able to combine this with a suitably populated company-backends variable to get access to other backend's completion candidates in strings or comments.

As I said, the first backends that came to mind are company-ispell, company-dabbrev and 'company-files'. company-ispell would be useful of course to assist with spelling in comments or strings. company-files might be useful when one has to hard code file paths in strings. company-dabbrev might be useful in a variety of cases, e.g. for documentation purposes or if you want to have access to any type of information in another buffer. These were the first use cases that came to mind.

Sarcasm commented 7 years ago

Committed as 55052b3bb4ef878732136f58c34403219f99b68d.

I kept the current behavior because company-backends documentation says:

The function should return ‘stop’ if it should complete but cannot (e.g. if it is in the middle of a string).

Also, I think I like this behavior, but maybe I will try the new behavior as well.

To use this setting, put this in your Emacs config file:

(setq company-irony-prefix-use-stop nil)

Or use the customize interface: M-x customize-variable RET company-irony-prefix-use-stop RET.

dgutov commented 7 years ago

because company-backends documentation says ...

I'm sorry to say it's not a great advice. Most of the built-in backends go against it. See company-clang or company-dabbrev-code.

It would be more accurate to say something like this:

should return `stop' if it should complete but cannot (e.g. if point is in the middle of a symbol).

On the other hand, saying only this, without any further explanation, might seem puzzling to the new users. It will still be better than being inaccurate, though.

I'll push the change soon, but comments welcome.

dgutov commented 7 years ago

About the variable, I don't think company-irony-prefix-use-stop is a great idea (or the default t), because the user could emulate the value t, if they so choose, by making a buffer-local company-backends value where company-irony is the last element.

But it's up to you, of course.

Sarcasm commented 7 years ago

Ok, thanks for the feedback. I committed f68c1b46d64b9f95cfc2a6d611089d0442ed58ba which just return nil when company-irony does not find the prefix. Is it worth it to return stop when in the middle of a symbol though?

dgutov commented 7 years ago

I say yes, but it's not trivial to explain.

It's a design decision until company-mode knows how to sanely behave when completing inside symbols.

Sarcasm commented 7 years ago

Ok, thanks for the feedback, I understand the issue, I now return stop in this situation.