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

made StdinDataRedirection compatible with modifiers writing to stdout #605

Merged
merged 2 commits into from
Jun 24, 2022

Conversation

jesteria
Copy link
Contributor

I.e. fixed:

(cat << "meow") & FG

Modifiers writing to stdout – e.g.: FG, TEE, RETCODE(FG=True) and
TF(FG=True) – invoke commands with the specification stdin=None.
This is handled correctly by BaseCommand; however, StdinDataRedirection
misinterprets this as a further attempt to specify stdin (i.e. to
specify something rather than nothing).

This change brings StdinDataRedirection in line with its base class,
ignoring None as well as PIPE.

(Though a technical detail, the class would also have failed any
invocation setting stdin=PIPE, along the lines of: "TypeError: got
multiple values for keyword argument 'stdin'". The kwargs passed to the
wrapped command's popen are corrected as well.)

resolves #604

_I.e._ fixed:

    (cat << "meow") & FG

Modifiers writing to stdout -- _e.g._: FG, TEE, RETCODE(FG=True) and
TF(FG=True) -- invoke commands with the specification `stdin=None`.
This is handled correctly by BaseCommand; however, StdinDataRedirection
misinterprets this as a further attempt to specify stdin (_i.e._ to
specify _something_ rather than _nothing_).

This change brings StdinDataRedirection in line with its base class,
ignoring `None` as well as `PIPE`.

(Though a technical detail, the class would also have failed any
invocation setting `stdin=PIPE`, along the lines of: "TypeError: got
multiple values for keyword argument 'stdin'". The kwargs passed to the
wrapped command's `popen` are corrected as well.)

resolves tomerfiliba#604
@coveralls
Copy link

coveralls commented Jun 24, 2022

Coverage Status

Coverage increased (+0.09%) to 83.673% when pulling ac15f1c on jesteria:604-fix-stdin-data-modifiers into 5fa1452 on tomerfiliba:master.

@henryiii henryiii closed this Jun 24, 2022
@henryiii henryiii reopened this Jun 24, 2022
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@henryiii henryiii merged commit 16113fd into tomerfiliba:master Jun 24, 2022
@henryiii
Copy link
Collaborator

Thanks!!!

@jesteria jesteria deleted the 604-fix-stdin-data-modifiers branch January 4, 2023 21:59
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.

StdinDataRedirection commands fail upon modification (TEE, FG, etc.)
3 participants