Add new option to auto save all buffers on navigation #144

Merged
merged 1 commit into from Aug 31, 2016

Projects

None yet

3 participants

@paymog
Contributor
paymog commented Aug 10, 2016

Summary: Added a new option which, when set, causes all open and changed
buffers to be written when navigating away from a vim session. If auto
save is turned on and this option is turned off, only the active buffer
gets saved.

Test Plan:

  1. Open up two tmux panes, one running vim and the other not

  2. In the vim pane open several files and modify each of them without
    explicitly writing any of them

  3. navigate away from the tmux split containing vim

  4. navigate back to the tmux split containing vim

  5. write all files with ":wa"

  6. see that nothing got written suggesting that every buffer was written
    in step 3

  7. enable new option

  8. open two tmux splits

  9. in one of the splits open vim without a file name. (just vim not
    vim <some-file-path>)

  10. enter text into the vim instance

  11. navigate away from tmux pnae containing unnamed vim

  12. see that no error is shown to user

@blueyed blueyed and 2 others commented on an outdated diff Aug 10, 2016
@@ -113,7 +113,11 @@ Left would be created with `nnoremap <silent> <c-h> :TmuxNavigateLeft<cr>`.
let g:tmux_navigator_save_on_switch = 1
-This will execute the update command on leaving vim to a tmux pane. Default is Zero
+This will execute the `update` command on leaving vim to a tmux pane. Default is Zero
+
+ let g:tmux_navigator_save_all_buffers_on_switch = 1
@blueyed
blueyed Aug 10, 2016 Collaborator

What about using 2 with the existing g:tmux_navigator_save_on_switch setting to indicate this behavior?

@paymog
paymog Aug 10, 2016 Contributor

Yeah, that seems like a pretty reasonable idea. If you think that's better I'm more than happy to implement it.

@paymog
paymog Aug 16, 2016 Contributor

Should we abandon this feature?

@christoomey
christoomey Aug 23, 2016 Owner

I like @blueyed's idea of opting into the existing variable. What do you think of the following for the help text:

You can configure the plugin to write the current buffer, or all buffers, when navigating from Vim to tmux. This functionality is exposed via the g:tmux_navigator_save_on_switch variable, which can have either of the following values:

Value Behavior
1 :update (write the current buffer, but only if changed)
2 :wall (write all buffers)

To configure, add the following (with the desired value) to your ~/.vimrc:

" Write all buffers before navigating from Vim to tmux pane
let g:tmux_navigator_save_on_switch = 2
@paymog
Contributor
paymog commented Aug 22, 2016

I'm not sure what standard github etiquette is so I'll ping this PR one last time. If there's no traction by the end of the week I'll just abandon it.

@blueyed
Collaborator
blueyed commented Aug 22, 2016

@paymog
Please be patient.
@christoomey is slow with reviewing stuff here.

As for me, I would squash it into a single option already, and then it can just wait here for more feedback.

@christoomey
Owner

Hey @paymog, sorry for the delays here. As @blueyed fairly pointed out, I can be a bit slow to respond :) I can't speak for other folks managing projects, but I'm personally fine if you ping after not hearing anything for > a week.

That said, I think this is great! I like @blueyed's idea re: using the existing variable and I've added a note about possible wording for the REAMDE section about it.

Steps to get this merged:

  1. Update to use the existing variable
  2. Squash down to a single commit (and if you're ok with it I'd remove the "test plan" from the commit message. Makes sense for PR discussion, but doesn't need to live on in the log. Want git log to focus on the "why" of a change)
  3. Ping me one more time (I'll keep an eye out for it) :)

Thanks again, @paymog and @blueyed

@paymog
Contributor
paymog commented Aug 23, 2016

Sorry for bothering you guys so much! Didn't mean to be a pain in the ass.

The updated PR should be out now. I promise I won't pester you guys anymore now that I know it's a slow process :)

@blueyed
Collaborator
blueyed commented Aug 23, 2016

Didn't mean to be a pain in the ass.

Don't worry - you are not.. :)

Let's see if @christoomey holds on to his promises.. ;)

@blueyed blueyed commented on an outdated diff Aug 23, 2016
-This will execute the update command on leaving vim to a tmux pane. Default is Zero
+Value | Behavior
+------ | ------
+1 | `:update` (write the current buffer, but only if changed)
+2 | `:wall` (write all buffers)
@blueyed
blueyed Aug 23, 2016 Collaborator

Can this be aligned to be better readable also in plain text (without being rendered)?

@blueyed blueyed commented on an outdated diff Aug 23, 2016
-This will execute the update command on leaving vim to a tmux pane. Default is Zero
+Value | Behavior
+------ | ------
+1 | `:update` (write the current buffer, but only if changed)
+2 | `:wall` (write all buffers)
+
+To configure, add the following (with the desired value) to your ~/.vimrc:
@blueyed
blueyed Aug 23, 2016 Collaborator

s/To configure,/To enable this/

@blueyed blueyed and 1 other commented on an outdated diff Aug 23, 2016
plugin/tmux_navigator.vim
@@ -67,12 +67,16 @@ function! s:TmuxAwareNavigate(direction)
" a) we're toggling between the last tmux pane;
" b) we tried switching windows in vim but it didn't have effect.
if tmux_last_pane || nr == winnr()
- if g:tmux_navigator_save_on_switch
- try
- update
- catch /^Vim\%((\a\+)\)\=:E32/
- endtry
- endif
+ try
+ if g:tmux_navigator_save_on_switch == 1
+ update " save the active buffer. See :help update
+ elseif g:tmux_navigator_save_on_switch == 2
+ wall " save all the buffers. See :help wall
+ endif
+ catch /^Vim\%((\a\+)\)\=:E32/ " catches the no file name error thrown by update
+ catch /^Vim\%((\a\+)\)\=:E141/ " catches the no file name error thrown by wall
+ endtry
@blueyed
blueyed Aug 23, 2016 Collaborator

You might want to put the command in their own try blocks (and only catch the relevant error).

@paymog
paymog Aug 23, 2016 Contributor

Great idea!

@blueyed
Collaborator
blueyed commented Aug 23, 2016

Thanks, LGTM - given some minor doc fix and other nitpicking.. ;)

Feel free to push this as a fixup commit (http://fle.github.io/git-tip-keep-your-branch-clean-with-fixup-and-autosquash.html), and we can then review it better (only the changes), and squash it from GitHub's UI.

@christoomey christoomey commented on an outdated diff Aug 23, 2016
-This will execute the update command on leaving vim to a tmux pane. Default is Zero
+Value | Behavior
+------ | ------
+1 | `:update` (write the current buffer, but only if changed)
+2 | `:wall` (write all buffers)
+
+To configure, add the following (with the desired value) to your ~/.vimrc:
+
+```
@christoomey
christoomey Aug 23, 2016 Owner

Can you add vim after the backticks so syntax highlighting kicks in?

@christoomey
Owner

@paymog No worries about bugging, you are not. I'm just a bit slower than we'd all probably like :)

All of @blueyed's comments sounds good to me, plus I added one more. Once we get those in we can get this merged. Thanks for continued effort.

@blueyed
Collaborator
blueyed commented Aug 23, 2016

Awesome! 👍

@paymog paymog Add new behaviour to save all buffer on navigation
Summary: Added a new behaviour to tmux_navigator_save_on_switch which, when
set, causes all open and changed buffers to be written when navigating away
from a vim session.
22db869
@paymog
Contributor
paymog commented Aug 31, 2016

@christoomey ping!

@christoomey christoomey merged commit 0c676d8 into christoomey:master Aug 31, 2016
@christoomey
Owner

@paymog merged! Thanks for continued efforts on this.

@paymog
Contributor
paymog commented Aug 31, 2016

Thanks! Feels good to contribute back!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment