Skip to content

Commit

Permalink
[builtin/read] Fix read -d behavior so that newline isn't special.
Browse files Browse the repository at this point in the history
Also fix the exit code when the delimiter isn't found.

Fixes issue #694.

Also refactor Oil's 'getline' builtin.
  • Loading branch information
Andy Chu committed Apr 7, 2020
1 parent f0fe30c commit 16bbec6
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 22 deletions.
8 changes: 4 additions & 4 deletions core/process_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,16 @@ def testStdinRedirect(self):
redirect_arg.Path(PATH))

fd_state.Push([r], waiter)
line1 = builtin_misc.ReadLineFromStdin(None)
line1, _ = builtin_misc.ReadLineFromStdin('\n')
fd_state.Pop()

fd_state.Push([r], waiter)
line2 = builtin_misc.ReadLineFromStdin(None)
line2, _ = builtin_misc.ReadLineFromStdin('\n')
fd_state.Pop()

# sys.stdin.readline() would erroneously return 'two' because of buffering.
self.assertEqual('one\n', line1)
self.assertEqual('one\n', line2)
self.assertEqual('one', line1)
self.assertEqual('one', line2)

def testProcess(self):

Expand Down
26 changes: 22 additions & 4 deletions oil_lang/builtin_oil.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from frontend import args
from frontend import match
from mycpp.mylib import tagswitch
from osh import builtin_misc # ReadLineFromStdin

import yajl
import posix_ as posix
Expand Down Expand Up @@ -355,6 +354,26 @@ def Run(self, cmd_val):
return 0


def _ReadLine():
# type: () -> str
"""Read a line from stdin.
TODO: use a more efficient function in C
"""
chars = []
while True:
c = posix.read(0, 1)
if not c:
break

chars.append(c)

if c == '\n':
break

return ''.join(chars)


GETLINE_SPEC = args.OilFlags()
GETLINE_SPEC.Flag('-cstr', args.Bool,
help='Decode the line in CSTR format')
Expand Down Expand Up @@ -387,9 +406,8 @@ def Run(self, cmd_val):
if next_arg is not None:
raise args.UsageError('got extra argument', span_id=next_spid)

# TODO: use a more efficient function in C
line = builtin_misc.ReadLineFromStdin(None)
if not line: # EOF
line = _ReadLine()
if len(line) == 0: # EOF
return 1

if not arg.end:
Expand Down
31 changes: 17 additions & 14 deletions osh/builtin_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
except ImportError:
help_index = None

from typing import Any, Optional, IO, TYPE_CHECKING
from typing import Tuple, Any, Optional, IO, TYPE_CHECKING
if TYPE_CHECKING:
from _devbuild.gen.runtime_asdl import value__Str
from core.pyutil import _FileResourceLoader
Expand Down Expand Up @@ -160,22 +160,26 @@ def _AppendParts(s, spans, max_results, join_next, parts):
# NOTE that dash, mksh, and zsh all read a single byte at a time. It appears
# to be required by POSIX? Could try libc getline and make this an option.
def ReadLineFromStdin(delim_char):
# type: (Optional[str]) -> str
"""Read a line, or read up until delim_char if set."""
# type: (Optional[str]) -> Tuple[str, bool]
"""Read a portion of stdin.
If delim_char is set, read until that delimiter, but don't include it.
If not set, read a line, and include the newline.
"""
found_delim = False
chars = []
while True:
c = posix.read(0, 1)
if not c:
break

if c == delim_char:
found_delim = True
break

chars.append(c)

if c == '\n':
break
return ''.join(chars)
return ''.join(chars), found_delim


class Read(object):
Expand Down Expand Up @@ -239,24 +243,23 @@ def Run(self, cmd_val):
else:
delim_char = '\0' # -d '' delimits by NUL
else:
delim_char = None # read a line
delim_char = '\n' # read a line

# We have to read more than one line if there is a line continuation (and
# it's not -r).
parts = []
join_next = False
while True:
line = ReadLineFromStdin(delim_char)
line, found_delim = ReadLineFromStdin(delim_char)
#log('LINE %r', line)
if not line: # EOF
if len(line) == 0: # EOF
status = 1
break

if line.endswith('\n'): # strip trailing newline
line = line[:-1]
status = 0
else:
# odd bash behavior: fail even if we can set variables.
status = 0
if not found_delim:
# odd bash behavior: fail if no newline, even though we're setting
# variables.
status = 1

spans = self.splitter.SplitForRead(line, not arg.r)
Expand Down
29 changes: 29 additions & 0 deletions spec/builtin-io.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,35 @@ v1= v2=
v1= v2= v3=
## END

#### read -rd
read -rd '' var <<EOF
foo
bar
EOF
echo "$var"
## STDOUT:
foo
bar
## END
## N-I dash stdout-json: "\n"

#### read -d when there's no delimiter
{ read -d : part
echo $part $?
read -d : part
echo $part $?
} <<EOF
foo:bar
EOF
## STDOUT:
foo 0
bar 1
## END
## N-I dash STDOUT:
2
2
## END

#### mapfile
type mapfile >/dev/null 2>&1 || exit 0
printf '%s\n' {1..5..2} | {
Expand Down

0 comments on commit 16bbec6

Please sign in to comment.