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

Support static json file caching #19

Merged
merged 2 commits into from
May 26, 2016
Merged

Support static json file caching #19

merged 2 commits into from
May 26, 2016

Conversation

zchee
Copy link
Member

@zchee zchee commented Feb 7, 2016

Avoid the execute gocode during this condition

Conditions

  • Hook the insert of dot(.)
  • Typed the package has not yet been imported use context['input']
  • Match the typed package name and json file name

Flow

1. Parse current import package

pkgs = self.GetCurrentImportPackages(buf)
.
.
def GetCurrentImportPackages(self, buf):
    pkgs = []
    start = 0
    for line, b in enumerate(buf):
        if re.match(r'^\s*import \w*|^\s*import \(', b):
            start = line
        elif re.match(r'\)', b):
            break
        elif line > start:
            pkgs.append(str(b).strip('\t'))
    return pkgs

2. Check current input

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.strip('.') not in pkgs and '.' in parent and os.path.isfile(pkg_data):
  • Contain your typed package in the list of 1. Parse current import package
  • Contain a dot (.) on your type
  • Match package name and json file name

3. If...else and return

If all true on 2. Check current input, return the static caching json data.

Avoid the execute gocode. So can be fast response.

.
.
with open(pkg_data) as json_pkg_data:
    result = loads(json_pkg_data.read())

else, exec the gocode, and return result.


Currently not yet tested.

@zchee zchee added the WIP label Feb 7, 2016
@zchee zchee force-pushed the json-caching branch 4 times, most recently from 8159343 to 8755547 Compare February 7, 2016 01:06
@fatih
Copy link

fatih commented Feb 7, 2016

I think I need is. deoplete-go is to slow :/ When I press fmt. I wait for 1-2 seconds. It was very fast previously. Did you changed something?

@zchee
Copy link
Member Author

zchee commented Feb 7, 2016

@fatih 1-2sec !?
It is strange is something... :(

I test now, return 0.3-0.5...

@zchee
Copy link
Member Author

zchee commented Feb 8, 2016

@fatih Statically caching for Go stdlib packages.
and It will also be completion func of the package that has not yet been imported into the buffer.
Also, Users can add any packages json data.
It is being tested.

If you test it, Move data/json to ~/.config/gocode/json.

BTW, Does not yet on/off setting value. but I will add. Don't worry :)

@nhooyr
Copy link
Contributor

nhooyr commented Feb 8, 2016

@fatih Mine is blazing fast.

I'll test and look at this tomorrow. Gonna do some other work now.

@zchee
Copy link
Member Author

zchee commented Feb 8, 2016

FYI, test/fmt.go has not e.g. unsafe, but typed unsafe and dot, It will be dipslay unsafe functions.

@nhooyr
Copy link
Contributor

nhooyr commented Feb 8, 2016

@zchee sorry not sure what you said there. What do you mean?

@zchee
Copy link
Member Author

zchee commented Feb 8, 2016

@nhooyr Open https://github.com/zchee/deoplete-go/blob/master/test/fmt.go, type unsafe..
Will displayed unsafe func.

@nhooyr
Copy link
Contributor

nhooyr commented Feb 8, 2016

Oh i see, interesting. Well I'll take a look at the rest tomorrow, bye.

@zchee
Copy link
Member Author

zchee commented Feb 8, 2016

We do not need goimports before getting completion word.

but it will be displayed in comments fmt.Printf(" | ") (maybe not related this PR).
and the name of the Go stdlib package there is those that are duplicated.

Fix this problem and need gocode format json data generator.
After they are completed, I want to merge.

@fatih
Copy link

fatih commented Feb 8, 2016

Also, Users can add any packages json data.

Hi @zchee, Is it possible to make caching automatically? And when is cache invalidated? For example If I add my own package and I modify the package after 2 days and add new identifiers into it, what will happen to the cache?

@zchee
Copy link
Member Author

zchee commented Feb 8, 2016

@fatih Not automatically. and I will only update commit the json for each version up of Go.
Users can add any package json data implementation is a by-product. So user will have to manage on their own.

And when is cache invalidated?

If you typed package has been import in the buffer, cache invalidated.

For example If I add my own package and I modify the package after 2 days and add new identifiers into it, what will happen to the cache?

If we create the wrapper(or decomposition) of gocode for generate json and watch the go get command script, so your example will be realized.
but, that is too much workload for deoplete-go...
It may be better managed by such vim-go...?

@svanharmelen
Copy link
Contributor

@zchee for me it's also really really fast!

@fatih did you also try it with a minimal nvim.init to check if the delay is related to a specific combination of plugins and/or custom settings?

@zchee zchee mentioned this pull request Feb 9, 2016
@zchee zchee force-pushed the json-caching branch 5 times, most recently from 2531196 to 7b05831 Compare February 9, 2016 22:40
@zchee zchee mentioned this pull request Feb 9, 2016
@zchee zchee changed the title Add static json file caching Support static json file caching Feb 26, 2016
@zchee zchee force-pushed the json-caching branch 12 times, most recently from e9b81f5 to 78e5da3 Compare April 9, 2016 00:29
@zchee
Copy link
Member Author

zchee commented Apr 9, 2016

@nhooyr @fatih @svanharmelen Hi,
I created json generator. It outputs a gocode syntax's json using the you use current go version.

Also included Go v1.6 darwin|linux_amd64 json data.
darwin is created by use Homebrewed Go 1.6.
linux is created by Docker golang:1.6.0-alpine container.

I usually use devel, but API is expected to change frequently, it did not included.

How to install:

If you have a time, Could you test it? 🙏
and If you found a bug, Please feel free to post comments.

If exists any strings before the current cursor, cache completion does not work.
It's known bug. will fix.
e.g.

package main

import (
    "fmt"
)

func main() {
    fmt.Println("test")

    // os package does not import
    // and exists string before cursor
    fmt.Println(os.|)

    // syscall package does not import, also not exists string before cursor
    // will be cache completion works fine
    syscall.|
}

I'm advance next plan. cgo completion use libclang-ptyhon3.
https://github.com/zchee/deoplete-go/tree/cgo-complete
So, I want to merge this pull request!

If no reply, It will be merged after a while. (maybe 2, 3 days)

Thanks!

@zchee
Copy link
Member Author

zchee commented Apr 9, 2016

also, now cgo_completeworks. Let's try edit ./tests/cgo.go.
If you type C., will be return <stdlib.h> C completion word list.
but need libclang.dylib(or .so), Please install it.

@svanharmelen
Copy link
Contributor

@zchee just played a little with this and it seems to work fine.

But I must admit I don't really notice much difference (if any) in speed as my completion was already very fast without the caching. So is speed the only reason for the caching, or does it fix any another use cases?

As for the usability, you currently have to do a few manual steps in order to get things setup. If using a newer or older Go version, you would first have to run make gen_json, then create the cache dir ~/.cache/gocode/json and copy over the generated JSON files.

Wouldn't it be an idea to not ship the pre-generated JSON files with the deoplete-go package and add a vim command to create the cache dir and generate the JSON files in that cache dir? :DeopleteGoCreateCache or something like that? And maybe a second one called :DeopleteGoUpdateCache for when you updated your local Go version.

I guess that would be a much better user experience as setting up stuff (or updating afterwards) will only be 1 vim command and you don't have to add and maintain all those version specific JSON files inside the deoplete-go repo...

@zchee
Copy link
Member Author

zchee commented Apr 11, 2016

@svanharmelen

But I must admit I don't really notice much difference (if any) in speed as my completion was already very fast without the caching. So is speed the only reason for the caching, or does it fix any another use cases?

Yes, fast result is important, but this branch's goal is not the fast result.
I don't know how to tell you because I'm not good at English.... but, the goal is "get completion list without import package current buffer".
The fast result is a byproduct.

OK, I'm consider :DeopleteGoCreateCache and :DeopleteGoUpdateCache commands.
Thanks review!!

@zchee zchee force-pushed the json-caching branch 3 times, most recently from 21b3659 to c51d16a Compare April 14, 2016 23:52
Avoid the call gocode during this condition

- Conditions
  - Hook the insert of dot(.)
  - Typed the package has not yet been imported use context['input']
  - Match the typed package name and json file name

Signed-off-by: Koichi Shiraishi <[email protected]>
@zchee zchee force-pushed the json-caching branch 4 times, most recently from c4850f3 to eef6f7b Compare May 26, 2016 12:58
- Add parses library name
  -  e.g. 'go/ast' is 'go'
  - If imported 'github.com/pkg/errors', ignore cache for 'errors'

- Change json cache files
  - 'go/ast' is '/path/to/go/ast.json'
  - 'errors' is '/path/to/errors/errors.json'

Signed-off-by: Koichi Shiraishi <[email protected]>
@zchee
Copy link
Member Author

zchee commented May 26, 2016

Now fixed the wrong completion list if duplicate package name (e.g. github.com/pkg/errors and stdlib errors)
:DeopleteGoCreateCache and :DeopleteGoUpdateCache commnad is will implements later.

Maybe no bug.
Merged.

@zchee zchee merged commit 22927e4 into master May 26, 2016
@zchee zchee removed the WIP label May 26, 2016
@zchee zchee deleted the json-caching branch October 14, 2016 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants