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

Allow specifying random formatting tools instead of four fixed options #587

Closed
xabinapal opened this issue Aug 31, 2020 · 11 comments
Closed

Comments

@xabinapal
Copy link

Is your feature request related to a problem? Please describe.
Current extension settings only allow to select gofmt, goimports, goreturns and goformat as formatters. There are other formatting tools that one would want to use, but this is not allowed.

Describe the solution you'd like
gofumpts and gofumports are two known alternatives to gofmt and goimports. It would be nice to be allowed to use them, or any other formatter tool. This should be allowed by using a textbox instead of a selector for this property, with maybe suggestions for the current options.

Describe alternatives you've considered
As a workaround, manually modifying the property go.formatTool in the settings.json file allows to use gofumports as the formatting tool. By doing this, VS Code complains about an invalid value for that setting, but even with that warning, gofumports is used and code is formatted as expected.

Additional context
gopls just added support for gofumpts, so it can used by enabling it in the language server (with some undocumented settings, but this is already stated in it's documentation), but not everybody can use it, so it would be nice to add this feature.

@hyangah
Copy link
Contributor

hyangah commented Aug 31, 2020

Try go.alternateTools. See the instructions given here: https://github.com/mvdan/gofumpt#installation

I am afraid that replacing the selector with a textbox would make common cases less visible. And the extension has some specializations depending on the chosen tool (e.g. flags) so support for arbitrary tools will not work perfectly.

@hyangah hyangah closed this as completed Aug 31, 2020
@xabinapal
Copy link
Author

xabinapal commented Aug 31, 2020

@hyangah so the recommended way of using any formatter (not specifically gofumpt) is setting the formatting tool as gofmt (because is the only one without hardcoded flags in goFormat.ts), override it in go.alternateTools and set the required flags to run it through go.formatFlags? Maybe it's just me, but it seems a little bit counterintuitive, requires knowledge of how the extension works internally and may break in future releases of this extension if hardcoded flags are added to gofmt...

And also this method feels does not feel completely right. The workaround I mentioned in my OP works but it triggers a Value is not accepted message. But using go.alternateTools also triggers a warning message: Property gofmt is not allowed. Both methods will end up executing the same code in goFormat.ts, and both feel like workarounds...

I think that if this is the recommended way, it should at least be clearly indicated in the UI and the go.alternateTools object must be extended to "recognize" the four fixed formatting tools so it feels less hackish.

Edit: maybe another alternative could be not to replace the dropdown with a textbox, but instead keep it and add an extra textbox for other formatters. If set, it would take precedence over the go.formatTool value without having to do all that gofmt magic. This could apply also for lint and doc tools.

@hyangah
Copy link
Contributor

hyangah commented Aug 31, 2020

@xabinapal If you find a way to add an extra textbox in addition to the dropdown, please send a PR. The UI is implemented by VS Code, so I'd like to know how to do it.

go.alternateTools triggering a warning message is a bug (#526) and will be fixed in a new version.

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/251897 mentions this issue: Allow usage of unsupported formatter and linter tools with new settings

@xabinapal
Copy link
Author

xabinapal commented Aug 31, 2020

As I don't think that a mix of a dropdown+textbox setting is possible, I've submitted a PR that simply adds two new nullable textbox properties for the lint and the format tools. By allowing null values, they are show in the UI but they are only editable via the settings.json file, so basic users won't mess with them. These settings override the dropdown values and don't perform any argument specialization.

@hyangah
Copy link
Contributor

hyangah commented Aug 31, 2020

@xabinapal Thanks for trying to fix this, but I am not sure if introducing two extra settings is a good idea. go.alternateTools is somewhat well established, used by many VS code users for many years. Basic users won't mess with them. Having extra knobs that influence the tool selection is more confusing and complicates tool selection logic.

A good news is that we are planning to default to gopls and then the old formatter/linter settings will be effectively deprecated.

@hyangah hyangah reopened this Aug 31, 2020
@xabinapal
Copy link
Author

Obviously using gopls by default is the best solution. Anyway, at least the alternateTool and gofmt magic can be found in this issue if anyone lands here while trying to figure it 😅.

Thank you!

@hyangah
Copy link
Contributor

hyangah commented Aug 31, 2020

If your goal is to use gofumpts or gofumports, adding them as available options for go.formatTool seems acceptable as well as these newer tools are getting more popular. Do you want to try to add them?

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/252922 mentions this issue: src/goTools.ts: recognize gofumpt, gofumports formatters

@xabinapal
Copy link
Author

Sorry, I missed your answer! I've just seen that you have submitted the patch you suggested, that's great! Thank you!

gopherbot pushed a commit that referenced this issue Sep 8, 2020
gofumpt and gofumports can be used as drop-in replacements of
gofmt and goimports, so we directed users to utilize
`"go.alternateTools"`
(https://github.com/mvdan/gofumpt#installation)

However, this is pretty convoluted and less user-friendly.
As more users are adopting gofumpt and gofumports,
promote them as officially recognized formatters.

It would be ideal if the extension allows to choose
any formatter as long as the tool follows a certain spec,
but we are not there yet. And, as we are moving towards
using the language server as the formatter, this setting
will become obsolete.

Updates #587

Change-Id: Ib1fc5c91c0d6fc7fed37132c6cbd0dfa44b6a08b
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/252922
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
@hyangah
Copy link
Contributor

hyangah commented Sep 8, 2020

@xabinapal thanks for understanding - hopefully, we can come up with more user-friendly ways of configuring formatter etc with gopls soon. I will close this issue for now.

@hyangah hyangah closed this as completed Sep 8, 2020
@hyangah hyangah added this to the v0.17.0 milestone Sep 8, 2020
@golang golang locked and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants