-
Notifications
You must be signed in to change notification settings - Fork 453
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
Add goimports beautifier for Go language #1726
Conversation
0af2f8e
to
f4e5c8d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Afterwards, you will also need to re-generate the documentation with npm run docs
. Thanks!
src/beautifiers/goimports.coffee
Outdated
isPreInstalled: false | ||
|
||
options: { | ||
Go: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like no options are supported, therefore this should be false
. 😃
src/beautifiers/goimports.coffee
Outdated
module.exports = class Goimports extends Beautifier | ||
name: "goimports" | ||
link: "https://godoc.org/golang.org/x/tools/cmd/goimports" | ||
isPreInstalled: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been deprecated.
We have switched to using Executables
: #1687
See https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/sass-convert.coffee#L7-L20 and https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/sass-convert.coffee#L30 for simple example. And another: example https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/autopep8.coffee#L12-L56
Please let me know if you have any questions! This is a new process to help standardize creating non-preinstalled beautifiers. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that goimports doesn't have version option or subcommand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If --version
is not available then I've used --help
and returned a fake version of 0.0.0
like https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/beautysh.coffee#L14-L16 after validating the --help
actually works (returned golang help...
or some substring).
7cc7353
to
22dc594
Compare
Could you re-generate the documentation and commit to master? |
Are you asking me? If so, I can, however I highly encourage you to do this part yourself. I need to see that your changes were setup properly before I merge and the auto-generated documentation usually shows it all worked. It's easy: |
I don't want to include changes related to #1708 |
Oh, I see what you mean. Pushed! |
22dc594
to
d2eaba7
Compare
Thank you for your help! |
version: { | ||
# Does not display version | ||
args: ['--help'], | ||
parse: (text) -> text.indexOf("usage: goimports") isnt -1 and "0.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also created a Docker image for goimports
: https://hub.docker.com/r/unibeautify/goimports/
I am going to use it during my testing right now.
Worked great! Thank you for contributing! |
Published to v0.30.3 |
What does this implement/fix? Explain your changes.
This adds goimports beautifier for Go language.
Does this close any currently open issues?
...
Any other comments?
...
Checklist
Check all those that are applicable and complete.
master
branchnpm run docs
CHANGELOG.md
under "Next" section