laloch / xontrib-fzf-widgets

Set of fzf widgets for xonsh
GNU General Public License v3.0
33 stars 18 forks source link

Fix: Use copy of os.environ to avoid mutating env #21

Open pedrovhb opened 2 years ago

pedrovhb commented 2 years ago

Hi! Thanks for maintaining this, it's quite useful.

I found that the xontrib modifies environment variables in a way that is probably not desired.

I set my $FZF_DEFAULT_OPTS to the following:

$FZF_DEFAULT_OPTS="-m --reverse --height=40 --preview 'bat --color=always --style=numbers --line-range=:500 {}'"

This uses bat to display a nice file preview:

image

The issue is that in a xonsh session, after using the file searcher (which modifies $FZF_DEFAULT_OPTS to include the default args), the args are carried over to the command history search. It then tries to fetch a preview for commands, which doesn't work, and I imagine could lead to unintended commands being run.

This PR modifies the code to use dict(os.environ) instead of os.environ directly, so that we can modify a copy without affecting env vars. Although os.environ is its own class and not a dict, subprocess.run takes a mapping as per the documentation:

If env is not None, it must be a mapping that defines the environment variables for the new process

And I've tested this to verify it now works as expected.