RFC: use REPL.REPLCompletions as completions provider instead/in addition to current method

[orialb]

I have been playing around with replacing the current completion mechanism with the built-in REPL.REPLCompletions . (I thought about doing so after seeing this closed PR in julia-emacs).

My motivation to try this came from two problems I saw with the current method:

1. It seems to suggest a lot of completions which are not really available in the current scope. For example for prin with the current method I get a long list of suggestions with most of them irrelevant (unavailable in current scope) such as print_array, print_matrix, print_quoted,... (those seem to be unexported methods from Base).

2. Another problem is that the current method might not be able to provide completion when some macros are involved. Admittingly I have only one example, but it is a macro I use all the time so I want to make it work :) Considering the code below. With the current method, the completion I get for MyS is @pack_MyStruct, @unpack_MyStruct.

using Parameters
@with_kw struct MyStruct
    x
end

Both of these issues seem to be solved when using REPL.REPLCompletions as the completions provider. But I guess they could also be solved within the current implementation (at least issue 1, not sure about issue 2).

A disadvantage of REPLCompletions is that it doesn't allow fuzzy search. So maybe it is also possible to generate a combined completion list using the current method and REPL.REPLCompletions if fuzzy search is desired.

Let me know what you think about this suggestion.

[gcv]

Interesting!

Since I want to keep fuzzy completion, I took a quick look at the problems you discovered.

For point 1, I think fixing the problem of out-of-scope suggestions is just a matter of changing all=true to all=false in the call to Main.JuliaSnail.lsnames in julia-snail--completions-base (and possibly in julia-snail--completions-core).

For point 2, the completion system explicitly filtered out DataType names (i.e., all structs). That happened while I was chasing down other problems and I never removed the filter.

I pushed a branch with changes: https://github.com/gcv/julia-snail/tree/completion-fixes

Could you please test it and see if it works for you? While you test, please try other completion situations. I want to make sure this doesn't break anything I haven't found in my own testing.

That said, even if it works and solves your problem, I really like your idea of integrating the "official" REPL completion into Snail as a fallback, and your approach seems to work pretty well in my testing. An untested idea about how to make a combined completion system:

• Take your implementation of julia-snail-completion-at-point and call it julia-snail-completion-at-point-from-repl.
• Add (and remove) julia-snail-completion-at-point-from-repl to completion-at-point-functions in the julia-snail-mode definition, so they are active alongside the current julia-snail-completion-at-point.

Let's give this a try, and if it works, I'll merge your PR.

[orialb]

Thanks for checking this out and providing the quick fix!

I confirmed that it indeed fixes the two problems I mentioned above. I will use the fix branch for a day or two and see if there are any problems.

I found two new cases where REPLCompletions provides completion and the current method doesn't, so I think it could be a good idea to add it as a fallback to cover more cases. (1) Latex symbols \alp -> \alpha (this is useful as they are replaced with unicode symbols) (2) Module.f -> Module.func

It's probably a good idea to wait until after https://github.com/gcv/julia-snail/tree/completion-fixes is merged and then I can update this PR as you suggested and test how it works.

[gcv]

@christopher-dG: Would you mind doing a bit of testing with the https://github.com/gcv/julia-snail/tree/completion-fixes branch? More eyes on it would help. I want to make sure it doesn't break completion in some other way.

[christopher-dG]

Sure thing!

[christopher-dG]

The completion-fixes branch does make using unqualified names a lot better! I thought that there was a regression in that completions for qualified modules (e.g. Base.UV_) weren't working, but turns out that they don't work on the master branch either. I get completions for Base., but if I add anything after that I get nothing.

[orialb]

I get completions for Base., but if I add anything after that I get nothing.

I think completions for qualified names inside a module work when REPLCompletions is used as the completion provider, i.e. using the code in this PR. At least from my limited testing it seemed to be working properly. In case it is hard to get this working with the current method, this is another reason we should add REPLCompletions as an additional completion provider :)

[gcv]

Could both of you please test a fix I tried to the qualified module problem? https://github.com/gcv/julia-snail/commit/fb99546d1a580d54be31a372dda7dd57b5890ea3

It still doesn't load completions for Base.UV_, not sure why, and don't have time to dig into it right now. Let's just make sure the completion-fixes branch is in good shape, merge it in, and focus on this PR (the REPLCompletions one) since it should make these minor things go away.

Ideally, please run with completion-fixes for a little while so we can be sure I didn't break anything.

[gcv]

I just merged the completion-fixes branch into master. @orialb, feel free to update this PR when you're ready.

[orialb]

Great, I've been using the completion-fixes branch for the last several days and didn't see any special problems. I'll try to update this PR at some point during this week.

[orialb]

I updated the PR, probably will need some testing that it doesn't break anything. I did a few small checks, but I will have to use it for a couple of days.

With this PR now Base.UV_ completion works. In addition it has the advantage that it can provide completion for qualified symbols when a module alias is used (I think this didn't work before when only the current method was used, but maybe I'm wrong).

Two more things I would like to improve on:

1. currently if point is at \alph , julia-snail--identifier-at-point will not detect the \ so it will pass only "alph" to the completion function. It would be nice to have a version of this which will detect \ so we can get completion \alph -> \alpha .
2. It should be possible to also get path completion when typing inside a string, e.g. "/home/user/my_" -> "/home/user/my_dir" because REPLCompletions can identify this and provide the possible paths. This still doesn't work in Snail, because julia-snail--identifier-at-point returns also the closing quotes doesn't capture the quotes, so I will have to look into this.

[orialb]

(I guess that the two features mentioned in the previous comment could be also implemented without relying on REPLCompletions or the Snail server at all)

[dahtah]

Note that there's an effort underway to support completion of unicode symbols in julia-emacs: https://github.com/JuliaEditorSupport/julia-emacs/pull/100 Maybe worth coordinating so that the two completion modes don't interfere

[gcv]

They should not interfere as long as no one uses exclusive completion-at-point mode (which the other patch does not, after this PR, we won't be :)). If I understand the other thread correctly, the julia-mode conversion for \alpha to α with TAB will still work, but will also allow completion for anyone who wants the \alpha literal, which we will also do. I don't think it'll be a problem if two separate completion-at-point sources provide the same suggestion.

[orialb]

OK, I managed to get path completion working. Latex completion is still not working for some reason (although Snail sends the correct string to the server), so I still have to figure it out (and once I do that I'll also do some squashing to get rid of all the WIP commits).

[gcv]

I'm really swamped right now, so can't look at your changes in great detail, but for \ detection, what about extending thing-at-point for Julia symbol entries to include \? You just have to add it to julia-snail--with-syntax-table. It might break something else, though, so needs testing.

[orialb]

Thanks for the tip, I appreciate you taking the time to reply. I actually already managed to make Snail send "\\alp" to JuliaSnail.replcompletion when point is at \alp (but maybe the solution you are suggesting is more performant than how I implemented it). If I manually run the call that is sent to the server I see that one gets back "\\alpha" in the completion list, so maybe the problem is that company is filtering this out for some reason. Anyway I'll keep investigating this until I figure it out.

[gcv]

In my testing, the suggestion I gave you to modify the syntax table doesn't work. thing-at-pt still strips out the backslash. 😕

I guess the fix is to hack julia-snail--identifier-at-point and julia-snail--identifier-at-point-bounds to do the right thing, i.e., detect backslashes and include them. Maybe use (thing-at-point 'symbol t), check the preceding character of the match, and if it's a backslash include it. It's kinda ugly though.

[orialb]

In my testing, the suggestion I gave you to modify the syntax table doesn't work.

That's odd isn't it? it works for me and I would have thought this would be a quite robust functionality? In any case I can add the solution you suggested, this is indeed how I did it in the code before the syntax-table suggestion.

I've been thinking recently, wouldn't it actually make more sense to unite the different completion mechanisms on the server side in such a way that only one request is sent to the server per completion? For example, I've seen some slowness with completion when working over SSH with my experimental setup and I was wondering maybe it is worth reducing the number of requests for performance reasons in general? Or is the bottleneck somewhere else anyway? Related to this I've found FuzzyCompletions.jl which is the fuzzy completions provider for Juno as far as I understand, and I think it does unite all functionalities that we want. Maybe worth taking a look into this at some point (package seems to be still quite new).

(Oh, and sorry for all the stray parentheses)

[gcv]

I bet the SSH slowness you observe is streaming large completion lists from lsnames to Emacs, since it filters client-side. Not sure what to do about that, I guess we could look at the FuzzyCompletions thing you found down the line..?

Another possible slowdown reason: when I look at the process buffer, it seems that completions get requested twice. I haven't figured this out, and it's not supposed to happen with completion-table-dynamic. Maybe we aren't using it correctly.

I've been poking at the syntax table \ nonsense, I think I have it figured out. Stand by.

[gcv]

The syntax table ?\\ trick doesn't work because julia-mode changes the meaning of ?\\ on the fly in julia-syntax-propertize-function. If you have julia-mode with commit https://github.com/JuliaEditorSupport/julia-emacs/commit/c9d8230b1ff73f663d975e83a5cf5acdcc3b6255 or later, julia-snail--identifier-at-point does the wrong thing. Then if you run (setq-local syntax-propertize-function nil) in that buffer, julia-snail--identifier-at-point will do the right thing — but only on text you type after changing syntax-propertize-function.

Makes me wonder if using the syntax table and thing-at-point is a good idea for julia-snail--identifier-at-point at all. It's pretty fragile. In any case, this means that your latest commit is the correct implementation. Apologies for wasting your time on this.

The latest code in this PR works pretty well for me. Do you want to make any further changes, or should I merge it?

[orialb]

The latest code in this PR works pretty well for me. Do you want to make any further changes, or should I merge it?

Maybe last thing is that currently path completion will not work on Windows because of the way I choose the bounds that I pass to company (i.e. part after the last "/" so it filters the completion list properly). But I'm not sure windows is even supported by Snail in general (due to libvterm) so maybe this is not so important?

bet the SSH slowness you observe is streaming large completion lists from lsnames to Emacs, since it filters client-side. Not sure what to do about that, I guess we could look at the FuzzyCompletions thing you found down the line..?

There is already a builtin function REPL.fuzzyscore which I think can be used to do some filtering on the lsnameslist on the server side. But we can discuss this later after I make some progress/initial code suggestion towards adding SSH support to Snail.

[gcv]

REPL.fuzzyscore is interesting. It's not useful by itself, but maybe the REPL.fuzzysort built on top of it can be useful to only send like the top 30 entries on any match. I gave a quick try to integrate it, but it doesn't seem to make things faster, and responses coming from the server don't look right in the process buffer. This is a good direction for future improvements, though. Feel free to take a look: https://github.com/gcv/julia-snail/tree/experimental-fuzzysort

For Windows support: yeah, libvterm basically means Windows does not work. Anyway, Emacs on Windows supports both forward and back slashes as directory separators, so I think whatever we do has to support both flexibly. Seems like too much work to do right now for no benefit. Just put a TODO next to the split-on setting and let's move on.

[orialb]

Ok, added the TODO comment. This is ready to merge from my side.

I'll have a look at the fuzzy stuff, thanks for trying that.