supercollider / scvim

Vim plugin for SuperCollider
GNU General Public License v3.0
121 stars 28 forks source link

Add multiplexer support #38

Closed gusano closed 6 years ago

gusano commented 6 years ago

Fix #32

cc @dvzrv : )

Both screen and tmux are supported:

dvzrv commented 6 years ago

@gusano you rule! Got too busy packaging and working on other software the last few months... will definitely check this out asap.

gusano commented 6 years ago

@dvzrv any chance to sneak this one in? :)

gusano commented 6 years ago

@dvzrv OK I pushed a change following your advice.. I hope this is the correct way to do it.

dvzrv commented 6 years ago

@gusano this works!

However, it seems your code assumes, that all variables are defined in vimrc. When starting from scratch, one gets this when using SClangStart:

Error detected while processing function SClangStart:
line    4:
E121: Undefined variable: g:scSplitDirection
E15: Invalid expression: (a:0 == 2) ? a:1 : g:scSplitDirection
line    5:
E121: Undefined variable: g:scSplitSize
E15: Invalid expression: (a:0 == 2) ? a:2 : g:scSplitSize
line    8:
E121: Undefined variable: l:splitDir
E15: Invalid expression: "tmux split-window -" . l:splitDir . " -p " . l:splitSize . " ;"
line    9:
E121: Undefined variable: l:cmd
line   10:
E121: Undefined variable: l:cmd
E116: Invalid arguments for function system

A better way would be to default back to the default settings (stated in README.md), in case the variables are not defined. Additionally 50% of the screen is maybe a little generous for g:scSplitSize as a default.

Apart from that, what has been bugging me (generally) is, that the sclang pane never gets closed, once one closes vim (or ends SC for that matter). I think this can be achieved with tmux kill-pane -t <pane>.

Another nice addition would be to be able to start panes in reverse (i.e. top pane for sclang output, instead of bottom pane, left pane for sclang output, instead of right pane). The README.md is also not overly verbose about what to actually set for g:scSplitDirection` (only "h" is mentioned). A little more info on it would be good for the user. E.g.:

h = horizontal split v = vertical split

gusano commented 6 years ago

However, it seems your code assumes, that all variables are defined in vimrc

Aah thanks for spotting this is a typo.

Additionally 50% of the screen is maybe a little generous for g:scSplitSize as a default

Agreed. I'll change that to 30%

Apart from that, what has been bugging me (generally) is, that the sclang pane never gets closed, once one closes vim (or ends SC for that matter)

I don't know how to know the ID of the post window pane in tmux to be able to close it.. And my screen knowledge is close to null... If you have any tips for that I'm all ear :)

gusano commented 6 years ago

@dvzrv more info on the issues you brought up:

In tmux the post window pane ID will change every time the user add a split:

| foo.scd - ID: 1 | post - ID: 2 |
| foo.scd - ID: 1 | bar.scd - ID: 2 | post - ID: 3 |
| post - ID: 1 | foo.scd - ID: 2 | bar.scd - ID: 3 |

Finding out which pane is the post window one looks hacky. I'd say this is not such of a problem for now (not mentioning that I have no idea on screen).

In tmux: CTRL-A { or CTRL-A } :)

Look a little bit down in the file in Terminal Multiplexer Options :)

To sum it all, I think this PR is in a nicely usable state already, I don't think we should add too much complexity for now.

gusano commented 6 years ago

@dvzrv Pushed a fix for default values

gusano commented 6 years ago

@dvzrv bump : )

dvzrv commented 6 years ago

Sorry, I'm swamped with work until July... LAC coming up, too... Let's merge this!

gusano commented 6 years ago

@dvzrv thanks! I'd still need somebody to approve the PR as I cannot merge it myself :) cc @brianlheim

gusano commented 6 years ago

@dvzrv I fixed the condition you mentioned

gusano commented 6 years ago

@dvzrv @brianlheim it's been 7 months now... I think this would be a great addition to sc-3.10 :)

mossheim commented 6 years ago

I don't use scvim with a mux so any review I can give of this will be cursory, but I will look this weekend. I'm under the impression you've been using this for awhile with no issue, is that right @gusano?

gusano commented 6 years ago

I'm under the impression you've been using this for awhile with no issue, is that right @gusano?

7 months with no issue (but I only use tmux).. never went back to the non-multiplex version either :)

mossheim commented 6 years ago

Sorry for the delay!!! @dvzrv I've invited you to admin status on this repo so you can merge things in the future. It makes much more sense for you to be involved in reviewing PRs than it does me. :)

dvzrv commented 6 years ago

@brianlheim thanks! will do my best to keep things rolling! :)