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

Allow multiple configuration files on command line #135

Merged
merged 4 commits into from Jan 30, 2016

Conversation

Projects
None yet
2 participants
@madprog
Contributor

madprog commented Jan 26, 2016

Fixes #133 as far as I'm concerned

I would like to write a test for this, but I haven't found any test example running the command line. Or did I miss it?

Allow multiple configuration files on command line
Fixes #133 as far as I'm concerned
@tony

This comment has been minimized.

Member

tony commented Jan 26, 2016

I'm not certain if this wouldn't inadvertently cause other commands to break potentially. I'm willing to test and QA this thoroughly by hand if thats what required.

However, have you considered this:

tmuxp load ~/path/to/config1.yaml -d && tmuxp load ~/path/to/config2.yaml -d

You can optionally remote the -d if you would like to attach the last one automatically:

tmuxp load ~/path/to/config1.yaml -d && tmuxp load ~/path/to/config2.yaml

Does this achieve the affect you'd want? IMO It may better to make the cli portion compatible with xargs so sessions could loaded in parallel.

@madprog

This comment has been minimized.

Contributor

madprog commented Jan 26, 2016

Ok, I did not think about -d.

However, attaching to 2 or more sessions at the same time does not makes sense. So we could document that in absence of -d, only the session described by the last configuration file would be attached.
Or that, in the case of multiple files, -d is implied for all of them.
What do you think?

About compatibility with xargs, do you mean that tmuxp would use xargs internally, or that a user could run tmuxp with xargs?
Because, in the second case, without a -L 1 or -n 1 flag, it would just call tmuxp with multiple arguments.

Instead of xargs, the workaround I'm using for the moment is:

for conf in ~/.config/tmux/autorun.d/*.yaml; do
    tmuxp load -d $conf
done
@tony

This comment has been minimized.

Member

tony commented Jan 26, 2016

About compatibility with xargs, do you mean that tmuxp would use xargs internally, or that a user could run tmuxp with xargs?

I also played around (while using -L and -n) because seeking to make a one-liner out of it, however didn't get the result I intended. This leaves open the possibility for using a for loop.

However, attaching to 2 or more sessions at the same time does not makes sense. So we could document that in absence of -d, only the session described by the last configuration file would be attached.

Definitely feel free to document and add anything that'd be helpful. Including -d

However, current behavior in this PR (with the absence of -d) will load the first session, and only continue loading the second config after the tmux client in that tty is detached. Conveniently, if the tmux session already exists, it will iterate through one by one and prompt them still.

So that leaves us with a bit of bikeshedding. I think we could by default do what you mentioned documenting, we would mark all but the last config as detached by default.

Load first configurations to the background
Only the last one will be attached if -d flag is not specified
@tony

This comment has been minimized.

Member

tony commented Jan 27, 2016

Works for me.

Care to document the behavior of -d and loading multiple configs?

madprog added some commits Jan 27, 2016

Rename for-loop variable name to avoid conflict
Local `config` was hidding global import `config`, used later in the
function.

tony added a commit that referenced this pull request Jan 30, 2016

Merge pull request #135 from madprog/pr-multiple-configurations
Allow multiple configuration files on command line

@tony tony merged commit 3c71b20 into tmux-python:master Jan 30, 2016

1 of 2 checks passed

coverage/coveralls Coverage decreased (-0.3%) to 63.073%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment