Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add import completion #32

Closed
dobegor opened this issue Feb 8, 2016 · 33 comments
Closed

Add import completion #32

dobegor opened this issue Feb 8, 2016 · 33 comments

Comments

@dobegor
Copy link

dobegor commented Feb 8, 2016

I wish there was import completion like in LiteIDE.
I.e. you typed:

import "fm

and it offers fmt package to import.

@nhooyr
Copy link
Contributor

nhooyr commented Feb 9, 2016

This is more of an issue with gocode. Also vim-go has the :GoImport command which offers autocomplete.

@dobegor
Copy link
Author

dobegor commented Feb 9, 2016

@nhooyr
LiteIDE has this feature without gocode, so I guess one can look at the implementation in it.
:GoImport isn't really useful when you don't know the head part of package's path.

@nhooyr
Copy link
Contributor

nhooyr commented Feb 9, 2016

@dobegor nsf/gocode#257

It would make much more sense to let gocode handle it than do it ourselves.

@nhooyr
Copy link
Contributor

nhooyr commented Feb 9, 2016

@dobegor what do you mean it isn't useful? it works perfectly fine for me.

@zchee
Copy link
Member

zchee commented Feb 9, 2016

@dobegor @nhooyr Hi,
I was also planned. but we have goimports, so was thinking whether we need.

However, see #19
If has been merged, the name of the Go stdlib can be retrieved.
and https://github.com/zchee/deoplete-jedi/blob/master/rplugin/python3/deoplete/sources/jedi.py#L41-L44

It is possible to display the package name if current input is import.

I think maybe performance reduced, but will not significantly affect because statically cache.

WDYT?

@zchee
Copy link
Member

zchee commented Feb 9, 2016

@dobegor @nhooyr or, If current input is import, call it.
https://gist.github.com/zchee/bdfe8b1967d3f958caee

It 's fmt, os, net, strconv, syscatll listed.

sorry.
I woke up earlier, so English is rough

@zchee
Copy link
Member

zchee commented Feb 9, 2016

@dobegor @nhooyr
I will explain in detail later...

@dobegor
Copy link
Author

dobegor commented Feb 9, 2016

@zchee looks great, good morning then :)
waiting for details on that feature.

@zchee
Copy link
Member

zchee commented Feb 9, 2016

@dobegor At first, I wrote the details of the flow.
Check it.
#19 (comment)

@zchee
Copy link
Member

zchee commented Feb 9, 2016

@dobegor At second, sloppy but...

    def gather_candidates(self, context):
        buf = self.vim.current.buffer
        pkgs = self.GetCurrentImportPackages(buf)
        parent = str(context['input']).strip('\t')
        pkg_data = os.path.join(self.data_directory, parent + 'json')

        if parent not in pkgs and '.' in parent and os.path.isfile(pkg_data):
            with open(pkg_data) as json_pkg_data:
                result = loads(json_pkg_data.read())

        elif re.match(r'^\s*import \w*', parent):
            # packages.json is https://gist.github.com/zchee/bdfe8b1967d3f958caee
            with open(os.path.join(self.data_directory,
                                   'packages.json')) as json_pkg_data:
                result = loads(json_pkg_data.read())

        else:
            line = self.vim.current.window.cursor[0]
            column = context['complete_position']

            offset = self.vim.call('line2byte', line) + \
                charpos2bytepos(self.vim, context['input'][: column],
                                column) - 1
            source = '\n'.join(buf).encode()

            .
            .
            .

Currently, conditions only parent is import.
If the newly created re.match(), performance is reduced.
so I'm going to fix the self.GetCurrentImportPackages(buf) method.
e.g. (arg name is rough)

self.GetCurrentImportPackages(buf, resultIsPackageNameOrJsonData?)

@zchee
Copy link
Member

zchee commented Feb 9, 2016

@dobegor May I ask if you understand what I mean?

@nhooyr We can be acquired without execute a gocode, It is whether to adopt this.

@nhooyr
Copy link
Contributor

nhooyr commented Feb 10, 2016

@zchee i really think we shouldn't bother, much rather let gocode. Its not really a urgent feature, goimports works perfectly fine. Its autocomplete is nice. If you really want to, go ahead, there is no real harm. It might add some needless complexity though.

@zchee
Copy link
Member

zchee commented Feb 10, 2016

@nhooyr Yes.
It's possible. but it is also complicated.
I think it is the work of goimports. so I do not like the implements of this.

@dobegor WDYT?

@dobegor
Copy link
Author

dobegor commented Feb 10, 2016

@zchee so this will only allow to auto-complete a fixed set of import paths?
I guess we need to make a way to generate package.json for all packages in $GOPATH, for example, (imports-cache) command. It will parse $GOPATH and generate import paths for each package, then autocompletion will perform fast fuzzy matching along this list.

I have literally 2 days experience with Vim, but I can certainly write such utility in Golang (using third-party lib for fast fuzzy matching).

@nhooyr
Copy link
Contributor

nhooyr commented Feb 10, 2016

@dobegor why don't you just modify gocode to do this. That way every single editor that uses gocode will be able to take advantage of that functionality without changing anything.

@zchee
Copy link
Member

zchee commented Feb 10, 2016

@dobegor

I guess we need to make a way to generate package.json for all packages in $GOPATH, for example, (imports-cache) command. It will parse $GOPATH and generate import paths for each package, then autocompletion will perform fast fuzzy matching along this list.
I have literally 2 days experience with Vim, but I can certainly write such utility in Golang (using third-party lib for fast fuzzy matching).

Now, gocode are looking for collaborators nsf/gocode#307.

If you said implements are realized, and faster than gocode, We will consider adoption it.
but at present, bad idea not to use the gocode.


I'm sorry if my understanding is wrong.

@nhooyr
Copy link
Contributor

nhooyr commented Feb 13, 2016

@zchee I'm gonna take a stab at implementing it in gocode. I'll update once I have it done.

@zchee
Copy link
Member

zchee commented Feb 14, 2016

@nhooyr I looking forward update.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 8, 2016

@zchee nsf/gocode#332

@zchee
Copy link
Member

zchee commented Mar 8, 2016

@nhooyr Thanks!
I'll check that PR later.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 8, 2016

Once its merged (likely tomorrow) we just need to update our regex for completion in strings, as well as add a new class import. Thats it.

@zchee
Copy link
Member

zchee commented Mar 8, 2016

@nhooyr I got it.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 9, 2016

Its been merged!

@zchee
Copy link
Member

zchee commented Mar 9, 2016

@nhooyr Yeah!!!!
I'll update and test :)

we just need to update our regex for completion in strings, as well as add a new class import.

I would like to confirm, but it meaning is input_pattern?

@nhooyr
Copy link
Contributor

nhooyr commented Mar 9, 2016

yes.

@zchee
Copy link
Member

zchee commented Mar 9, 2016

@nhooyr OK.

I will update the input pattern corresponding to it. (Maybe tomorrow...)

@zchee
Copy link
Member

zchee commented Mar 11, 2016

@nhooyr I test import completion feature.
Not required modifying input_pattern. It worked perfectly :)

I small fixed for when align_class is enabled.

@dobegor This feature has been the implementation.
Close :)

@zchee zchee closed this as completed Mar 11, 2016
@nhooyr
Copy link
Contributor

nhooyr commented Mar 11, 2016

@zchee https://asciinema.org/a/3xz29k70w2zmi27sg2zugde74

just a slight problem.

@zchee
Copy link
Member

zchee commented Mar 11, 2016

@nhooyr Oh, Good catch. Thanks.

Actually, it seemed necessary fixes for self.input_pattern :(
(It is very difficult to understand (?:\b[^\W\d]\w*|[\]\)])\.(?:[^\W\d]\w*)? regex... 😱)

@nhooyr
Copy link
Contributor

nhooyr commented Mar 11, 2016

haha. take a look at the previous PRs, its explained there
#35
#36

@zchee
Copy link
Member

zchee commented Mar 11, 2016

@nhooyr Yea.
When to implementing this feature, because I thought it was necessary to change the input_pattern, I had to try it. About 30 minutes...
However, was not able to completely understand 😂

I will try again from now... (I do not know how long it takes. lol)

FYI, I separated this problem #46

@zchee
Copy link
Member

zchee commented Mar 11, 2016

@nhooyr
Off topic: Could you tell me your twitter ID?
My TW: https://twitter.com/_zchee_

@nhooyr
Copy link
Contributor

nhooyr commented Mar 11, 2016

@zchee its the same, nhooyr. I'll change it if you want, I think I know whats wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants