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

IO: remove flush_on_newline and only sync on TTY devices #7470

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 0 additions & 22 deletions spec/std/io/buffered_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -262,28 +262,6 @@ describe "IO::Buffered" do
str.to_s.should eq("hello" * 10_000)
end

it "flushes on \n" do
str = IO::Memory.new
io = BufferedWrapper.new(str)
io.flush_on_newline = true

io << "hello\nworld"
str.to_s.should eq("hello\n")
io.flush
str.to_s.should eq("hello\nworld")
end

it "doesn't write past count" do
str = IO::Memory.new
io = BufferedWrapper.new(str)
io.flush_on_newline = true

slice = Slice.new(10) { |i| i == 9 ? '\n'.ord.to_u8 : ('a'.ord + i).to_u8 }
io.write slice[0, 4]
io.flush
str.to_s.should eq("abcd")
end

describe "sync" do
it "syncs (write)" do
str = IO::Memory.new
Expand Down
1 change: 0 additions & 1 deletion src/http/server/response.cr
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ class HTTP::Server
@in_buffer_rem = Bytes.empty
@out_count = 0
@sync = false
@flush_on_newline = false
@chunked = false
@closed = false
end
Expand Down
26 changes: 0 additions & 26 deletions src/io/buffered.cr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ module IO::Buffered
@out_count = 0
@sync = false
@read_buffering = true
@flush_on_newline = false

# Reads at most *slice.size* bytes from the wrapped `IO` into *slice*.
# Returns the number of bytes read.
Expand Down Expand Up @@ -123,17 +122,6 @@ module IO::Buffered
return unbuffered_write(slice)
end

if flush_on_newline?
index = slice[0, count.to_i32].rindex('\n'.ord.to_u8)
if index
flush
index += 1
unbuffered_write slice[0, index]
slice += index
count -= index
end
end

if count >= BUFFER_SIZE
flush
return unbuffered_write slice[0, count]
Expand Down Expand Up @@ -161,20 +149,6 @@ module IO::Buffered
end
out_buffer[@out_count] = byte
@out_count += 1

if flush_on_newline? && byte === '\n'
flush
end
end

# Turns on/off flushing the underlying `IO` when a newline is written.
def flush_on_newline=(flush_on_newline)
@flush_on_newline = !!flush_on_newline
end

# Determines if this `IO` flushes automatically when a newline is written.
def flush_on_newline?
@flush_on_newline
end

# Turns on/off `IO` **write** buffering. When *sync* is set to `true`, no buffering
Expand Down
6 changes: 5 additions & 1 deletion src/io/file_descriptor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ class IO::FileDescriptor < IO
clone_fd = LibC.open(path, LibC::O_RDWR)
return new(fd, blocking: true) if clone_fd == -1

new(clone_fd).tap(&.close_on_exec = true)
# We don't buffer output for TTY devices to see their output right away
io = new(clone_fd)
io.close_on_exec = true
io.sync = true
io
end

def blocking
Expand Down
19 changes: 8 additions & 11 deletions src/kernel.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,12 @@
# The standard output file descriptor.
#
# Typically used to output data and information.
#
# NOTE: Gets flushed when a newline is written.
STDOUT = IO::FileDescriptor.new(1).tap { |f| f.flush_on_newline = true }
STDOUT = IO::FileDescriptor.new(1)

# The standard error file descriptor.
#
# Typically used to output error messages and diagnostics.
#
# NOTE: Gets flushed when a newline is written.
STDERR = IO::FileDescriptor.new(2).tap { |f| f.flush_on_newline = true }
STDERR = IO::FileDescriptor.new(2)
{% else %}
require "c/unistd"

Expand All @@ -25,15 +21,17 @@
#
# Typically used to output data and information.
#
# NOTE: Gets flushed when a newline is written.
STDOUT = IO::FileDescriptor.from_stdio(1).tap { |f| f.flush_on_newline = true }
# When this is a TTY device, `sync` will be true for it
# at the start of the program.
STDOUT = IO::FileDescriptor.from_stdio(1)

# The standard error file descriptor.
#
# Typically used to output error messages and diagnostics.
#
# NOTE: Gets flushed when a newline is written.
STDERR = IO::FileDescriptor.from_stdio(2).tap { |f| f.flush_on_newline = true }
# When this is a TTY device, `sync` will be true for it
# at the start of the program.
STDERR = IO::FileDescriptor.from_stdio(2)
{% end %}

# The name, the program was called with.
Expand Down Expand Up @@ -118,7 +116,6 @@ end
# See also: `IO#print`.
def print(*objects : _) : Nil
STDOUT.print *objects
STDOUT.flush
end

# Prints a formatted string to `STDOUT`.
Expand Down
4 changes: 2 additions & 2 deletions src/process.cr
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,8 @@ class Process
end

ORIGINAL_STDIN = IO::FileDescriptor.new(0, blocking: true)
ORIGINAL_STDOUT = IO::FileDescriptor.new(1, blocking: true).tap { |f| f.flush_on_newline = true }
ORIGINAL_STDERR = IO::FileDescriptor.new(2, blocking: true).tap { |f| f.flush_on_newline = true }
ORIGINAL_STDOUT = IO::FileDescriptor.new(1, blocking: true)
ORIGINAL_STDERR = IO::FileDescriptor.new(2, blocking: true)

# :nodoc:
protected def self.exec_internal(command, args, env, clear_env, input, output, error, chdir) : NoReturn
Expand Down