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

more ergonomic stream redirection #37978

Merged
merged 27 commits into from
May 28, 2021
Merged

Conversation

jw3126
Copy link
Contributor

@jw3126 jw3126 commented Oct 10, 2020

Usually when I want to redirect stdout, I also want to redirect stderr. And often I want to redirect them to some kind of logfile.
This is quite a bit of boilerplate:

open("stdout.log", "w") do new_stdout
     open("stderr.log", "w" do new_stderr
         redirect_stdout(new_stdout) do
             redirect_stderr(new_stderr) do
                   f()
             end
        end
    end
end

With this PR it is just

redirect_stdio(f, stdout="stdout.txt", stderr="stderr.txt")

@jw3126
Copy link
Contributor Author

jw3126 commented Oct 10, 2020

CI fail looks related to PR. However I cannot reproduce this error locally. Ideas?

@JeffBezanson
Copy link
Member

What happens when you open the same file twice, and redirect stdout and stderr to the two descriptors?

@JeffBezanson JeffBezanson added the io Involving the I/O subsystem: libuv, read, write, etc. label Oct 12, 2020
@jw3126
Copy link
Contributor Author

jw3126 commented Oct 13, 2020

Ah, good spot. I need to make sure if stdout=stderr=/same/path that only a single io is used.

base/stream.jl Show resolved Hide resolved
@jw3126
Copy link
Contributor Author

jw3126 commented Oct 15, 2020

I think freebsd CI fail is unrelated and this is ready to be reviewed.

base/stream.jl Outdated Show resolved Hide resolved
@jw3126
Copy link
Contributor Author

jw3126 commented Nov 3, 2020

Can this be merged?

@StefanKarpinski
Copy link
Member

Bump.

@StefanKarpinski StefanKarpinski added the triage This should be discussed on a triage call label Nov 30, 2020
@jw3126
Copy link
Contributor Author

jw3126 commented Dec 1, 2020

@StefanKarpinski I am a bit surprised, that this needs triage. Anyway, I am glad that this PR gets some attention. How can I find out when the next triage session is?

@oscardssmith
Copy link
Member

In general, any user facing change needs triage. This doesn't mean it's expected to be controversial, just that we don't want to be adding features carelessly. Triage meetings are (almost) every week at 14:30 EST on Thursdays. You're welcome to come, but don't feel like you need to.

@jw3126
Copy link
Contributor Author

jw3126 commented Jan 12, 2021

Bump! Is there interest in this? If so I can resolve the merge conflicts.

@StefanKarpinski
Copy link
Member

Would be good to look at this on a triage call.

base/stream.jl Outdated Show resolved Hide resolved
base/stream.jl Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
@jw3126
Copy link
Contributor Author

jw3126 commented May 13, 2021

Can this be merged? Would love to have this in 1.7.

@jw3126
Copy link
Contributor Author

jw3126 commented May 21, 2021

bump

@jw3126
Copy link
Contributor Author

jw3126 commented May 25, 2021

Bump can this please be merged before 1.7 feature freeze? This PR was ready from my side before 1.6 feature freeze and it would be frustrating to miss 1.7 as well.

base/stream.jl Outdated Show resolved Hide resolved
@jw3126
Copy link
Contributor Author

jw3126 commented May 26, 2021

Merge :)?

@jw3126
Copy link
Contributor Author

jw3126 commented May 28, 2021

@KristofferC can you merge this?

@oscardssmith oscardssmith merged commit ecea238 into JuliaLang:master May 28, 2021
@jw3126
Copy link
Contributor Author

jw3126 commented May 28, 2021

I am glad, this could be merged before the feature freeze. Thanks, everybody!

simeonschaub added a commit to JuliaDiff/ChainRules.jl that referenced this pull request May 28, 2021
simeonschaub added a commit to JuliaDiff/ChainRules.jl that referenced this pull request May 28, 2021
@simeonschaub simeonschaub removed the forget me not PRs that one wants to make sure aren't forgotten label May 29, 2021
shirodkara pushed a commit to shirodkara/julia that referenced this pull request Jun 9, 2021
Co-authored-by: Jeff Bezanson <jeff.bezanson@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Co-authored-by: Kristoffer Carlsson <kcarlsson89@gmail.com>
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
Co-authored-by: Jeff Bezanson <jeff.bezanson@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Co-authored-by: Kristoffer Carlsson <kcarlsson89@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io Involving the I/O subsystem: libuv, read, write, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants