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

package_bin: Fix callback arg to package path instead of alias name #357

Merged
merged 1 commit into from Jun 9, 2016

Conversation

zchee
Copy link
Contributor

@zchee zchee commented Jun 9, 2016

Fix callback arg to package path.
This fix will resolve that issue.
#352

I don't know whether this pull request is correct. If the wrong fix, please point out.

Also, there might be a problem because I tested only a little.
but _testing/all.bash test is all passed.

  • p.callback(parent.path, tdecl)
    • for decl_var.
  • p.callback(parent.path, &ast.FuncDecl{
    • for decl_func.

Test sample:

package main

import (
    "go/parser"
    "go/token"
    "io/ioutil"
    "log"
)

func main() {
    buf, err := ioutil.ReadFile("hello.go")
    if err != nil {
        log.Fatal(err)
    }

    fset := token.NewFileSet()
    f, err := parser.ParseFile(fset, "hello.go", buf, parser.Mode(0))
    if err != nil {
        log.Fatal(err)
    }

    f.
}

byte offset: 348
cmd: gocode --in ./parser.go autocomplete 348

package main

import "github.com/derekparker/delve/service/rpc2"

const addr = "localhost:41222" // d:4 l:12 v:22

func pointerStruct() {
    client := rpc2.NewClient(addr)
    bp, _ := client.GetBreakpoint(-1)

    bp.
}

byte offset: 210
cmd: gocode --in ./delve.go autocomplete 210

Signed-off-by: Koichi Shiraishi <k@zchee.io>
@nsf
Copy link
Owner

nsf commented Jun 9, 2016

Yeah, I wish I knew. I will take a look at it this weekend for sure, but for now I'll just pull it.

@nsf nsf merged commit bb0c614 into nsf:master Jun 9, 2016
@zchee
Copy link
Contributor Author

zchee commented Jun 9, 2016

@nsf Thanks!! I learned a lot from that.
Thanks for the great tool.

@nsf
Copy link
Owner

nsf commented Jun 12, 2016

Well, I took a look at parsed output of both Go 1.6 (text) and Go 1.7 (bin) data. Yeah, seems like your patch fixes it. Ideally I guess I need to make sure that both parsers generate exactly the same output if that's possible. But I'll leave for another time for now. Thanks for the fix.

@zchee
Copy link
Contributor Author

zchee commented Jun 12, 2016

@nsf Hi,

I took a look at parsed output of both Go 1.6 (text) and Go 1.7 (bin) data. Yeah, seems like your patch fixes it.

Thanks review.
and yes, It seems to could not be parsed package because gc_bin_parser callback is not full path.

Ideally I guess I need to make sure that both parsers generate exactly the same output if that's possible.

Sorry, Is it meaning adding some of the tests? or another meaning?
If the meaning is test, I can write the test.
If other than that, I will implement it. (In both cases, I might not be able to live up to your demands)

@nsf
Copy link
Owner

nsf commented Jun 12, 2016

Don't worry it's all great, you don't have to do anything.

@zchee
Copy link
Contributor Author

zchee commented Jun 13, 2016

@nsf understand. Thanks :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants