Add new-window command functionality

[ikirudennis]

See issue #142

[coveralls]

Coverage decreased (-0.01%) to 63.794% when pulling f4aef07 on ikirudennis:new-window-command into f0e5600 on tony:master.

[coveralls]

Coverage decreased (-0.01%) to 63.794% when pulling 8a3cff8 on ikirudennis:new-window-command into f0e5600 on tony:master.

[ikirudennis]

I tried to see if I could figure out how to add a test for it, but it didn't seem to work. :\

[tony]

Can you add the docstrings (:param window_index: description, so on) for these params?

[kmactavish]

The code you added will only run the command once on building the workspace, after that, any new panes/windows won't be run with this command. This functionality was already available though shell_command_before.

Maybe you want to add tmux key bindings? That will let any new windows/panes you open run a given command when they open.

In order to have the whole session live inside a virtual environment, including any new windows, you can use tmuxp's before_script.

[tony]

The code you added will only run the command once on building the workspace, after that, any new panes/windows won't be run with this command.

Can you clarify who / what this refers to? Just double checking

@kmactavish giving this and #146 a look tomorrow/sat. You're on a roll!

[ikirudennis]

I understand what you're saying, but this code is the functionality I'm looking for. I have a configuration where there are some windows which should load the virtual environment, and others which don't. (In my case, it's a directory for a static_files system separate from the virtual environment's project directory.) Adding a before_script for this would be unnecessary. To me, it just seemed like an oversight to cover all the other functionality of the new-window command but leave out the ability to specify a command to run in it.

[coveralls]

Coverage decreased (-0.01%) to 63.794% when pulling b438c3b on ikirudennis:new-window-command into f0e5600 on tony:master.

[kmactavish]

Hmm, I see. Did you want the functionality for the API, or for building a session? I think it makes sense for the API, but maybe not the WorkspaceBuilder.

Right now, in the WorkspaceBuilder, the window_command is the default (your shell), which then has the shell_command from the first pane sent to it via send-keys. So the effective window_command is just the first pane's shell_command running inside of bash.

I see three potential problems for using a window_command in the config file:

1. What would happen in the WorkspaceBuilder if window_command was ls, or something very transient, and the window closed before the other panes could be created? They might get created in the previous window?
2. It would be confusing that the first pane's shell_command doesn't get run; rather, the keys get sent to whatever window_command is---I expect you'll see text appearing if window_command is an editor, and the first pane has a shell_command.
3. New users likely won't be aware that when the window_command ends, that pane/window dissapears, they would expect it to work like the pane's shell_command and drop back to a shell (which it won't): try running tmux new-window "ls; sleep 1" inside a session.

@tony P.S. that comment was me missunderstanding what @ikirudennis was trying to accomplish, which I may still be doing 😉 .

[kmactavish]

BTW, what is the window_command you want to run? If it's loading the virtualenv, it seems well suited to the window's shell_command_before, which just gets prepended to the shell_command of all its children panes (only at the window level, unlike before_script).

[ikirudennis]

Yes, window_command could be confusing for the uninitiated. However, as you point out, it is part of tmux functionality. And I can understand how it might be troublesome explaining to a user that window_command may have unintended consequences, and that what might appear to be a bug with tmuxp is actually everything working correctly. Perhaps we can put our heads together on explaining how it works in documentation and any helpful error messages returned to the user. If it's any consolation, I feel the tmux documentation on new-window command is kind of thin.

I did a test with a command which exits quickly, and yes it produces an error.

In my setup, I'm loading a virtualenvironment via pew. Pew works a little differently in that it starts a new shell instead of injecting the virtualenv into an existing shell. The reason I dislike shell_command_before or any of the other options I've tried is because it leaves a shell open after I exit the environment. To me, new-window -n name 'pew workon venv_name' is just a cleaner way to start the environment, and I know that when I exit it, I intend to exit the entire shell. It's how I was doing things before I found tmuxp, and it just seemed like a missing feature. But I hope I'm not being troublesome like "longtimeuser4" in XKCD: Workflow. 😄

[kmactavish]

Gotcha. You've won me over, and I like the fact that it's an undocumented feature---there is no example for it, so it won't be confusing for new users. Since people will only find it if they look at the unit test or in workspace builder, maybe comment in both those places its intended use and perils 😉 .

[kmactavish]

P.S. window_shell might be more accurate than window_command.

[coveralls]

Coverage increased (+0.2%) to 63.992% when pulling 2079cd1 on ikirudennis:new-window-command into f0e5600 on tony:master.

[tony]

These are intended to be integration tests in the truest sense of the word. See .travis.yml.

We have a minimum tmux version we officially support (though we can bump it of course). I'd like to be sure that this behavior would work on older versions.

Try something like this:

diff --git a/tmuxp/testsuite/workspacebuilder.py b/tmuxp/testsuite/workspacebuilder.py
index 253834e..a8b79dc 100644
--- a/tmuxp/testsuite/workspacebuilder.py
+++ b/tmuxp/testsuite/workspacebuilder.py
@@ -266,7 +266,7 @@ class WindowOptions(TmuxTestCase):
           - pane
           - pane
           window_name: editor
-          window_shell: test_command
+          window_shell: top
         """
         s = self.session
         sconfig = kaptan.Kaptan(handler='yaml')
@@ -277,7 +277,14 @@ class WindowOptions(TmuxTestCase):

         for w, wconf in builder.iter_create_windows(s):
             if 'window_shell' in wconf:
-                self.assertEqual(wconf['window_shell'], text_type('test_command'))
+                self.assertEqual(wconf['window_shell'], text_type('top'))
+            for i in range(10):
+                self.session.server._update_windows()
+                if w['window_name'] != 'top':
+                    break
+                time.sleep(.2)
+
+            self.assertNotEqual(w.get('window_name'), text_type('top'))


 class EnvironmentVariables(TmuxTestCase):
@@ -305,7 +312,7 @@ class EnvironmentVariables(TmuxTestCase):

         self.assertEqual('BAR', self.session.show_environment('FOO'))
         self.assertEqual('/tmp', self.session.show_environment('PATH'))
-        
+
 class WindowAutomaticRename(TmuxTestCase):

     yaml_config = """

[coveralls]

Coverage increased (+0.3%) to 64.14% when pulling 7eb9441 on ikirudennis:new-window-command into f0e5600 on tony:master.

[tony]

That was odd.

Restarted the process, and its ok now.

[tony]

Can you rebase against master and squash?

[kmactavish]

@tony

That was odd.
Restarted the process, and its ok now.

Hmm, that is strange. It is correct in identifying that as a test failure, echo isMissing is not supposed to be added to the bash history.

The only thing I can think of is that when the pane was captured, it hadn't finished being initialized, and the history_cmd erroneously refers to the same line as sent_cmd. A potential patch is increasing the timeout before capturing the pane (below). I tried to find a more robust, synchronous way of capturing the bash history, but this was all that I found worked.

If that's seen again, I can submit this patch.

diff --git a/tmuxp/testsuite/workspacebuilder.py b/tmuxp/testsuite/workspacebuilder.py
index 79b5329..206f52e 100644
--- a/tmuxp/testsuite/workspacebuilder.py
+++ b/tmuxp/testsuite/workspacebuilder.py
@@ -238,7 +238,7 @@ class SuppressHistoryTest(TmuxTestCase):

         builder = WorkspaceBuilder(sconf=sconfig)
         builder.build(session=self.session)
-        time.sleep(0.2) # give .bashrc, etc. time to load
+        time.sleep(0.3) # give .bashrc, etc. time to load

         s.server._update_windows()
         for w in s.windows:
@@ -250,7 +250,7 @@ class SuppressHistoryTest(TmuxTestCase):
                 # Print the last-in-history command in the pane
                 self.session.cmd('send-keys', ' fc -ln -1')
                 self.session.cmd('send-keys', 'Enter')
-                time.sleep(0.01) # give fc time to run
+                time.sleep(0.1) # give fc time to run

                 # Get the contents of the pane
                 self.session.cmd('capture-pane')

References #146 .

[tony]

@kmactavish Yep, that'll give travis a bit more breathing room.

Can you rebase against master and squash too? Once that's good we can merge

[ikirudennis]

I hope I did that right.

[coveralls]

Coverage increased (+0.3%) to 64.14% when pulling e015b71 on ikirudennis:new-window-command into f0e5600 on tony:master.

[tony]

re: squashing, I meant to direct that to @ikirudennis.

It seems GH has a "squash and merge" available after you hit "Merge Pull Request".