Pass through package names, use it for godoc in emacs-company #463

antifuchs · 2017-08-16T05:58:16Z

This pull request adds a few fields to the decl, candidate, scope and package_import structures, populated with the import path for the package that each symbol is defined in. This makes it possible to later (in emacs's company-go package) automatically perform a documentation lookup for each symbol being completed.

I believe this PR addresses #415 (it builds on the sample code that @nikclayton kindly provided in this issue). Here's a screenshot of how it looks in practice:

I hope you find this change acceptable - I ran the tests, and the same tests fail as on master (3); hope that's OK too! And of course, please let me know if there is anything you would like to see improved. I'll be happy to fix this if it means I can get automatic docs in emacs (-:

This should let us retain each package's import path, potentially useful for attaching to the symbols we emit (definitely useful for gocode further down the line).

This makes JSON and CSV formatters emit the import path for the package defining the symbol being completed. This should help tools like company get documentation for symbols, as gocode expects `importpath.symbol` as input.

Using the package information the 4th field in CSV output, call `godoc` on the symbol being completed. This activates the automatic documentation functionality for go in company.

lloiser · 2017-08-16T07:41:40Z

autocompletecontext.go

 			}
 		}
 	}
 	// propose all children of its embedded types
-	b.append_embedded(cc.partial, cc.decl, class)
+	pkgname := c.pcache[cc.decl.name].import_name


It almost always panics here (for me)

Thanks for pointing these two out! I'll guard against that, but just to make sure I test with the right thing, do you have a source file that triggers the panic?

gocode itself in various places.
e.g. in here if you try decl.

gocode/autocompletecontext.go

Lines 68 to 86 in f1eef9a

func (b *out_buffers) append_decl(p, name string, decl *decl, class decl_class) {

c1 := !g_config.ProposeBuiltins && decl.scope == g_universe_scope && decl.name != "Error"

c2 := class != decl_invalid && decl.class != class

c3 := class == decl_invalid && !has_prefix(name, p, b.ignorecase)

c4 := !decl.matches()

c5 := !check_type_expr(decl.typ)

if c1 || c2 || c3 || c4 || c5 {

return

}

decl.pretty_print_type(b.tmpbuf, b.canonical_aliases)

b.candidates = append(b.candidates, candidate{

Name: name,

Type: b.tmpbuf.String(),

Class: decl.class,

})

b.tmpbuf.Reset()

}

So I guess it's happening whenever you try to access variables or functions within the same package.

Yup, I just managed to reproduce it with that, too. Thanks!

lloiser · 2017-08-16T07:41:47Z

autocompletecontext.go

 	}
 	// propose all children of an underlying struct/interface type
 	adecl := advance_to_struct_or_interface(cc.decl)
 	if adecl != nil && adecl != cc.decl {
 		for _, decl := range adecl.children {
 			if decl.class == decl_var {
-				b.append_decl(cc.partial, decl.name, decl, class)
+				pkgname := c.pcache[decl.name].import_name


this could panic too

There were some places that could have been nil, and the pcache might just not contain an entry either. This change should guard against these cases & use a default (no package) instead.

antifuchs · 2017-08-16T14:58:17Z

The update I just pushed should relieve the problem with panics when completing a symbol in the current package, but it doesn't yet show the right package in that case.

lloiser · 2017-08-16T15:17:52Z

Would be great if we could get the package for those functions, variables, etc. too.

antifuchs · 2017-08-16T15:25:42Z

Agreed! I'll take a peek at this later today (:

nsf · 2017-08-16T19:05:33Z

I'm ok with the changes overall, but also I think we should keep backwards compatibility in output formats if possible. It's ok to make a new output format if necessary. But for people who never update their packages in emacs, etc. it should just keep working.

asf-stripe · 2017-08-16T21:37:52Z

That's a great point, @nsf, and I agree! I hope the format I choose for the csv and json formatters works for the packages that I know about - the company-go package I have installed does it right already (splits the csv formatter's result, uses field names by number); is there a scenario you had in mind where that wouldn't work?

(Edit: hah, I used my work account to post this by accident! Won't delete the comment, sorry about any confusion about identity)

nsf · 2017-08-16T21:46:32Z

I worry about people who won't update the company-go.el. But I haven't looked at the code in details yet. Will do so on the weekend.

* Add the -u flag (customizably) to the `go doc` command line: This ensures that un-exported/private names get matched & have their docs displayed, too. * Add a company-go customization option for the `go doc` command line. * Make sure that if company-go gets no package, it looks in the current directory. This can more reliably show documentation for the current package's names (including internals).

antifuchs · 2017-08-17T06:44:37Z

@lloiser - I just pushed an update for the elisp portion that allows it to handle unexported names, and will look them up in the current package (identified by an absence of an import path); this seems to have been all that was needed to get reliable docs display there - please update this if you find any more places where things break!

nsf · 2017-08-20T17:29:49Z

Sorry guys, will have to take a look at it next weekend probably. Or maybe I'll do so during the week sometime.

Emacs-company plugin is switched to "csv-with-package" format. Adding one field to "csv" formatter breaks local sublime plugin. It's python there, args spread goes into a function with 3 arguments, it says you give it 4 and throws an exception. Fucking fuck. I said - backwards compatibility, please. See: #463

nsf · 2017-08-25T16:48:24Z

And I was right about backwards compatibility. Adding another field to "csv" format breaks sublime plugin I use. I renamed the format with 4 fields to "csv-with-package", yes it's ugly. Old "csv" format returns as usual 3 field. Company-go plugin is switched to use "csv-with-package".

Sorry it took so long to merge the thing.

P.S. Thanks for the patch.

asf-stripe · 2017-08-25T17:02:06Z

Aaah, good catch! Thanks for merging this & fixing it up!

antifuchs added 3 commits August 14, 2017 22:58

Rename "path" to "abspath", add a relative import path

65402ba

This should let us retain each package's import path, potentially useful for attaching to the symbols we emit (definitely useful for gocode further down the line).

Integrate godoc lookup into company-go

7729b36

Using the package information the 4th field in CSV output, call `godoc` on the symbol being completed. This activates the automatic documentation functionality for go in company.

antifuchs force-pushed the emit-package-name-and-importpath branch from f56b417 to 7729b36 Compare August 16, 2017 06:01

This was referenced Aug 16, 2017

Godoc not appearing in code-completion microsoft/vscode-go#194

Closed

VSCode style tooltip joefitzgerald/go-plus#542

Closed

lloiser reviewed Aug 16, 2017

View reviewed changes

Guard against packages not present on the cache

8aef1e9

There were some places that could have been nil, and the pcache might just not contain an entry either. This change should guard against these cases & use a default (no package) instead.

lloiser mentioned this pull request Aug 16, 2017

WIP: show descriptions in autocomplete suggestions joefitzgerald/go-plus#678

Closed

nsf merged commit abcc7f1 into nsf:master Aug 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass through package names, use it for godoc in emacs-company #463

Pass through package names, use it for godoc in emacs-company #463

antifuchs commented Aug 16, 2017

lloiser Aug 16, 2017

antifuchs Aug 16, 2017

lloiser Aug 16, 2017

antifuchs Aug 16, 2017

lloiser Aug 16, 2017

antifuchs commented Aug 16, 2017

lloiser commented Aug 16, 2017

antifuchs commented Aug 16, 2017

nsf commented Aug 16, 2017

asf-stripe commented Aug 16, 2017 •

edited

Loading

nsf commented Aug 16, 2017

antifuchs commented Aug 17, 2017

nsf commented Aug 20, 2017

nsf commented Aug 25, 2017 •

edited

Loading

asf-stripe commented Aug 25, 2017

	func (b out_buffers) append_decl(p, name string, decl decl, class decl_class) {
	c1 := !g_config.ProposeBuiltins && decl.scope == g_universe_scope && decl.name != "Error"
	c2 := class != decl_invalid && decl.class != class
	c3 := class == decl_invalid && !has_prefix(name, p, b.ignorecase)
	c4 := !decl.matches()
	c5 := !check_type_expr(decl.typ)

	if c1 \|\| c2 \|\| c3 \|\| c4 \|\| c5 {
	return
	}

	decl.pretty_print_type(b.tmpbuf, b.canonical_aliases)
	b.candidates = append(b.candidates, candidate{
	Name: name,
	Type: b.tmpbuf.String(),
	Class: decl.class,
	})
	b.tmpbuf.Reset()
	}

Pass through package names, use it for godoc in emacs-company #463

Pass through package names, use it for godoc in emacs-company #463

Conversation

antifuchs commented Aug 16, 2017

lloiser Aug 16, 2017

Choose a reason for hiding this comment

antifuchs Aug 16, 2017

Choose a reason for hiding this comment

lloiser Aug 16, 2017

Choose a reason for hiding this comment

antifuchs Aug 16, 2017

Choose a reason for hiding this comment

lloiser Aug 16, 2017

Choose a reason for hiding this comment

antifuchs commented Aug 16, 2017

lloiser commented Aug 16, 2017

antifuchs commented Aug 16, 2017

nsf commented Aug 16, 2017

asf-stripe commented Aug 16, 2017 • edited Loading

nsf commented Aug 16, 2017

antifuchs commented Aug 17, 2017

nsf commented Aug 20, 2017

nsf commented Aug 25, 2017 • edited Loading

asf-stripe commented Aug 25, 2017

asf-stripe commented Aug 16, 2017 •

edited

Loading

nsf commented Aug 25, 2017 •

edited

Loading