GeezerVison is showing github.com — Implements `if` conditions for pane and window by soraxas · Pull Request #942 · tmux-python/tmuxp
Conversation
Closes #741
This implements both shell and python conditions.
-
if can be a:
- string or, (short-hand for
shell key)
- dict that contains
-
A python key will be evaluated as a python expression, and will test for its pythonic truthfulness
-
A shell key will test as a shell variable and any of the following will be evaluated to true: ("y", "yes", "1", "on", "true", "t") (case-insensitivity)
session_name: if conditions test
environment:
Foo: 'false'
show_htop: 'true'
windows:
# the following would not shows up as it evaluates to false
- window_name: window 1 ${ha} $Foo
if:
shell: ${Foo}
panes:
- shell_command:
- echo "this shouldn't shows up"
- echo neither should this $Foo
- window_name: window 2
panes:
# should not shows up
- if:
python: 1+1==3
shell_command:
- echo the above is a false statement
# no if conditions
- shell_command:
- echo no condition
- python -m http.server
# display by default, but can be disabled by running `show_htop=false tmuxp load .....`
- if: ${show_htop}
shell_command:
- echo the above is a true statement (by default), but can be disabled on-demand
- htop
In the example, by default, show_htop=true, but since it's a shell variable, it can be override by user. Hence, use can on-deamnd customise their pane/window configuration by
tmuxp load examples/if-conditions.yaml
and
show_htop=false tmuxp load examples/if-conditions.yaml
which will have different behaviour, for different use-cases
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
All committers have signed the CLA.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice documentation, example, well annotated!
Allowed CI to run.
-
Rebase to get the latest updates
-
Adding a test would make it 5-stars (example: EXAMPLE_PATH w/ blank-panes.yaml in test_builder.py::test_blank_pane_spawn)
-
ruff check has some complaints (job)
You may get automated fixes from something like ruff check --select ALL . --fix --unsafe-fixes --preview --show-fixes --ignore T201 --ignore F401 --ignore PT014 --ignore RUF100; ruff format ..
(While I actually prefer your styling better than what ruff would do above, I use ruff to enforce consistency at scale)
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
I've expanded the approach, where shell now refers to actual shell expressions, for expressions like:
or
echo ${MY_ENV} | grep foo
The shell_var now refers to just testing for variables.
python now supports statements (using eval only supports expression, so 1+2>=3 works but not import sys; 1+2>=3). Instead, We can use exec and extract the results (from the last expression) afterwards.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
tony
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewinding a bit, it this safe?
Can you think of any other software projects that have declarative configurations directly execute code? 1
I suppose:
- CI: Jenkins, CircleCI, GitHub actions
- Container software: Docker (perhaps the CI systems could be considered a subset of that)
- Configuration managers: Ansible, Salt
- Shell script: Run commands like bashrc, zshrc
- Vim: Vim configuration
Add second qualifier on top of that: when the code is also executed in an if expression.
Is eval() or subprocess.run(..., shell=True) too permissive for an if?
I can't tell yet. I would like to see more examples from other projects to see their best practices.
Alternative approach?
Could you make a second PR with a more limited if that avoid eval / subprocess passthroughs?
operator expressions
One way, it could have something wrapping operator:
if:
- a: ${SHELL}
op: eq
b: 'bash'
Where a and b could be replaced by envionmental variables via os.path.expandvars() and op could accept a method of operator.
Separate script
To make the conditional more explicit, they can be separate shell scripts and run though a subprocess.Popen, similar to run_before_script(), but they're explicit commands and piped through libtmux.
Anything else?
Anything that could make if flexible, yet constrained?
Footnotes
-
The shell_commands in tmuxp configurations are typed into a pty / tmux pane, to be fair. ↩
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
soraxas
commented
Aug 9, 2024
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
Yes I also partially agree that the use of eval and supprocess tends to be discourage (especially when we are executing unsanitised user input); therefore I did put comments on stating the potential danger within this approach in previous revision.
However, the more I think about it, the more I reckon that the nature of tmuxp or other similar tools like tmuxinator might render these concerns a bit pointless. The fact is, tmuxp will already be executing arbitrary commands based on the given list of shell_command.
From my point of view,
- The use of
subprocess to evaluate shell output shouldn't be of concern, as it's not too different than what tmuxp is designed to do (execute arbitrary commands to setup workspace).
- My only potential concern is using
eval / exec for python statements, for the fact that they are executing inside the python interpreter, and they might change the internal state inside tmuxp (e.g. they can modify some variables within tmuxp).
- Perhaps this concern can be eliminated by evaluating the python statement inside a subprocess as well (to isolate the effect)? We can probably re-export the environment variable to the subprocess Python to achieve the same effect.
RE:
Can you think of any other software projects that have declarative configurations directly execute code? 1
Aside from the examples that you have given (many of which just execute any commands given without any security measure), my experience with direnv is that it requires user to explicitly opt-in when encountering a new .envrc file.
- However, the main difference is that
direnv auto-load files when cd-ing, so it provides user a chance to manually review the file before auto-loading (it hashes the file to monitor changes to .envrc file), so even if they had just cloned some repository online direnv would prompt user before auto-loading it.
- In
tmuxp's case, users manually issues a load command, so they should be in-charge of reviewing the file (if it comes from internet; the same pretty goes for any scripts that comes from the internet)?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This branch has not been deployed
No deployments
Reviewers
tony
tony requested changes
Requested changes must be addressed to merge this pull request.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|