search

christoomey / vim-tmux-runner

Vim and tmux, sittin' in a tree...
MIT License
291 stars 37 forks source link

Add command to focus runner without zooming #93

Closed redxtech closed 4 years ago

redxtech commented 4 years ago

I'm making this PR to address the fact that there's no way to focus the runner pane without zooming into it (#91).

It adds a new command (VtrSelectRunner), which calls a new function (s:SelectRunnerPane), which is a clone of s:FocusRunnerPane, except it has the function call that zooms into the pane after selecting it removed.

I've updated the help file to reflect this command, declared the command, and added a new leader keybind behind the config options like the rest of the commands.

christoomey commented 4 years ago

Hey @redxtech, thanks for the change, and for the updates to the docs. Can't believe I started this thing over 7 years ago!

The functionality makes sense, although I'm wondering if it might be useful to group this functionality with the existing command as a ! variant of :VtrFocusRunner, rather than as a distinct command?

function! s:FocusRunnerPane(should_zoom)
    if !s:ValidRunnerPaneSet() | return | endif
    call s:FocusTmuxPane(s:runner_pane)
    if a:should_zoom
      call s:SendTmuxCommand("resize-pane -Z")
    endif
endfunction

command! -bang VtrFocusRunner call s:FocusRunnerPane(<bang>!0)
redxtech commented 4 years ago

7 years ago? Wow that's been a while. Glad to see it's still working well.

If breaking other people's configs wasn't a concern the way I would do it would honestly be like this, except having it default to not zooming, as that seems like base functionality and zooming is extra.

Since that isn't the case, both of the implementations seem like quite reasonable ways to go about it. The new command has more obvious separation, while the bang approach has less repeated code and probably a better interface for scripts & etc.

Let me know which you would prefer and I'll update the PR.

christoomey commented 4 years ago

Hi @redxtech, I've just merged the other PR. I agree with all of your comments above, and I'm slightly in favor of the bang version (and having the ! variant be the new functionality in order to not break existing usage, although I agree that it intuitively feels reversed).

Thanks for the update, and thanks for working through the alternative approach!

redxtech commented 4 years ago

Great! Glad to have this feature added, and I can wholly understand taking the path of least broken setups.