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

format with gopls #2483

Closed
bhcleek opened this issue Sep 9, 2019 · 7 comments
Closed

format with gopls #2483

bhcleek opened this issue Sep 9, 2019 · 7 comments

Comments

@bhcleek
Copy link
Collaborator

bhcleek commented Sep 9, 2019

Add support for formatting with gopls instead of gofmt or goimports.

acceptance criteria
  • when g:go_fmt_command is gopls vim-go uses gopls to format Go source files
  • when g:go_fmt_command is set to gopls, gopls should be used regardless of the value of g:go_fmt_experimental.
  • g:go_fmt_experimental should be respected regardless of the value of g:go_fmt_command.
@bhcleek bhcleek added this to the vim-go 1.22 milestone Sep 9, 2019
@rbisewski
Copy link
Contributor

I'll take a shot at doing this, if nobody else is.

@bhcleek
Copy link
Collaborator Author

bhcleek commented Sep 17, 2019

This will be a bit tricky to get right, but the basics are that gopls will provide a list of changes to make. Those changes need to be applied starting in reverse order (i.e. starting at the last line), because of the way the LSP protocol defines them.

Let me know if you have any questions or need a hand.

@rbisewski
Copy link
Contributor

So you think a simple gopls format -w /path/to/file.go and some new tests won't suffice? I've been testing this on my own code for the last day or so and it seems to format correctly; to my knowledge anyway, I will admit I am far from a gopls expert.

If it's alright with you, I have pushed my changes to a branch, so I'll make a pull request in a few minutes and we can continue the discussion there :)

@bhcleek
Copy link
Collaborator Author

bhcleek commented Sep 19, 2019

Yeah, as I mentioned on the PR, gopls format doesn't add any value over gofmt, because they both use the same underlying packages to do the formatting. However, gopls regularly outputs a formatted file. Basically, any time it's sent the document/didChange or document/didOpen request, it will send a diagnostic with the formatted output.

As I've been thinking about it more, I'm starting to lean towards:

  1. leaving go_fmt_command as-is so that goimports can be used on save, because if gopls doesn't add imports.
  2. (if gopls doesn't automatically add imports) Introduce a new config value, g:go_gopls_fmt_enabled, that controls whether the formatting provided by gopls automatically updates. We might need to modify the workspace/didChange integration to use incremental updates.
  3. When g:go_gopls_fmt_enabled is set, update formatting as the user types (maybe on pressing Enter?)

I think it's worth closing the #2499 and collaborating to define how this should work. What do you think?

@rbisewski
Copy link
Contributor

Agreed and yeah, gopls doesn't add imports, so probably your suggestion is the most straightforward way it could be done :)

@bhcleek
Copy link
Collaborator Author

bhcleek commented Sep 19, 2019

FWIW, I think that gopls will add imports, but gopls format does not.

@bhcleek
Copy link
Collaborator Author

bhcleek commented Feb 23, 2020

Implemented in #2729

@bhcleek bhcleek closed this as completed Feb 23, 2020
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

2 participants