Skip to content

Add Process.capture#16773

Merged
straight-shoota merged 11 commits intocrystal-lang:masterfrom
straight-shoota:feat/process.capture
Mar 28, 2026
Merged

Add Process.capture#16773
straight-shoota merged 11 commits intocrystal-lang:masterfrom
straight-shoota:feat/process.capture

Conversation

@straight-shoota
Copy link
Copy Markdown
Member

@straight-shoota straight-shoota commented Mar 23, 2026

Implement Process.capture, Process.capture? and Process.capture_result from RFC 0025.

This implements the basic capture behaviour, which may be enhanced in the future.
In particular, this patch misses truncation of the captured error stream which will be a follow-up (#16774).

Resolves #7171

@straight-shoota
Copy link
Copy Markdown
Member Author

straight-shoota commented Mar 24, 2026

This interpreter failure is pretty bizarre:

In spec\std\process_spec.cr:835:5

 835 | it "doesn't capture closed stderr" do
       ^
Error: compiling -> do

  result = Process.capture_result(to_ary(shell_command("1>&2 echo hello")), :close)
  result.status.success?.should(be_true)
  result.output?.should(eq(""))
  result.error?.should(be_nil)
end


In spec\std\process_spec.cr:836:88

 836 | result = Process.capture_result(to_ary(shell_command("1>&2 echo hello")), error: :close)
                                                                                        ^
Error: BUG: missing downcast_distinct from Symbol to Process::Redirect (Crystal::SymbolType to Crystal::EnumType)
  1. The example right before it uses exactly the same syntax, just with the output parameter. Both output and error have the same type restriction, though (Stdio). The other one should be analysed first, so why does it not fail?
  2. It only fails on Windows, but not on Linux. The interpreter should behave exactly the same on both targets.Ah, but we're skipping process specs in the interpreter on anything but Windows because SIGCHLD doesn't work

@ysbaddaden ysbaddaden mentioned this pull request Mar 24, 2026
@ysbaddaden
Copy link
Copy Markdown
Collaborator

I'm having second thoughts on #capture_result. Looking at the implementation, I feel like it's mostly duplicating #run with implicit output = IO::Memory.new and error = IO::Memory.new and that's it 🤔

@straight-shoota
Copy link
Copy Markdown
Member Author

Yeah, these methods are all essentially convenience wrappers. Process.run(...) itself is just a shortcut for Process.new(...).wait.
But the important thing is that they extract that meaning for a commonly used behaviour and give it a name. Maybe the name capture_result is not ideal. But the method certainly does more than I would want to repeat on every call site.

@straight-shoota straight-shoota added this to the 1.20.0 milestone Mar 26, 2026
@straight-shoota straight-shoota merged commit 7b5e7ae into crystal-lang:master Mar 28, 2026
48 checks passed
@straight-shoota straight-shoota deleted the feat/process.capture branch March 28, 2026 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A safe alternative for capturing child process output

2 participants