Skip to content

fmt: don't call command twice for successful output#422

Merged
fatih merged 1 commit intomasterfrom
fmt-improvement
May 14, 2015
Merged

fmt: don't call command twice for successful output#422
fatih merged 1 commit intomasterfrom
fmt-improvement

Conversation

@fatih
Copy link
Copy Markdown
Owner

@fatih fatih commented May 13, 2015

Previously we were calling gofmt/goimports twice when the command was
successful. First we call it so we can catch any errors and display
it(in quickfix window), if there is no errors, we called it again,
purely so we could pipe the commands output (which in our case is the
formatted output) directly to the current buffer. This was very fast
for gofmt, but with goimports it was slow.

Now, we don't call the command again if it's successful, instead we
just replace the output of the first command with the current buffer.
This is actually so obvious and I'm thinking why I didn't I this in the
first place. Humans ...

@fatih
Copy link
Copy Markdown
Owner Author

fatih commented May 13, 2015

Just remembered that there was a problem with Vim's own history when we try to replace it, but again it could be something entire different. I'm going to use this for a couple of days extensively to catch any errors before I merge this PR.

Previously we were calling gofmt/goimports twice when the command was
successful. First we call it so we can catch any errors and display
it(in quickfix window), if there is no errors, we called it again,
purely so we could pipe the commands output (which in our case is the
formatted output) directly to the current buffer. This was very fast
for gofmt, but with goimports it was slow.

Now, we don't call the command again if it's successful, instead we
just replace the output of the first command with the current buffer.
This is actually so obvious and I'm thinking why I didn't I this in the
first place. Humans ...
@fatih fatih force-pushed the fmt-improvement branch from 36541d3 to 7264cc9 Compare May 13, 2015 23:48
@fatih
Copy link
Copy Markdown
Owner Author

fatih commented May 14, 2015

This is good to go. No significant problems found yet.

fatih added a commit that referenced this pull request May 14, 2015
fmt: don't call command twice for successful output
@fatih fatih merged commit 618b6c1 into master May 14, 2015
@fatih fatih deleted the fmt-improvement branch May 26, 2015 07:41
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.

1 participant