Skip to content
This repository has been archived by the owner on Jul 27, 2018. It is now read-only.

TODO: Auto calculate startcol via cm_refresh_patterns #30

Closed
roxma opened this issue Feb 28, 2017 · 17 comments
Closed

TODO: Auto calculate startcol via cm_refresh_patterns #30

roxma opened this issue Feb 28, 2017 · 17 comments

Comments

@roxma
Copy link
Owner

roxma commented Feb 28, 2017

This is the cm_refresh_pattern of bufkeyword currently:

cm_refresh_patterns=[r'[0-9a-zA-Z_#]{4,}$'],

The framework should be able to calculate startcol using the last matching group of the pattern, if the pattern is changed into this:

cm_refresh_patterns=[r'([0-9a-zA-Z_#]{4,})$'],

Props:

Cons:

  • Less readable code?
@prabirshrestha
Copy link
Contributor

It would be interesting to see how VS code and language server protocol implements this

@roxma
Copy link
Owner Author

roxma commented Feb 28, 2017

Yeah, It seems there's no concept of startcol in the language server protocol.

@prabirshrestha
Copy link
Contributor

LSP has textDocument/completion and a way to cancel a request. It would be interesting to see how often textDocument/didChange and textDocument/completion is called when typing in vscode.
For some reason I think having patterns to trigger autocomplete is wrong. The autocomplete provider should do the calculation all the time. In visual studio and vscode I can type Ctrl+<space> to force show me suggestions. This also means the completion provider should never check for the length of keywords.

@roxma
Copy link
Owner Author

roxma commented Mar 1, 2017

IMO, the pattern is for the framework figuring out when to notify the source and then get the popup.

It is useful, and should be configurable. For example, there are users who want longer typing to trigger completion popup, to keep themselves from distraction.

In visual studio and vscode I can type Ctrl+<space> to force show me suggestions. This also means the completion provider should never check for the length of keywords.

It is also possible for NCM to have Ctrl+<space> like mapping, and I've been preparing for this kind of feature, as long as the source itself, which is the provider, does not filter the notification by patterns.

roxma pushed a commit that referenced this issue Mar 1, 2017
roxma pushed a commit to roxma/nvim-cm-tern that referenced this issue Mar 1, 2017
@roxma roxma closed this as completed Mar 1, 2017
roxma pushed a commit that referenced this issue Mar 1, 2017
@prabirshrestha
Copy link
Contributor

VsCode completion is very fast even though it is running js. So I created a vscode extension and played around with how it works.

export function activate(context: vscode.ExtensionContext) {
    const PLAIN_TEXT: vscode.DocumentFilter = { language: '*', scheme: 'untitled' };

    context.subscriptions.push(vscode.languages.registerCompletionItemProvider(PLAIN_TEXT, new MyCompletionItemProvider(), 'c'));
}

export class MyCompletionItemProvider implements vscode.CompletionItemProvider {
    provideCompletionItems(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken): Thenable<vscode.CompletionList> {
        return new Promise<vscode.CompletionList>(resolve => {
            const items: vscode.CompletionItem[] = [
                new vscode.CompletionItem("test"),
                new vscode.CompletionItem("test2"),
                new vscode.CompletionItem("abcdef"),
                new vscode.CompletionItem("abcdefgh"),
            ];
            resolve(new vscode.CompletionList(items, false));
        });
    }
}

registerCompletionItemProvider has variable args. 'c' is a trigger character.

I put a breakpoint at resolve() to see when the provideCompletionItems function is called. provideCompletionItems seems to be called only during start of a word (a word is specific to a language which in this case in cm_refresh_patterns) or during triggerCharacters or when <ctrl+space> is triggered.

https://github.com/Microsoft/vscode/blob/2540cbb603f25e5a8f92c8d0657646c77540dfef/src/vs/editor/common/model/wordHelper.ts. There is a default word pattern so the completion provider doesn't need to specify it. go completion for vscode seems to do extra computation before actually asking the server https://github.com/Microsoft/vscode-go/blob/798da91e2708df4bb56d6dda764515682a2e51e3/src/goSuggest.ts#L51

@roxma
Copy link
Owner Author

roxma commented Mar 1, 2017

@prabirshrestha
Thanks for the explanation.

I'll consider adding default word pattern.

VIM has \k pattern, which uses the iskeyword option, but it is vim regex instead of PCRE.

It would still be nice to reuse this option. Then buffer keyword completion would be more sensible for the buffer.

@roxma roxma reopened this Mar 1, 2017
@roxma
Copy link
Owner Author

roxma commented Mar 1, 2017

I put a breakpoint at resolve() to see when the provideCompletionItems function is called. provideCompletionItems seems to be called only during start of a word

So this should be the reason for being very fast? By caching the completion results early (the start of a word) before displaying the popup menu (after specific amout of characters being typed)?

@prabirshrestha
Copy link
Contributor

If you have code installed creating an extension and debugging should be very easy. They recently officialy added yeomen generator so creating extension is very easy. https://code.visualstudio.com/Docs/extensions/example-hello-world

Seems like they cache at start of the word before displaying the popupmenu but doesn't wait for specific amount of characters being typed or wait for a timer.

Here are the things I tried and noticed.

  • open a blank file (nothing happens here since the user hasn't started typing)
  • type the first character a. It will call provideCompletionItems() because it is the start of the word. Then keep typing bcde nothing happens. It doesn't matter whether you type the character faster or slower. Even if you type each character with 5 sec difference slowly the provideCompletionItems was only called when a was typed. If you type space and then f, f will trigger provideCompletionItems since it is start of a new word. New word is not just space it can be other characters like in vscode. So if I type ghi nothing will happen. Then if I type .j, j will trigger since . starts a new word. Backspace also doesn't call provideCompletionItems() since the startcol remained the same. <ctrl-space> is the only one besides this that forces completion. This means timers are never used. We could even port async completion to vim 7 if we wanted. provideCompletionItems() does take the cancellation token.

Each completion provider can provide optional addition list of trigger characters. Theoretically this same could be used instead of \k if converted to PCRE. Here is the snippet form the above vscode link.

export const USUAL_WORD_SEPARATORS = '`~!@#$%^&*()-=+[{]}\\|;:\'",.<>/?';

/**
* Create a word definition regular expression based on default word separators.
* Optionally provide allowed separators that should be included in words.
*
* The default would look like this:
* /(-?\d*\.\d\w*)|([^\`\~\!\@\#\$\%\^\&\*\(\)\-\=\+\[\{\]\}\\\|\;\:\'\"\,\.\<\>\/\?\s]+)/g
*/
function createWordRegExp(allowInWords: string = ''): RegExp {
   var usualSeparators = USUAL_WORD_SEPARATORS;
   var source = '(-?\\d*\\.\\d\\w*)|([^';
   for (var i = 0; i < usualSeparators.length; i++) {
   	if (allowInWords.indexOf(usualSeparators[i]) >= 0) {
   		continue;
   	}
   	source += '\\' + usualSeparators[i];
   }
   source += '\\s]+)';
   return new RegExp(source, 'g');
}

// catches numbers (including floating numbers) in the first group, and alphanum in the second
export const DEFAULT_WORD_REGEXP = createWordRegExp();

Provide completion items can return Thenable<vscode.CompletionList> instead of just an array of CompletionItem[]. This is where it becomes interesting. This means the language server protocol can return the result and say the list is incomplete. If it is incomplete and the user types a character it will call provideCompletionItems(). Most of the time I have seen this to be false like in the above case so everything is cached.

/**
    * Represents a collection of [completion items](#CompletionItem) to be presented
    * in the editor.
    */
export class CompletionList {

    /**
        * This list it not complete. Further typing should result in recomputing
        * this list.
        */
    isIncomplete: boolean;

    /**
        * The completion items.
        */
    items: CompletionItem[];
}

Now this finally makes sense to me why LSP interface is the way it is.

Here are other interesting tricks to make it fast that are not related to startcol.

  • As soon as the buffer is loaded, the language server should start and warm up the cache. Don't wait for users to type. Here is one link: make sendCommand public runoshun/tscompletejob#1 (comment). This way by the time you actually start writing your server will return results fast.
    This is what the typescript server returns. Notice that the results are not sorted. That is exactly what the typescript server returns but the selected menu is what matches the written code. Then I think it uses ranking and weight to figure out the best matches for future but this is per code session so exiting and opening again will reset the preference.

image

Seems like the function should be changed to something like call cm#complete(a:opt, a:ctx, l:startcol, l:matches, true) where the last option isIncompete and can even remove l:startcol and store that in core internally instead, but that could be breaking change.

@roxma
Copy link
Owner Author

roxma commented Mar 1, 2017

Seems like the function should be changed to something like call cm#complete(a:opt, a:ctx, l:startcol, l:matches, true) where the last option isIncompete and can even remove l:startcol and store that in core internally instead, but that could be breaking change.

I've already changed the function parameters in order to work with LSP server, https://github.com/roxma/nvim-completion-manager/blob/master/doc/nvim-completion-manager.txt#L261

The startcol should stay. In this way, cm#complete() is more like an extended version of vim's builtin complete() function.

roxma pushed a commit that referenced this issue Mar 1, 2017
@roxma
Copy link
Owner Author

roxma commented Mar 1, 2017

default_word_pattern is already implemented here.

Other optimizations for speed are omitted currently for simplicity.

@roxma
Copy link
Owner Author

roxma commented Mar 2, 2017

Early caching for candidates has been implemented now.

Warm up at startup does not seems to be NCM's duty, I think I should leave it to the source implementation.

The NCM python source framework can receive buffer events so it should be able to implement it easilly.

@roxma
Copy link
Owner Author

roxma commented Mar 3, 2017

Related startcol issue autozimu/LanguageClient-neovim#15 (comment)

The default word pattern by NCM does not work for php language server, since PHP variable start's with $

@roxma roxma reopened this Mar 3, 2017
@prabirshrestha
Copy link
Contributor

prabirshrestha commented Mar 3, 2017

looking at the source of language provider I see $ as a trigger character.

$serverCapabilities->completionProvider->triggerCharacters = ['$', '>'];

Once the server is initialized and you have to add these two characters as part of trigger and it should work.
Ideally ncm sources should only be registered once the initialization is complete.

@roxma
Copy link
Owner Author

roxma commented Mar 3, 2017

@prabirshrestha Thanks for the information!

autozimu pushed a commit to autozimu/LanguageClient-neovim that referenced this issue Mar 3, 2017
@bounceme
Copy link

bounceme commented Mar 5, 2017

#30 (comment)

couldn't the regex be changed to just splitting on not alnum ? I don't know the spec for what is in the list though

@roxma
Copy link
Owner Author

roxma commented Mar 5, 2017

I'm wondering on this too. Maybe it takes care of unicode character better.

@bounceme
Copy link

bounceme commented Mar 5, 2017

in vimregex :
[^[:lower:][:upper:][:alnum:]_] plus any other keyword characters, or the other way around [[:punct:]]

roxma added a commit that referenced this issue Mar 6, 2017
…ased on the last matching group of cm_refresh_patterns. It could easily lead to confusion. #30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants