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

x/tools/gopls: support fillstruct for partially-filled structs #39804

Closed
stamblerre opened this issue Jun 24, 2020 · 4 comments
Closed

x/tools/gopls: support fillstruct for partially-filled structs #39804

stamblerre opened this issue Jun 24, 2020 · 4 comments

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jun 24, 2020

We currently only offer fillstruct for completely empty struct literals. We should also offer to fill in values for partially filled structs.

/cc @joshbaum

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 13, 2020

Change https://golang.org/cl/262018 mentions this issue: x/tools/gopls: support fillstruct for partially-filled structs

@jadekler
Copy link
Member

@jadekler jadekler commented Oct 16, 2020

https://golang.org/cl/262018 now adds support for partially filled structs - wooo! But, all comments get destroyed. =)


Coming to a crossroads, now that I'm tackling comments. The ast package is proving to be a huge hindrance, since fillstruct doesn't use *ast.File in its implementation. Instead, it prints with token.FileSet. I'm not seeing a way to print subsets of the file (which seems to require token.FileSet) with comments (which appear to only exist on ast.File and printer.CommentedNode).

I had hoped that printer.CommentedNode would do the trick, but printing printer.CommentedNode with an ast.Expr and some comments seems to not work. (Does that meet expectations?)

So, the crossroads I'm at:

Option A: github.com/dave/dst

It looks like github.com/dave/dst has what I'm looking for: comments tied directly to nodes. But, introducing a v0 dependency seems sketchy.

Option B: hacky printing

I vaguely bet there's a way to figure out the fields that are already set (expr.Elts), then just go to the end of the struct and stick zero values of all the not-set fields at the end with printing hackery. That would leave all the comments and set fields up at the top, and all the zero'd values at the bottom. Not pretty but I thiiiink doable?


@joshbaum @stamblerre are there any directions that I'm missing? If not, any preference on the above?

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Oct 21, 2020

I think Option B would be best, as we'd like to avoid adding dependencies to x/tools.
I would honestly say that overwriting the comments is also fine--but I'm guessing this is incompatible with our test frameworks?

@jadekler
Copy link
Member

@jadekler jadekler commented Oct 21, 2020

Ah! Ok, that helps a lot. I actually haven't tried all the other tests, but not sure. I'll give option B a shot, and then if that doesn't work will either rewrite comments or just not work when comments exist. :) Will report back soon!

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

Successfully merging a pull request may close this issue.

None yet
4 participants