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

Cwd #7

Merged
merged 3 commits into from
Jun 10, 2018
Merged

Cwd #7

merged 3 commits into from
Jun 10, 2018

Conversation

ysimenya
Copy link

Hello Cédric,

I would like that "cwd" parameter from Popen be available for LocalShell objects.
This enhances code readability.

Regards,
Yves

Yves Simenya added 3 commits May 14, 2018 19:07
Rationnal:
==========

I found myself writing code like this:

    shell = LocalShell()
    os.chdir(first_custom_path)
    shell('first_command')
    os.chdir(second_custom_path)
    shell('second_command')

I didn't like it!
Shell is built upon Popen.
It's cwd paramater allows to set a custom path.
With this option avalaible on LocalShell object,
I can then change the code to:

    shell = LocalShell()
    shell('first_command', cwd=first_custom_path)
    shell('second_command', cwd=second_custom_path)

Better ;-)

Remark:
======

I was tempted to make cwd persitant so that code like this:

    shell = LocalShell(cwd=custom_path)
    shell('first_command')
    shell('second_command')

runs commands in "custom_path".
But I find it a bit hard to read.
I will have to go back and forth from object instantiation to shell command
to figure out from where those are run.
So I didn't go any further this way!
@meuter meuter merged commit 41a9689 into meuter:master Jun 10, 2018
@meuter
Copy link
Owner

meuter commented Jun 10, 2018

Hi Yves,

Thanks for the PR! I merged it, extended it so that cwd is also available in other kinds of shell. This is now included in the latest release (v2.2.0),

If you encounter any issue, let me know

-Cédric

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

Successfully merging this pull request may close these issues.

None yet

2 participants