aggressive-indent-mode
aggressive-indent-mode copied to clipboard
Malabarba
Prevents company from completing in certain situations
Matching issue over at company-mode: https://github.com/company-mode/company-mode/issues/406
Tl;dr: When I remove aggressive-indent--keep-track-of-changes from after-change-functions I get back completions.
What if you keep the after change function but remove the post command hook instead? Does that also fix it?
Have you tried changing the order of the hooks? I.e. Ensure that the Aggressive-indent functions come before company or vice versa.
Moving aggressive-indent first in the post-command-hook also fixes the issue. That was somewhat unexpected, because it means the list is traversed in reverse order.
@dgutov Would it be possible for company to use the append argument when adding its function to post-command-hook?
That sounds like the simplest way to ensure it always works with other functions that might be added to the hook.
The docstring doesn't say anything about the order in which the functions in the post-command-hook are consumed, so this might be not be a very robust solution. I obviously assumed in-order traversal, but it turned out be reverse-order which makes me suspect that this has changed at least once already.
I obviously assumed in-order traversal, but it turned out be reverse-order which makes me suspect that this has changed at least once already.
This seemed so weird to me that I had to test it to be sure, it is indeed traversed in-order. My bad.
Yes, I would think the traversal order would be kept stable.
Still, regardless of the order, the fact is that company-mode's functions rely on being at the end of the hook.
Would it be possible for company to use the append argument when adding its function to post-command-hook?
That would interact badly with e.g. flyspell-post-command-hook. Flyspell uses (sit-for flyspell-delay). As a result, the Company popup will work with a huge delay.
I see.
Well, I just played around and I can reproduce the issue too.
As you've gathered, @expez, company-post-command needs to come after aggressive-indent--indent-if-changed.
I'd love to add some code to aggressive-indent to make that work. But there's no easy way to do this from aggressive-indent because the problem only happens when company-mode is activated after aggressive-indent-mode.
It's easy for the user to fix (just make sure the modes are activated in the right order), but I'd like to find a way to make it work without th user's intervention.
@dgutov how important is this test, (eq tick (buffer-chars-modified-tick))? If you cancel the company timer every time a new one is started is there a consequence to removing this check?
but I'd like to find a way to make it work without th user's intervention
While you're thinking on that, I suppose clear instructions on enabling global-aggressive-indent-mode after global-company-mode might be sufficient enough.
how important is this test (eq tick (buffer-chars-modified-tick))?
It checks that the user hasn't done anything to the buffer contents in the time between the time we've sent the request for completions and received the result, that would invalidate the result. Comparing buffer-chars-modified-tick is the easiest check to make, though I suppose comparing (buffer-substring-no-properties (point-min) (point)) might work almost as well. Or, as a performance optimization, maybe compare not the whole contents but like 1000 characters in both directions.
I don't see obvious drawbacks in that plan that are likely to manifest in practice, but it still feels like a kludge, to be honest. Care to suggest anything else?
...unless the issue is that you don't see completions when agressive-indent-mode performs some actual modifications to the buffer contents (the fact that they're whitespace-only changes seems hard to detect). In that case including the characters after point in the comparison won't work.
Can we be sure that in Clojure, at least, completions don't depend on the file contents following the current position?
It checks that the user hasn't done anything to the buffer contents in the time between the time we've sent the request for completions and received the result, that would invalidate the result.
Yes, this much is obvious. But if the user has kept typing then that would have triggered more completion requests right? So if each completion request invalidates the previous requests, then we don't need to look at the buffer's state at all, right? All we have to do is make sure only the most recent request is shown. Am I missing something?
Can we be sure that in Clojure, at least, completions don't depend on the file contents following the current position?
(let [foo 1]
fo|)
Gives me foo as a candidate but
(let [foo 1]
fo|
does not.
I think there are other examples e.g the contextual completions available in the ns macro. An argument can be made that this is a bug, but given how everyone uses either paredit or smartparens to ensure the latter example never occurs, and that getting at the context is made quite a bit harder by being taxed with balancing the toplevel form containing point before analysis, I don't think it's worth the effort to 'fix' this.
unless the issue is that you don't see completions when agressive-indent-mode performs some actual modifications to the buffer contents
Yes, this is when it happens for me.
Gives me foo as a candidate but
(let [foo 1] fo|
does not.
Here's another example, with all parens balanced, and in elisp. This offers me no completions.
(compa| 1
2)
@expez
So if each completion request invalidates the previous requests
Currently that "invalidation" happens only via comparing the value of point and buffer-chars-modified-tick.
All we have to do is make sure only the most recent request is shown.
That can still lead to wrong completions being displayed. Example: you typed (fo, the asynchronous request for completions was sent, you've backspaced over o and typed a, and just now the response with completions has arrived. It's entirely plausible that no next completion request has been sent in the meantime, and then that response will consider itself to be up to date if we only compare the value of point.
An argument can be made that this is a bug... I don't think it's worth the effort to 'fix' this.
It does look like a bug. But I was thinking more of an example like this (add a type annotation if needed):
(let [s "abc"]
(.| s))
Even if Compliment doesn't look at the text after point currently, it might very well do so in the future.
Here's another example, with all parens balanced, and in elisp.
@Malabarba Oh. So that happens even with synchronous backends. I suppose that makes sense. Somehow, I figured that CIDER switched to asynchronous since the last time I looked.
But here the comparison is in company-idle-begin, and it checks that the buffer hasn't been modified since the idle timer was scheduled. Which is probably not as important than with asynchronous completion since we re-create the timer after each command, but still: some code could have changed the buffer contents, likewise from post-command-hook or from a timer.