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

handle autowriteall the same as autowrite #1653

Merged
merged 1 commit into from
Jan 28, 2018

Conversation

bhcleek
Copy link
Collaborator

@bhcleek bhcleek commented Jan 20, 2018

Vim docs say that autowriteall implies autowrite, so make sure that
go#cmd#autowrite() has the same behavior when only one of either
autowrite or autowriteall is set.

Vim docs say that autowriteall implies autowrite, so make sure that
go#cmd#autowrite() has the same behavior when only one of either
autowrite or autowriteall is set.
@codecov-io
Copy link

codecov-io commented Jan 20, 2018

Codecov Report

Merging #1653 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1653   +/-   ##
=======================================
  Coverage   21.73%   21.73%           
=======================================
  Files          53       53           
  Lines        4173     4173           
=======================================
  Hits          907      907           
  Misses       3266     3266
Flag Coverage Δ
#nvim 16.89% <0%> (ø) ⬆️
#vim74 19.38% <100%> (ø) ⬆️
#vim80 20.6% <100%> (ø) ⬆️
Impacted Files Coverage Δ
autoload/go/cmd.vim 5.73% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffb2f43...561365d. Read the comment docs.

@fatih
Copy link
Owner

fatih commented Jan 22, 2018

@bhcleek I'm not sure about this change. Maybe I don't want to enable autowriteall because I don't want it to save on :exit ? (this is what I do btw). So in that regard, I think go#cmd#autowrite() is correct that it only checksautowrite. Is there something I'm missing here?

@bhcleek
Copy link
Collaborator Author

bhcleek commented Jan 22, 2018

@fatih Thanks for taking the time to review this tiny PR.

Like you, my vim is configured with autowrite and noautowriteall, but after reading the docs for autowriteall, and verifying some of vim's behaviors when it's configured with noautowrite and autowriteall, I realized that go#cmd#autowrite was incomplete.

  • go#cmd#autowrite is called to make sure any unsaved changes in a buffer are saved before running a command like :GoVet if the user has set autowrite.
  • When a user sets only autowriteall and not autowrite (e.g. set autowriteall, set noautowrite), vim will behave as if autowrite is set. From :help autowriteall:

Setting this option also implies that Vim behaves like 'autowrite' has been set.

  • You can see for yourself that when autowriteall is set and autowrite is not, that moving to another buffer saves the changes before showing the next buffer.

All this PR does is make sure that vim-go saves the buffer using the same rules with respect to autowrite and autowriteall that vim itself applies.

@bhcleek bhcleek merged commit 6de9daa into fatih:master Jan 28, 2018
@bhcleek bhcleek deleted the handle-autowriteall branch January 28, 2018 15:37
bhcleek added a commit that referenced this pull request Jan 29, 2018
Update CHANGELOG.md for #1652, #1653, #1656, and #1664.
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.

4 participants