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

Consider supporting redirecting stderr to stdout #113

Closed
lread opened this issue May 3, 2023 · 1 comment · Fixed by #115
Closed

Consider supporting redirecting stderr to stdout #113

lread opened this issue May 3, 2023 · 1 comment · Fixed by #115

Comments

@lread
Copy link
Contributor

lread commented May 3, 2023

Problem

Babashka/process currently easily supports controlling destinations for stderr and stdout.

But... it does not currently easily support merging stderr and stdout to a single output stream.
This can be very useful when you don't care about stderr vs stdout but do care about capturing all output in the order it occurs over time.

JDK Support

ProcessBuilder includes redirectErrorStream.
Docs on its effect are found in the ProcessBuilder class JavaDoc.

Explore Current Behaviour

Using the current release 0.4.16 of babashka/process, and a wee bb outerr.clj script to explore:

(println "I am written to stdout")

(binding [*out* *err*]
  (println "I am written to stderr"))

Today we can easily get stderr and stdout separately:

(require '[babashka.process :as p])

(-> (p/shell {:out :string :err :string} "bb outerr.clj")
    (select-keys [:out :err]))
;; => {:out "I am written to stdout\n", :err "I am written to stderr\n"}

But merging stderr to stdout, while possible, is a bit lower level than it could be:

(let [pb (p/pb {:out :string} "bb outerr.clj")
      pb-obj (:pb pb)]
  (.redirectErrorStream pb-obj true)
  (:out @(p/start pb)))
;; => "I am written to stdout\nI am written to stderr\n"

Proposed API change

@borkdude and I hashed out syntax on Slack and he came up with the succinct :err :out.
Nice!

With this new syntax, the lower-level example above could be re-expressed like so:

(-> (p/shell {:out :string :err :out} "bb outerr.clj")
    :out)
;; => "I am written to stdout\nI am written to stderr\n"

The :err :out syntax will be applicable to all API fns where :err is accepted in opts.

Next Steps

After @borkdude eyeballs the above, I'll proceed with a PR for review that includes both code and doc updates.

@borkdude
Copy link
Contributor

borkdude commented May 3, 2023

Eyeballed!

lread added a commit to lread/process that referenced this issue May 3, 2023
lread added a commit to lread/process that referenced this issue May 3, 2023
New tests launch clojure to as part of testing capturing stderr.
Adjusted CI to download clojure deps before running tests so we did not
pollute stderr with clojure's `Downloading: org/clojure...` stderr
messages.

Bumped GitHub action setup-clojure. Prior to bump installed clojure was
not launching in GitHub Actions default Windows shell.

Closes babashka#113
borkdude added a commit that referenced this issue May 4, 2023
Fixes #113

Co-authored-by: Michiel Borkent <michielborkent@gmail.com>
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 a pull request may close this issue.

2 participants