update deps, vendor go/printer from Go 1.19 #255
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These two packages affect the formatting of gofmt, and by extension gofumpt as well. For example, most changes in formatting to gofmt are done in go/printer, as it implements most of the formatting logic. This tight coupling between gofmt and std packages like go/printer is not a problem for gofmt, as gofmt is released with Go itself. It is very unlikely that any user would build a version of gofmt with a different version of those std packages. gofumpt is a separate Go module, so when users `go install` it, they may currently be using Go 1.18.x or 1.19.x. This can cause problems, as choosing a different major version can easily lead to different behavior. To get the same tight coupling that gofmt has, vendor the two libraries which affect formatting. go/doc/comment is also in this bag since it affects how gofmt reformats godoc comments. Note that this vendoring is not ideal, as it adds code duplication. This shouldn't bloat the size of gofumpt binaries, as they simply swap out the original packages for the vendored ones. For users of the mvdan.cc/gofumpt/format library such as gopls, they may get duplication if they use packages like go/printer elsewhere. This is a small amount of unfortunate bloat, but I don't see a better way to ensure that each version of gofumpt behaves in a consistent way. Note that we're not vendoring std packages for parsing Go code like go/parser or go/token, and neither are we vendoring go/ast. If a project uses new Go syntax, such as generics, they will still need to build gofumpt with a new enough version of Go. The copied code comes from Go 1.19.4 and will be updated regularly. The copying was done manually this time, skipping test files and rewriting imports as necessary. In the future, we an automate it. Also note that we ran gofumpt on the copied code. Fixes #244.
|
cc @adamroyjones, would love your review as well :) |
|
also cc @findleyr to hear whether this would be good or bad news for gopls. |
Oiyoo
approved these changes
Jan 1, 2023
adamroyjones
reviewed
Jan 7, 2023
|
Yes, soon. I want to check with the gopls people first, to see if this sort of vendoring will be a problem for them. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(see commit messages - please do not squash)
Fixes #244.