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

Integrate asmfmt #673

Merged
merged 1 commit into from
Jan 16, 2016
Merged

Integrate asmfmt #673

merged 1 commit into from
Jan 16, 2016

Conversation

cespare
Copy link
Contributor

@cespare cespare commented Jan 1, 2016

This change integrates asmfmt support for .s files, in a similar way to how :GoFmt works.

Do take a careful look at this change; I'm not at all a vimscript expert.

@cespare
Copy link
Contributor Author

cespare commented Jan 1, 2016

/cc @klauspost just FYI

@fatih
Copy link
Owner

fatih commented Jan 3, 2016

Thanks @cespare. I'm right now fixing my backlog of issues (FIFO order). I'll look into it.

@fatih
Copy link
Owner

fatih commented Jan 16, 2016

Hi @cespare. This is amazing work. Thank you very much. So looking at it, it seems like we're treating any *.s file as asm. For formating itself, I think doing the following would be also sufficient:

let g:go_fmt_command = "asmfmt"

if get(g:, "go_fmt_autosave", 1)
  autocmd BufWritePre *.go,*.s call go#fmt#Format(-1)
endif

Doesn't this work? I'm just curious because theoretically it should work.

@cespare
Copy link
Contributor Author

cespare commented Jan 16, 2016

No, this doesn't work. It breaks formatting of Go files because asmfmt doesn't get the right filename, so it tries to format everything as asm.

We could probably get it to work by constructing a tempfile with the same file extension as the current file. However, I'm opposed to using asmfmt for formatting Go code from vim in general:

  • If there's an improvement or bugfix to gofmt (or goimports, etc) you have to wait for that change to make it into asmfmt
  • If you want to use some other gofmt variant that isn't bundled into asmfmt (it includes goimports and goreturns), you're out of luck

The changes I've made are bigger than what you've suggested, but it also makes it easy to add some better highlighting and indent files for Go asm in the future as well.

@fatih
Copy link
Owner

fatih commented Jan 16, 2016

Got it. I don't have much to say, it's already a great work, and thanks again for the work, much appreciated 👍

fatih added a commit that referenced this pull request Jan 16, 2016
@fatih fatih merged commit 7708bce into fatih:master Jan 16, 2016
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

Successfully merging this pull request may close these issues.

2 participants