Navigation Menu

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

go/printer: force multiline literal to end with } on line by itself #10570

Open
nightlyone opened this issue Apr 24, 2015 · 16 comments
Open

go/printer: force multiline literal to end with } on line by itself #10570

nightlyone opened this issue Apr 24, 2015 · 16 comments
Milestone

Comments

@nightlyone
Copy link
Contributor

  • What version of Go are you using (go version)?
    go version go1.4.1 linux/amd64
  • What operating system and processor architecture are you using?
    Linux on amd64
  • What did you expect to see?
    I expect to see b: 2 then comma, newline, closing brace/curly bracket as in:
package main

type A struct {
    a int
    b int
}

func main() {
    x := A{
        a: 1,
        b: 2,
    }
}
  • What did you see instead?
    I did see a closing curly brace hugging the field value 2 of A.b as in:
package main

type A struct {
    a int
    b int
}

func main() {
    x := A{
        a: 1,
        b: 2}
}
@robpike
Copy link
Contributor

robpike commented Apr 24, 2015

I believe this is working as intended, but I'll leave the bug open for gri. There are legitimate cases where leaving the brace on that line makes sense.

@minux
Copy link
Member

minux commented Apr 24, 2015 via email

@nightlyone
Copy link
Contributor Author

Sadly gofmt should unify style and two styles proliferated already in our company due to this issue.
So I am very interested in the legitimate use cases here.

@nightlyone
Copy link
Contributor Author

Any news here?

@griesemer
Copy link
Contributor

I'll have a look at this for 1.6. But it is not the most urgent issue.

@griesemer griesemer modified the milestones: Go1.6, Unplanned Sep 10, 2015
@rsc rsc changed the title cmd/gofmt: should re-format multiline struct literal end from "a:foo}\n" to "a:foo,\n}\n" go/printer: force multiline literal to end with } on line by itself Nov 5, 2015
@rsc
Copy link
Contributor

rsc commented Nov 5, 2015

[Reworded request title, but not saying whether it should happen.]

@rsc rsc modified the milestones: Unplanned, Go1.6 Nov 30, 2015
@rsc
Copy link
Contributor

rsc commented Nov 30, 2015

Too late for Go 1.6.

@nightlyone
Copy link
Contributor Author

Gentle ping.

@griesemer griesemer modified the milestones: Go1.7Maybe, Unplanned May 8, 2016
@griesemer
Copy link
Contributor

Not sure we're getting to this due to many higher priority items, but marked 1.7Maybe so we keep an eye on it. If not, we'll try to do a concentrated effort to address most (all) the open gofmt bugs in the future, as gofmt could use some love.

@rsc
Copy link
Contributor

rsc commented May 9, 2016

It's too late for Go 1.7 at this point. This is a purely cosmetic change; we're down to semantic release-blocking changes.

@rsc rsc modified the milestones: Unplanned, Go1.7Maybe May 9, 2016
@nightlyone
Copy link
Contributor Author

@rsc / @griesemer while I agree that it is too late for a CL, could we at least finish the strategic/aesthetic decisions about how this kind of code should be formatted?

Then collaboration on the technical implementation can be done by a wider group of developers.

Thanks!

@nightlyone
Copy link
Contributor Author

@griesemer any progress on the aesthetics front here?

@griesemer
Copy link
Contributor

@nightlyone Generally, gofmt respects the line-breaks that exist already. In this case there isn't one after the last element. So arguably, this is "correct" as is. From a purely aesthetic point of view, if the first element of a composite literal element list is not on the same line as the opening '{', I'd put the '}' on a separate line as well.

@griesemer griesemer modified the milestones: Go1.9, Unplanned Nov 18, 2016
@lnshi
Copy link

lnshi commented Feb 28, 2017

Well, just encounter this now in go go1.7.4 darwin/amd64, so the code need to be like this:

user := model.Users {
  UserId: "BJg4Kj2HOe",
  Gender: constant.G_U,
  MobilePhone: mobilePhone,
}

the comma , at the last line, the one after mobilePhone is mandatory, a bit wried, but still acceptable.

@griesemer griesemer modified the milestones: Go1.10, Go1.9 Jun 20, 2017
@rsc rsc removed this from the Go1.10 milestone Nov 22, 2017
@rsc rsc added this to the Go1.11 milestone Nov 22, 2017
@griesemer griesemer modified the milestones: Go1.11, Go1.12 Jun 27, 2018
@griesemer griesemer modified the milestones: Go1.12, Go1.13 Dec 11, 2018
@griesemer griesemer modified the milestones: Go1.13, Go1.14 May 6, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@nightlyone
Copy link
Contributor Author

Note that @mvdan actually implemented this in https://github.com/mvdan/gofumpt as one of the additional rules it supports. So it seems possible.

@mvdan could you give some implementation hints or contribute the solution back?

@mvdan
Copy link
Member

mvdan commented Nov 12, 2020

You can see the code here if you're curious. I don't really have any hints per se; I just tried to implement simple rules that better restrict how most people write good Go. Of course I'm happy to contribute back some of the rules if Robert wants, but note that they'll be more like reimplementations since gofumpt needs to alter a program before being fed to go/printer.

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

No branches or pull requests

8 participants