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

Environment setting support. #137

Merged
merged 4 commits into from Feb 29, 2016
Merged

Conversation

@tasdomas
Copy link
Contributor

@tasdomas tasdomas commented Feb 11, 2016

Added new config directive 'environment', that lists environment variables to be set on the tmux session. Needs tests and documentation, but feedback if the general approach seems sensible would be very useful.

@tasdomas tasdomas force-pushed the tasdomas:set_environment_support branch from 280bfa6 to db1fe28 Feb 12, 2016
if isinstance(proc.stderr, list) and len(proc.stderr) == int(1):
proc.stderr = proc.stderr[0]
raise ValueError('tmux set-environment stderr: %s' % proc.stderr)

This comment has been minimized.

@tony

tony Feb 12, 2016
Member

thanks for the documentation

@@ -186,7 +186,7 @@ def test_focus_pane_index(self):
self.assertNotEqual(w.get('window_name'), 'man')

pane_path = '/usr'
for i in range(10):
for i in range(20):

This comment has been minimized.

@tony

tony Feb 12, 2016
Member

Fine

This comment has been minimized.

@tasdomas

tasdomas Feb 21, 2016
Author Contributor

This test fails for me at a noticeable frequency with only 10 retries.

@tony
Copy link
Member

@tony tony commented Feb 12, 2016

Good call on this @tasdomas

I opened a question at tmux/tmux#304 to double check if set-environment with -g is just server only. If so we may want to chuck a set_environment to the server as well.

Tests / documentation / examples for this would be a good idea. It seems like a feature that'd be helpful in many situations.

@tasdomas
Copy link
Contributor Author

@tasdomas tasdomas commented Feb 12, 2016

@tony, cool - I'll update the pull request and submit it for your consideration.

@tony
Copy link
Member

@tony tony commented Feb 18, 2016

We probably should also add set_environment at the server level. -g / "global" is the same as server.

Any updates on this @tasdomas? If you're having trouble / time constraints let me know.

@tasdomas
Copy link
Contributor Author

@tasdomas tasdomas commented Feb 18, 2016

@tony, working on it - currently trying to figure out a way to test the functionality. Once that's done, everything else is relatively easy. Should have a PR by next week - a little busy at the moment.

@tony
Copy link
Member

@tony tony commented Feb 18, 2016

No problem, if you need help or need me to finish the test part, I'm happy to.

@tasdomas
Copy link
Contributor Author

@tasdomas tasdomas commented Feb 21, 2016

Hello, @tony, I've updated the pull request. Extended the functionality to support setting global variables on the server as well, added some docs and enabled environment variable expansion for the variables (so setting something like PATH would be less tedious). Looking forward to your reviews.

@tasdomas tasdomas force-pushed the tasdomas:set_environment_support branch from cd24948 to dacb70f Feb 22, 2016
@tasdomas tasdomas force-pushed the tasdomas:set_environment_support branch from dacb70f to aafa0de Feb 23, 2016
tony added a commit that referenced this pull request Feb 29, 2016
@tony tony merged commit 460672d into tmux-python:master Feb 29, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.7%) to 63.806%
Details
@tony
Copy link
Member

@tony tony commented Feb 29, 2016

@tasdomas: Pardon the delay. Merged.

This looks good to me and is useful enough of a feature to bump the version up.

Uploaded to pypi as 0.11.0.

@coveralls
Copy link

@coveralls coveralls commented Feb 2, 2018

Coverage Status

Coverage decreased (-23.5%) to 39.531% when pulling aafa0de on tasdomas:set_environment_support into 0a8568d on tony:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants